You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ICE: -Z unstable-options without extra argument #21715
If I understand correctly, the problem is that "show-span" is missing from the getopts::Matches here in run_compiler:
if sess.unstable_options(){
sess.opts.show_span = matches.opt_str("show-span");}
It's missing due to the way we parse unstable options in handle_options:
First we try to parse command-line arguments using an [OptGroup] that only defines the stable options. If we succeed, then we return a getopts::Matches that only records the results for the stable options. (show-span is an unstable option, so it is omitted in this case.)
If we failed, then maybe it was because the command-line had an unstable option on it. We parse arguments again, this time using a [OptGroup] value with both stable and unstable options. If this succeeds, and if we found -Z unstable-options in the result, then we return a getopts::Matches recording results for all options.
One obvious fix is to ensure that getopts::Matches records the unstable options whenever -Z unstable-options is set. If the first parse succeeds, but that flag is set, then reparse.
Is it worth considering adding one or more run-make tests covering these use-cases for invoking rustc itself? It sounds like having such a test would have prevented the prior -Z unstable-options implementation (I did not want to block #23274 on such a requirement, but it might be a nice followup task.)
@rprichard actually, doing this nicely might be harder than I initially thought, since in all the cases listed above, the rustc invocation is expected to return a non-zero error code, right?
... so the run-make test is going to need to grep through the output ... it does not sound like a very fun test to maintain ... hmm.
Update: I guess we could at least ensure that we do not emit internal compiler error; that is probably good enough.
It's supposed to succeed quietly in the case where -Z unstable-options is passed. At one point, I had written a run-pass test to verify this, but I nixed it because running the test seemed wasteful. We could test this by adding -Z unstable-options to any existing test, but that conflates things, I think.
AFAIK, we don't have a test verifying that unstable options require -Z unstable-options to work.
@rprichard I guess I was solely looking at the original description ... running rustc -Z unstable-options on a newly created empty.rs shouldn't quietly succeed. But clearly you have a different test in mind, so okay.
Activity
japaric commentedon Jan 28, 2015
cc @alexcrichton
alexcrichton commentedon Jan 28, 2015
Oh dear, sorry about that!
mrmonday commentedon Feb 4, 2015
Just hit this myself - it would seem that the pretty option has disappeared too, making it very difficult to debug compiler plugins.
Munksgaard commentedon Feb 10, 2015
I believe that this specific error has been fixed, only to be replaced by this:
Note that the unspecified options is now
show-spaninstead ofpretty.Munksgaard commentedon Feb 10, 2015
And it also happens if you simply do
rustc -Z unstable-options help.rprichard commentedon Mar 8, 2015
If I understand correctly, the problem is that
"show-span"is missing from thegetopts::Matcheshere inrun_compiler:It's missing due to the way we parse unstable options in
handle_options:[OptGroup]that only defines the stable options. If we succeed, then we return agetopts::Matchesthat only records the results for the stable options. (show-spanis an unstable option, so it is omitted in this case.)[OptGroup]value with both stable and unstable options. If this succeeds, and if we found-Z unstable-optionsin the result, then we return agetopts::Matchesrecording results for all options.One obvious fix is to ensure that
getopts::Matchesrecords the unstable options whenever-Z unstable-optionsis set. If the first parse succeeds, but that flag is set, then reparse.I think I have a fix; I'll test it.
pnkfelix commentedon Mar 11, 2015
Is it worth considering adding one or more
run-maketests covering these use-cases for invokingrustcitself? It sounds like having such a test would have prevented the prior-Z unstable-optionsimplementation (I did not want to block #23274 on such a requirement, but it might be a nice followup task.)rprichard commentedon Mar 11, 2015
I think so. I'll look into it.
pnkfelix commentedon Mar 11, 2015
@rprichard actually, doing this nicely might be harder than I initially thought, since in all the cases listed above, the
rustcinvocation is expected to return a non-zero error code, right?... so the
run-maketest is going to need to grep through the output ... it does not sound like a very fun test to maintain ... hmm.Update: I guess we could at least ensure that we do not emit
internal compiler error; that is probably good enough.rprichard commentedon Mar 11, 2015
It's supposed to succeed quietly in the case where
-Z unstable-optionsis passed. At one point, I had written arun-passtest to verify this, but I nixed it because running the test seemed wasteful. We could test this by adding-Z unstable-optionsto any existing test, but that conflates things, I think.AFAIK, we don't have a test verifying that unstable options require
-Z unstable-optionsto work.pnkfelix commentedon Mar 11, 2015
@rprichard I guess I was solely looking at the original description ... running
rustc -Z unstable-optionson a newly createdempty.rsshouldn't quietly succeed. But clearly you have a different test in mind, so okay.rprichard commentedon Mar 11, 2015
Oh, right, the original test should still have failed for lack of a
mainfunction.Rollup merge of rust-lang#23274 - rprichard:fix-21715, r=pnkfelix