Project

General

Profile

Actions

Misc #20242

closed

`--parser=prism_without_warning`

Added by kddnewton (Kevin Newton) 3 months ago. Updated 3 months ago.

Status:
Closed
Assignee:
-
[ruby-core:116610]

Description

I was hoping to add another option to --parser which would be --parser=prism_without_warning. The issue is that it's making it hard to run the tests/specs that test against the output of stderr. This would do the exact same thing as --parser=prism it just wouldn't warn. (The name is derived from --dump=insns_without_opt, but I'm okay with anything.

Updated by Eregon (Benoit Daloze) 3 months ago

Do you have an example? That could help to clarify the need for this.
Is it problematic to modify these tests/specs to accept Prism warnings too? I think that would be best (since it would be needed longer-term anyway).
For compatibility, --parser=prism should at some point pass the test suite and specs, so that will mean some modifications to expected warning/parse errors.

Updated by kddnewton (Kevin Newton) 3 months ago

I think you misunderstood what I'm asking for. Right now CRuby warns with:

The compiler based on the Prism parser is currently experimental and compatibility with the compiler based on parse.y is not yet complete. Please report any issues you find on the `ruby/prism` issue tracker.

when you pass --parser=prism. I'm specifically asking to turn this off with --parser=prism_without_warning. I'm not asking to change the warning messages within Ruby itself.

The specs assert against the output of stderr, which means if this warning gets emitted, a lot of them fail. Specifically there are 70 specs that fail when this warning is present. In order to run the spec suite, I am currently manually commenting this out and recompiling.

Updated by Eregon (Benoit Daloze) 3 months ago

Oh right, that makes a lot more sense.
I wonder if it could be a configure flag or so to disable that warning, until it's stable enough to remove the warning entirely.

Updated by mame (Yusuke Endoh) 3 months ago

This issue was discussed at the dev meeting. No one agreed with --parser=prism_without_warning because it is too ad-hoc.

How about putting the warning in the "experimental" category? ruby -W:no-experimental --parser=prism will prevent the warning.

Updated by kddnewton (Kevin Newton) 3 months ago

That works for me, thanks!

Updated by kddnewton (Kevin Newton) 3 months ago

I've added the PR here: https://github.com/ruby/ruby/pull/9963.

@mame (Yusuke Endoh) would you mind reviewing? I had to add rb_warning_category_update(opt->warn.mask, opt->warn.set); when the -W option is passed because it needs to get updated before ruby_opt_init is called in order to properly track the state of the warning category. I hope that's okay.

Actions #7

Updated by kddnewton (Kevin Newton) 3 months ago

  • Status changed from Open to Closed
Actions

Also available in: Atom PDF

Like1
Like0Like0Like0Like0Like0Like0Like0