test: ignore unused arguments in tests #1762
Conversation
|
@swift-ci Please test |
|
I don't have push rights, could you please take care of this for me? |
|
-1 from me. I definitely want to know when I misspelled an argument and Clang allowed it through anyway. |
|
@jrose-apple while I agree that would be nice, it doesn't always work out. I just disabled it for the case where there can be an error due to the unused argument (-fmodules-cache-path). |
|
When is that unused? |
|
@jrose-apple I ran into a couple of cases of this causing errors against a newer clang. I suppose that we could limit this to master-next, but, Im not sure that there is any harm in pushing this to master instead. |
|
I maintain that this makes it harder to write new tests. We already ignore most warning output anyway, and it isn't an error, so why do we care? |
|
I ran into a few cases where the unused option was an error for the clang importer. BTW, this doesn't disable the unknown warnings, it just permits adding options that are unused into the invocation. |
|
Sorry, I'd like to see that there's not a reasonable workaround before accepting this. |
|
@compnerd I think we would rather catch those issues via this option and just update the tests. |
|
Okay, in that case, the following tests were the ones that complained:
Would it be preferable to add a new lit "tool" to avoid the -fmodule-cache-path? |
|
If there's really just that few, I think it's best to just add Thanks for looking into this, Saleem! |
compnerd
force-pushed
the
squelch-arguments
branch
2 times, most recently
from
527f06c to
d5e4c5e
Compare
These tests explicitly disable the use of modules. However, the lit configuration is such that the module cache path will be passed unconditionally to the clang invocation. Squelch the unused option warning (error).
compnerd
force-pushed
the
squelch-arguments
branch
from
d5e4c5e to
89d37bc
Compare
|
@jrose-apple sounds reasonable to me; Ive uploaded a modified version of the change that just applies it on those particular tests. Happy to help :-). |
|
Sanity-check time: @swift-ci Please smoke test |
|
@jrose-apple don't have push rights, would you mind pushing this for me? |
|
Oops, meant to after the checks passed. Thanks! |
What's in this pull request?
Resolved bug number: (SR-)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Note: Only members of the Apple organization can trigger swift-ci.
When invoking clang directly, with a newer clang, it is possible to get an
unused argument error failing the test. Silence this by telling clang to ignore
unused arguments. Since this is for testing purposes we are not too concerned
about ensuring that the exact set of arguments are supplied to the compiler.