this 11yr old report has been relatively dormant for >7 years. Is this still an active concern?

There were two draft patches submitted that likely no longer apply, one was to enable parse-time customized warnings, the other was to let pkg_add specify parse-time warnings for pkg files, all of which were to work around issues with Octave applying warnings in rather non-selective manner.  Other discussions about broadcasting have been superseded by this becoming standard in both programs. Personally, even with some manually created files containing octave exclusive syntax/functions, I can't get the warning to trigger at ally.

I'm reverting the Assigned To, and changing status to Need Info: Is the issues still valid, and/or should it be changed to a Won't fix.

All patches attached to this bug report are lost.  Please upload them again, if possible.

This time with the files...

(file #37667, file #37668)

Following up from jwe's comment #9, I'm not sure if the PKG_ADD file should specify run-time warnings.  That would mean it would need to be 'stat'ed every time a function file is to make sure that it didn't change.  Would that overhead be excessive?

It also involves much more bookkeeping than simply changing parse-time settings.  The attached patch allows parse-time pragmas per file.  (This is just a minimally tested proof-of-concept -- please do NOT commit it.)


I've been thinking of a big overhaul of the warning system to make warnings more flexible and, perhaps, faster.

The flexibility would come from having a hierarchy of "scopes" of warnings:
  local_only  (current stack frame)
  file        (from pragma in current file)
  directory   (from pragma in PKG_ADD)
  local       (current stack frame and all descendants)
  global

At each scope, a warning's state can either be specified explicitly, or fall through to the next lower scope.

Each warning has an 8-bit state, consisting of four 2-bit fields.

1. Action: fallback, off, on, error
2. verbosity: fallback,
              quiet (set lastwarn only),
              verbose (print ID, to allow it to be turned off),
              normal
3. backtrace: fallback,
              off,
              on,
              unused
4. debugging: fallback,
              off,
              on,
              unused   (perhaps "on and beep")

Fallback first falls back to the status of "all" in the current scope, and if that is also fallback, it falls back to the next lower scope.

If the state is "off", the remaining fields are ignored. (This isn't quite what Matlab does; it treats "off" as "action=on, verbosity=quiet".)

Instead of using an octave_map, the octave: namespace would use a look-up table, autogenerated from warning_ids.m.  From C++ code, it could be accessed through a enumeration.  From .m code, it would be accessed by a binary search or hash table.

This also allows throwing errors when the state of a non-existent warning is set -- useful for example when a warning is re-named, or merged with another (like octave:automatic-broadcast was).

Having lookup more efficient would mean that we could have finer-grained IDs to give more flexibility about what is to be turned on/off.

The code to set initial states and define the enum of states could be generated from warning_ids.m, to keep them in sync.  That way, when introducing a new ID or the default of an existing one, it is only necessary to change one file.

A similar framework could apply to debug_on_...

To keep the overhead of creating a stack frame low, a stack frame would start with a NULL pointer to a warning_options object, and only create the object if a "local_only" is called.

One gotcha will be ensuring that "warning on ID" updates the global scope, and also ensures that the new state isn't overridden by one of the higher scopes -- unless we want it to be.  The purpose of the directory and file scopes is to override everything, so we would probably just want to make sure that local and local_only are set to fallback.

My planned work order would be
1. Replace the current warning_options (an octave_map) by a dedicated err_warn_debug object, keeping it as the same octave_map internally.

2. Implement per-directory parse-time pragmas.  (That is enough for this bug.)

3. Implement "Action = fallback".

4. Replace octave_map by look-up table (for "Octave:" namespace) + STL map (for everything else).

5. Implement remaining state fields

6. Re-implement "local" warnings.  (This may need to be done in conjunction with 4.)

7. Implement "file" and "local_only" scopes.

What do people think?

(file #36991)

> I wasn't aware that automatic broadcasting used a different syntax. [...]

A misunderstanding over the meaning of "syntax".  Yes, it is not a separate symbol for automatic broadcasting.  My point was why should we treat automatic broadcasting different from everything else that we do different from Matlab.

> You also refer to "doing input checking". As far as I can see, that is impossible. [...]

I mean to check the sizes of your vectors before doing your operations.  Or to just reshape them (which is what most often happens).  All of Octave plays with the same "rules" as you, so this is perfectly possible.

If your functions expect two vectors, your functions should check if they are of the right size and throw an error if they are not.

> [...] As it stands, it will fail the next time I assume that length(c)=2, at a place far from where the real error is.

You should have checked if `a` and `b` where both vectors of same shape.  If you are typing at the interpreter, you will catch the error pretty soon. If this is a function part of some library, it should perform input checking.

> automatic broadcasting (...) is something that we should have the option of being warned about.

The warnings were during 3.6 and 3.8.  That's a 2 years warning.

>> If the reason is that you didn't saw warnings back in 3.8 [...]
> No, I did see warnings back in 3.8, as I wanted.

I meant that you didn't saw warnings about use of broadcasting from inside core functions.  And that's because the code in core and in packages disabled the warnings locally. Automatic broadcasting is Octave valid syntax so functions should not have to disable it.

-----

Sounds to me, like the only thing you want is to enable the automatic broadcasting warning only. This is nothing to do with Matlab compatibility and a small part of the original bug report. It is about a syntax that you don't like and where you trip too often.

The language extension warning could be useful, even if it is not complete and doesn't catch every possible thing that is different from Matlab.

It is pretty useless, however, if it flags "system" code in Octave as well.

Marking functions as "system" or somehow otherwise exempt from this class of warnings makes sense to me, it's just a matter of how to do it.

We could introduce some kind of "pragma" comment that would apply to a single function:

#pragma: warning Octave:language-extension off

so that warnings could be disabled for the given function when it is parsed and evaluated.

To make this a global thing for a directory (or possibly a directory tree) we could place pragma directives in a special file that Octave reads when it parses function files (or adds the directory to the load path).  If we do this when the directory is added to the load path, then perhaps we could use the existing PKG_ADD file for this purpose.

Using a per-directory file might be better than maintaining a list separately (by naming them in some other startup file, for example).

@Mike, the reason I said "syntactically" is that I was aware of the distinction that Carnë was making, and wanting to separate behavioural issues from syntactic ones. In particular, I was thinking of automatic broadcasting, which really bugs me.  That needs to be a separate warning (as it was in 3.8, IIRC).

Most of my use of Octave is for lists rather than true vectors.  If I add a 1xn to an nx1, I want either to get an n-dimensional vector, or an error.  I don't want to get a matrix, which causes an error in an unrelated line of code.  What is my solution?  Turn on "warning("Octave:language-extension")".  Then I get inundated with errors.  Even if I didn't, it would preclude the many useful Octave extensions, like +=, ++, (A+B)(10), which are syntactically different and so presumably intentional.

It would be very easy to have a "systempath" variable, and exclude static syntax checking for all files read from that path.  This only needs to affect the parser.

@Carnë, I can see a case for getting rid of the option.  However, the overhead of checking syntax is only incurred when the file is parsed -- when it is first read in -- not every time it is executed.  (Try turning the warning on, and watch it appear only when a new code path is executed.)

Maybe this warning ID ought to just be removed completely.

Here's my argument. This warning ID is only useful to write Matlab code. For that purpose, it needs to identify Octave only syntax (auto-increment, double-quotes, endifs, etc) and use of functions with options not present in Matlab.

We are only covering the first case which is very very easy for the user to identify visually, and an even easier job for any text editor (exception for automatic broadcasting).

The second case is much more difficult for the user and therefore the most useful. This would require us to go through all the octave code, and throw warnings when a specific codepath is followed (for example, calling linspace() with a vector instead of scalar). For it to work properly, according to this bug report, it would also require that a function knows if it's being called by a "user" or a "core/library" function. We shouldn't be polluting all the octave and packages code with such warnings, that would be a lot of work to start, and a lot of maintenance.

If we are not going to cover the second case, this warning is pretty much useless and will only provide a false sense of security to the user.  We might as well remove and be done with this.

Also, I wonder how much of a performance issue is to check at every endif, endfunction, autoincrement, automatic broadcasting, etc, the warning status of the id.

This comment is so that this bug turns up on a search for "Octave:language-extension".

(My bug report about that warning was marked as a duplicate of this one, but a search for that warning didn't return this bug report, since the warning_id changed.)

I suppose Octave's "initial path" include OF packages?
i.e.,
  <OCTAVE_HOME/share/octave/packages/
and 
  ~/octave/ ?
(...which can change during the day as pkgs get loaded/unloaded)


AFAIU this warning_id is only useful for development code meant to also be run with the competition (ML / Freemat / JMathlib / (Scilab (after translation)) so your suggestion makes sense to me.

For a start, the entire <OCTAVE_HOME>/share/ tree can be excluded from this warning_id.

~/octave/ can be trickier as:
- on Linux that's where OF pkgs end up when installed as non-root (at least on my box), so it should be excluded for this warning_id, but:
- on Windows that's where I suppose people do their dev work (based on a.o., the 3.2.4 binary which created that directory) so it should be included for the warning_id.
AFAIK on Windows, m-files in OF packages are always installed in <OCTAVE_HOME/share/octave/packages/

Wouldn't checking for path in warning() induce a lot of overhead in the parser?