Skip to content

Support revisions on global commands #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RalfJung opened this issue Nov 26, 2022 · 7 comments · Fixed by #33
Closed

Support revisions on global commands #31

RalfJung opened this issue Nov 26, 2022 · 7 comments · Fixed by #33

Comments

@RalfJung
Copy link
Collaborator

RalfJung commented Nov 26, 2022

From what I could tell, revisions are currently only supported on error patterns. Would be nice to also support them on global commands, so that one can do things like

//@revisions: isolated free
//@only-target-linux
//[free]@compile-flags: -Zmiri-disable-isolation

FWIW I would have probably picked this syntax

//@revisions: isolated free
//@only-target-linux
//@[free]compile-flags: -Zmiri-disable-isolation

but that would be inconsistent with //[fn_ptr]~^ ERROR: pattern... maybe we should change that to //~[fn_ptr]^ ERROR: pattern, too, so that the first character after the // always indicates what this is?

OTOH, that might create parsing ambiguities if there is no ERROR level, and //[revision] is also consistent, so... whatever.

@oli-obk
Copy link
Owner

oli-obk commented Nov 28, 2022

We could require //@[revision]~ foo if you want to use revisions

@RalfJung
Copy link
Collaborator Author

That feels odd, I associate @ with global commands and ~ with patterns.

@oli-obk
Copy link
Owner

oli-obk commented Nov 28, 2022

considering square brackets in diagnostics are rare and you could always use a longer or regex pattern in that case (and get an error if that [revision] is not actually a revision), I think we should just use the possibly ambigous case

@RalfJung
Copy link
Collaborator Author

I forgot, do we even still have the no-error-level syntax?

@oli-obk
Copy link
Owner

oli-obk commented Nov 28, 2022

oh haha, no I scrapped that and added a global min-level flag to get the "don't require annotaitons for all notes" behaviour. Ok no ambiguity, woo

@RalfJung
Copy link
Collaborator Author

Right so the only question is if we want to incur all that churn for when/if rustc itself uses this crate.

@oli-obk
Copy link
Owner

oli-obk commented Nov 28, 2022

Right so the only question is if we want to incur all that churn for when/if rustc itself uses this crate.

I can incrementally move the rustc test suite to use the same syntax to keep the final diff small

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants