Project

General

Profile

Actions

Misc #20242

closed

`--parser=prism_without_warning`

Added by kddnewton (Kevin Newton) 11 months ago. Updated 10 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) 11 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) 11 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) 10 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) 10 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) 10 months ago

That works for me, thanks!

Updated by kddnewton (Kevin Newton) 10 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) 10 months ago

  • Status changed from Open to Closed
Actions

Also available in: Atom PDF

Like1
Like0Like0Like0Like0Like0Like0Like0