-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
compiletest: disable -Aunused for run-pass tests #64078
compiletest: disable -Aunused for run-pass tests #64078
Conversation
cc #43896 |
Please use |
If we're just going to re-add all the tests to have allow then there's probably no point in doing this, I was just posting this so I didn't forget and for some review on the output. I personally am not sure this is the right approach anymore, as this seems fairly noisy as you said and has a lot of false positives... |
It's not that many tests really. As for triaging them, I know that at least the warnings in the ATB tests can be safely ignored since I wrote all of those tests. |
Not sure I follow you, but I'm not particularly against adding |
So, my idea was that if a run-pass test has dead code, then it's either
So, I expected doing triage rather than adding |
@Mark-Simulacrum Oh ofc we should do triage and not stick |
I'll add the allows for ATB tests next session I have for impl work; happy to go either route for the rest of the tests. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Agreed. In fact, we are specifically working around this |
433711e
to
77bd34a
Compare
Updated. A few tests are moved to check-pass since we don't need to actually run them to verify their results, and otherwise most have allows tagged on. |
77bd34a
to
a959b47
Compare
Fixed review comments. |
a959b47
to
09a442d
Compare
Force-pushed fixes. |
@bors r+ |
📌 Commit 09a442d has been approved by |
51ad535
to
b338903
Compare
@bors r=petrochenkov |
📌 Commit b33890333b739db284aa527e583f957a5eddd51f has been approved by |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r- Rebase error, misunderstood test results locally. |
b338903
to
cb41ce3
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
cb41ce3
to
c17e109
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
c17e109
to
cf8a499
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
cf8a499
to
6fdbece
Compare
@bors r=petrochenkov |
📌 Commit 6fdbece has been approved by |
⌛ Testing commit 6fdbece with merge 288962a9274795bf6f0d30ea002fd371df6fdb41... |
…used, r=petrochenkov compiletest: disable -Aunused for run-pass tests Disabled the flag, but that led to quite a bit of fall out -- I think most of it is benign but I've not investigated thoroughly. r? @petrochenkov
@bors retry rolled up. |
Rollup of 4 pull requests Successful merges: - #64078 (compiletest: disable -Aunused for run-pass tests) - #64263 (Replace "feature gated" wording with "unstable".) - #64280 (Factor out pluralisation into syntax::errors) - #64288 (use 'get_toml' instead of regular expression) Failed merges: r? @ghost
Disabled the flag, but that led to quite a bit of fall out -- I think most of it is benign but I've not investigated thoroughly.
r? @petrochenkov