Add a check against multiple only- directives in the same line
#123730
Comments
rustbot
added
the
needs-triage
|
The reason is that I believe compiletest directive parsing for where anything following |
jieyouxu
added
A-testsuite
|
We are able to detect unknown directives on their own, because we just have a non-exhaustive list of known directives and match against that. I suppose we could just try a match against the same known directive list for |
|
Brainstorming: maybe it'd be reasonable to require that the comment be in parens or or something like that? |
|
It is reasonable, the problem is that we have a lot of pre-existing directives that don't take this form (and we'd have to do another massive migration pass). I'm also not sure what |
|
This would've been a less surprising directive grammar from the beginning, but of course hindsight is 20/20. I'm not sure it's worth the churn to change the grammar now. What we can do now without churn, however, is to explicitly document that directives are intended to be one-per-line in the dev guide (rust-lang/rustc-dev-guide#1962). |
|
Without changing the syntax, we could probably still error out if the sentence right after the directive is a well known directive. That way we should be able to catch |
…ve, r=jieyouxu compiletest: error when finding a trailing directive This PR introduce a supplementary check that when checking for a compiletest directive, will also check that the next "word" after that directive is not also by itself a directive. This is done to avoid situations like this `//@ only-linux only-x86_64` where one might think that both directives are being taken into account while in fact the second in a comment, and so was ignored, until now. Related to rust-lang#123730 cc `@scottmcm` r? `@jieyouxu`
…ve, r=jieyouxu compiletest: error when finding a trailing directive This PR introduce a supplementary check that when checking for a compiletest directive, will also check that the next "word" after that directive is not also by itself a directive. This is done to avoid situations like this `//@ only-linux only-x86_64` where one might think that both directives are being taken into account while in fact the second in a comment, and so was ignored, until now. Related to rust-lang#123730 cc ``@scottmcm`` r? ``@jieyouxu``
Rollup merge of rust-lang#123753 - Urgau:compiletest-trailing-directive, r=jieyouxu compiletest: error when finding a trailing directive This PR introduce a supplementary check that when checking for a compiletest directive, will also check that the next "word" after that directive is not also by itself a directive. This is done to avoid situations like this `//@ only-linux only-x86_64` where one might think that both directives are being taken into account while in fact the second in a comment, and so was ignored, until now. Related to rust-lang#123730 cc ``@scottmcm`` r? ``@jieyouxu``
|
This was done. |
In 556b47e I wrote
which based on #123185 (comment) appears like it didn't run on just x64.
Assuming I'm right that this is what happened (if not, please close this) then it would be nice for the same helpful checker that tells me I did it wrong when I write
linux-onlyto also tell me that I can't put multipleonlychecks on the same line.The text was updated successfully, but these errors were encountered: