This is an archive of the discontinued LLVM Phabricator instance.

Do not perform the analysis based warning if all warnings are ignored
ClosedPublic

Authored by ogoffart on Nov 20 2017, 3:47 AM.

Details

Summary

Do not perform the analysis based warning if all warnings are ignored.

This saves some cycles when compiling with "-w".

But more importantly, for my tool, this fixes a potential crash which may happen on invalid code. Because the analysis might compute the CFG which crashes if the code contains invalid declaration. This does not happen normally with because we also don't perform these analysis if there was an error. But the tool is better at recovering and hit this condition.

Diff Detail

Event Timeline

ogoffart created this revision.Nov 20 2017, 3:47 AM

You probably want to add a test for this.

I do not know how to add a test: there is no real visible change for clang. It just should be faster when passing "-w".

I do not know how to add a test: there is no real visible change for clang. It just should be faster when passing "-w".

Oh right, i was thinking of something else there.

lib/Sema/AnalysisBasedWarnings.cpp
2084

I guess you can try commenting-out that entire check and seeing whether any test fails.

ogoffart marked an inline comment as done.Nov 20 2017, 6:29 AM
ogoffart added inline comments.
lib/Sema/AnalysisBasedWarnings.cpp
2084

Commenting out the entire if does not cause any test to fail.

You should add a test case that demonstrates code which would otherwise trigger an analysis-based warning but doesn't due to disabling all warnings.

ogoffart marked an inline comment as done.Nov 20 2017, 10:56 PM

You should add a test case that demonstrates code which would otherwise trigger an analysis-based warning but doesn't due to disabling all warnings.

No warnings are triggered. Because the diagnostic engine ignores them.
This just optimize the code so no analysis are preformed when no warning can be triggered.

You should add a test case that demonstrates code which would otherwise trigger an analysis-based warning but doesn't due to disabling all warnings.

No warnings are triggered. Because the diagnostic engine ignores them.
This just optimize the code so no analysis are preformed when no warning can be triggered.

Correct -- I'm asking for a test that ensures we don't regress the intended behavior of this patch by accident at some point (behavioral changes should always have at least one accompanying test).

ogoffart updated this revision to Diff 123720.Nov 20 2017, 11:48 PM

I added a test.
I did not add such test before because i know that calling Reset() on the diagnostics is kind of a hack.

This change does not have any behavioral difference normally, it is just an optimization that skips the computation of the CFG if all warnings are ignored.

This revision is now accepted and ready to land.Nov 21 2017, 12:02 PM
This revision was automatically updated to reflect the committed changes.