Previously we only accepted the -fdiagnostics-misexpect-tolerance= at
CC1, when it should have been handled identically to
-fdiagnostics-hotness-threshold=. It should not have been required to
pass this flag w/ -Xclang as reported here:
https://reviews.llvm.org/D115907#inline-1440745
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/test/Profile/misexpect-branch.c | ||
---|---|---|
10 ↗ | (On Diff #516922) | The more common way to test this would be a test under clang/test/Driver/ that just checks that forwarding the flag works. |
clang/test/Profile/misexpect-branch.c | ||
---|---|---|
10 ↗ | (On Diff #516922) | Good point, I just defaulted to adding the RUN line here, but that's definitely the right way to test this. |
clang/test/Driver/clang_f_opts.c | ||
---|---|---|
144 | Instead of checking for absence of the warning, could we just check that the flag is present among the cc1 flags? I think that's how most of the other tests here do it. |
Update test to check if the flag is in cc1, instead of just looking for a warning.
clang/test/Driver/clang_f_opts.c | ||
---|---|---|
144 | It took me a minute to realize what you meant here. I was pretty surprised to find out that "-fdiagnostics-misexpect-tolerance=10" is how we test for cc1, but '-fdiagnostics-misexpect-tolerance=' is a test for an error/warning string. The reason I wrote the check this way was so that I wouldn't accidentally match on the non-warning case. TBH Ithink checking the cc1 flags like this is pretty brittle. They would all silently pass (even when they shouldn't) if the warning diagnostic used " instead of '. I'm sure some other tests would fail/need to be changed in that case, but there wouldn't be any indication that these tests wouldn't be functioning as intended. |
Instead of checking for absence of the warning, could we just check that the flag is present among the cc1 flags? I think that's how most of the other tests here do it.