The verbose option for parsing should have 3 levels, like $VERBOSE

[eregon]

There are 3 states for $VERBOSE (true, false, nil) and some parser warnings warn at false while some only warn at true (and of course none warn at nil):

ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]
$ ruby -e 'def foo; END {}; end' 
-e:1: warning: END in method; use at_exit
$ ruby -e '(1; 2)'            
$ ruby -w -e '(1; 2)'
-e:1: warning: unused literal ignored
-e:1: warning: possibly useless use of a literal in void context

But the current API only has true/false, and nil means default and not a separate value (AFAIK):
https://ruby.github.io/prism/rb/Prism.html#method-c-parse
And in fact pm_options_suppress_warnings_set takes a boolean.

Also the serialization has a reversed option called suppress warnings:
https://github.com/ruby/prism/blob/main/docs/serialization.md#apis
And that caused quite a bit of confusion (e.g. #2080 but not only), so a good occasion to change that to a 3-states verbose.

Originally posted by @eregon in #2080 (comment)

[eregon]

A typical encoding of $VERBOSE (nil, false, true) values is (0, 1, 2), which also reflects the ruby -W flag:

     -W[level=2]    Turns on verbose mode at the specified level without printing the version message at the beginning.
                    The level can be;

                          0       Verbose mode is "silence". It sets the $VERBOSE to nil.

                          1       Verbose mode is "medium". It sets the $VERBOSE to false.

                          2 (default) Verbose mode is "verbose". It sets the $VERBOSE to true.  -W2 is the same as -w

(the (default) there means when passing -W and not -W, actual default without flags is false)

[kddnewton]

Yeah, makes sense

[eregon]

One more very-related concern about this, that I forgot when filing this issue but remembered now:
I think we need to have a boolean on ‎Prism::ParseWarning, for whether this is a $VERBOSE==true warning or $VERBOSE==false warning.

The reason is when caching the serialized output of a file, it would be best to reuse the serialized output if nothing changes but the $VERBOSE level (which is a runtime value).

TruffleRuby already uses some kind of in-process parse caching (which Truffle provides) and so how it deals with that is all warnings (so as if $VERBOSE was always true) are always saved into an array, and around the compiled AST there is an extra node (EmitWarningsNode)/instruction to emit warnings (or not) based on the $VERBOSE level when executing it.

A flag per ‎Prism::ParseWarning seems better than two lists of warnings so that the order of warnings is kept.

This means the 3 levels for the option might not be strictly needed, because ‎Prism::ParseWarning objects know which kind of warning they are ($VERBOSE==true or $VERBOSE==false).
OTOH it might still be useful/convenient (when not caching the result per file, and it's nice it maps to levels of $VERBOSE clearly) and also faster as if e.g. $VERBOSE is false (the default) then $VERBOSE==true warnings don't need to be generated (similar for $VERBOSE==nil case where no need to allocate warnings at all).
Basically it's the same as having the current 2-levels verbose/suppress_warnings, as far as I understand it's a performance optimization but is otherwise not strictly necessary (one could just ignore the warnings in the ParseResult).

[enebo]

@eregon From reading that I got that we could emit all warning modes into all parsing and then allow consumer of that to emit the proper subset of warnings are used depend on on $VERBOSE value?

If not this is definitely a requirement for precompilation since a precompiled file would not know what verbosity it will be loaded with. We 100% want this for JRuby but I would be surprised if everyone will not want the option.

So I think we should remove data field and always emit all warnings with their verbosity level (0-2) and then let the runtime decide which ones they should warn with.

[kddnewton]

Oh my that's definitely a consideration. In that case I think this ticket becomes that Prism::ParseWarning should have a verbosity level and that's it

[eregon]

Yep that would work and be simpler.
The only downside is it might take a bit of extra time to generate warnings which might not be needed, but it's probably negligible.

[eregon]

I'll try to fix this.