This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Introduce common bug category "Unused code".
ClosedPublic

Authored by NoQ on Mar 16 2021, 1:39 PM.

Details

Summary

This is a cosmetic change. "Dead store" will now be displayed as "Unused code" which is a nice broad category that could incorporate more than one checker. It also doesn't mention dead people which despite being a common source of inside jokes in the static analyzer community doesn't need to be translated onto innocent users.

There's one more alpha checker that fits into the category, namely UnreachableCode checker which flags code that wasn't covered by symbolic execution. I don't immediately plan to actually make this checker useful as it has to be quite an undertaking.

A broader plan that i have here is that some clang-tidy checks (eg., bugprone-redundant-branch-condition or misc-redundant-expression) can be put into that category through D95403.

Diff Detail

Event Timeline

NoQ created this revision.Mar 16 2021, 1:39 PM
NoQ requested review of this revision.Mar 16 2021, 1:40 PM
Charusso accepted this revision.Mar 16 2021, 1:49 PM

Great idea!

clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
11413

"Initialization was never used"? (Swift style: https://swift.godbolt.org/z/c17EMb)

This revision is now accepted and ready to land.Mar 16 2021, 1:49 PM
NoQ added inline comments.Mar 16 2021, 4:04 PM
clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
11413

That's a scan-build bugtype so it follows a different tradition. The actual warning is a few lines above and it says "Value stored to 'foo' during its initialization is never read".

Additionally, unlike clang or swift (which inherits the tradition from clang), static analyzer warnings are traditionally expected to be complete sentences. So we typically won't be able to synchronize wording.

But in this case it might be possible given that the warnings already have a similar structure. I'll think about it ^.^

steakhal accepted this revision.Mar 17 2021, 2:00 AM

Unused code seems to be broader and probably a better fit for a generic bug category.

clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
263–265

This is the only case where I think it's debatable.
However, I have no better option, and I'm still in favor of your change.

This revision was landed with ongoing or failed builds.Mar 17 2021, 8:58 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2021, 8:58 PM