[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

2021-03-17 Thread Artem Dergachev via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc75b2261a0aa: [analyzer] Introduce common bug category 
Unused code. (authored by dergachev.a).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98741/new/

https://reviews.llvm.org/D98741

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -2169,7 +2169,7 @@
 

descriptionValue stored to foo during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -5654,7 +5654,7 @@
 

descriptionValue stored to x is never read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores

Index: clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -382,7 +382,7 @@
 

descriptionValue stored to x during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -450,7 +450,7 @@
 

descriptionValue stored to obj1 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -518,7 +518,7 @@
 

descriptionValue stored to obj4 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -586,7 +586,7 @@
 

descriptionValue stored to obj5 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -654,7 +654,7 @@
 

descriptionValue stored to obj6 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1064,7 +1064,7 @@
 

descriptionValue stored to cf1 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1132,7 +1132,7 @@
 

descriptionValue stored to cf2 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1200,7 +1200,7 @@
 

descriptionValue stored to cf3 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1268,7 +1268,7 @@
 

descriptionValue stored to cf4 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===
--- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -2368,7 +2368,7 @@
 

descriptionValue stored to x is never read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores

@@ -11409,7 +11409,7 @@
 

descriptionValue stored to foo during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
===
--- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
+++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
@@ -22,6 +22,7 @@
 const char *const CXXObjectLifecycle = "C++ object lifecycle";
 const char *const CXXMoveSemantics = "C++ move semantics";
 const char *const SecurityError = "Security error";
+const char *const UnusedCode = "Unused code";
 } // namespace

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

2021-03-17 Thread Balázs Benics via Phabricator via cfe-commits 
steakhal accepted this revision.
steakhal added a comment.

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




Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:263-264
 
-BR.EmitBasicReport(AC->getDecl(), Checker, BugType, "Dead store", os.str(),
-   L, R, Fixits);
+BR.EmitBasicReport(AC->getDecl(), Checker, BugType, categories::UnusedCode,
+   os.str(), L, R, Fixits);
   }

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.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98741/new/

https://reviews.llvm.org/D98741

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

2021-03-16 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist:11413
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

Charusso wrote:
> "Initialization was never used"? (Swift style: 
> https://swift.godbolt.org/z/c17EMb)
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 [[ 
https://github.com/apple/swift/blob/main/docs/Diagnostics.md | 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 ^.^


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98741/new/

https://reviews.llvm.org/D98741

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

2021-03-16 Thread Csaba Dabis via Phabricator via cfe-commits 
Charusso accepted this revision.
Charusso added a comment.
This revision is now accepted and ready to land.

Great idea!




Comment at: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist:11413
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98741/new/

https://reviews.llvm.org/D98741

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

2021-03-16 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ created this revision.
NoQ added reviewers: vsavchenko, xazax.hun, Szelethus, martong, 
baloghadamsoftware, Charusso, steakhal, balazske, ASDenysPetrov.
Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, 
rnkovacs, szepet.
NoQ requested review of this revision.

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 .


Repository:
  rC Clang

https://reviews.llvm.org/D98741

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -2169,7 +2169,7 @@
 

descriptionValue stored to foo during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -5654,7 +5654,7 @@
 

descriptionValue stored to x is never read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores

Index: clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -382,7 +382,7 @@
 

descriptionValue stored to x during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -450,7 +450,7 @@
 

descriptionValue stored to obj1 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -518,7 +518,7 @@
 

descriptionValue stored to obj4 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -586,7 +586,7 @@
 

descriptionValue stored to obj5 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -654,7 +654,7 @@
 

descriptionValue stored to obj6 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1064,7 +1064,7 @@
 

descriptionValue stored to cf1 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1132,7 +1132,7 @@
 

descriptionValue stored to cf2 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1200,7 +1200,7 @@
 

descriptionValue stored to cf3 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1268,7 +1268,7 @@
 

descriptionValue stored to cf4 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===
--- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -2368,7 +2368,7 @@
 

descriptionValue stored to x is never read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores