[PATCH] D84843: [Analyzer] Remove inclusion of uniqueing decl from diagnostic profile.

2020-07-30 Thread Balázs Kéri 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 rG1745ba41b196: [Analyzer] Remove inclusion of uniqueing decl 
from diagnostic profile. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84843

Files:
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/test/Analysis/report-uniqueing.cpp


Index: clang/test/Analysis/report-uniqueing.cpp
===
--- /dev/null
+++ clang/test/Analysis/report-uniqueing.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=security
+
+void bzero(void *, unsigned long);
+
+template  void foo(T l) {
+  // The warning comes from multiple instances and with
+  // different declarations that have same source location.
+  // One instance should be shown.
+  bzero(l, 1); // expected-warning{{The bzero() function is obsoleted}}
+}
+
+void p(int *p, unsigned *q) {
+  foo(p);
+  foo(q);
+}
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1134,7 +1134,6 @@
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID ) const {
   ID.Add(getLocation());
   ID.Add(getUniqueingLoc());
-  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);


Index: clang/test/Analysis/report-uniqueing.cpp
===
--- /dev/null
+++ clang/test/Analysis/report-uniqueing.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=security
+
+void bzero(void *, unsigned long);
+
+template  void foo(T l) {
+  // The warning comes from multiple instances and with
+  // different declarations that have same source location.
+  // One instance should be shown.
+  bzero(l, 1); // expected-warning{{The bzero() function is obsoleted}}
+}
+
+void p(int *p, unsigned *q) {
+  foo(p);
+  foo(q);
+}
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1134,7 +1134,6 @@
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID ) const {
   ID.Add(getLocation());
   ID.Add(getUniqueingLoc());
-  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84843: [Analyzer] Remove inclusion of uniqueing decl from diagnostic profile.

2020-07-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.

Aha, ok, yeah, so that's what caused it!

I guess if we want a uniqueing decl for this case, we should use the original 
template decl rather than the instantiation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84843

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


[PATCH] D84843: [Analyzer] Remove inclusion of uniqueing decl from diagnostic profile.

2020-07-29 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision.
vsavchenko added a comment.
This revision is now accepted and ready to land.

LGTM!  Thanks for the fast fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84843

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


[PATCH] D84843: [Analyzer] Remove inclusion of uniqueing decl from diagnostic profile.

2020-07-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
balazske requested review of this revision.

The uniqueing decl in PathDiagnostic is the declaration with the
uniqueing loc, as stated by documentation comments.
It is enough to include the uniqueing loc in the profile. It is possible
to have objects with different uniqueing decl but same location, at
least with templates. These belong to the same class and should have
same profile.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84843

Files:
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/test/Analysis/report-uniqueing.cpp


Index: clang/test/Analysis/report-uniqueing.cpp
===
--- /dev/null
+++ clang/test/Analysis/report-uniqueing.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=security
+
+void bzero(void *, unsigned long);
+
+template  void foo(T l) {
+  // The warning comes from multiple instances and with
+  // different declarations that have same source location.
+  // One instance should be shown.
+  bzero(l, 1); // expected-warning{{The bzero() function is obsoleted}}
+}
+
+void p(int *p, unsigned *q) {
+  foo(p);
+  foo(q);
+}
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1134,7 +1134,6 @@
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID ) const {
   ID.Add(getLocation());
   ID.Add(getUniqueingLoc());
-  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);


Index: clang/test/Analysis/report-uniqueing.cpp
===
--- /dev/null
+++ clang/test/Analysis/report-uniqueing.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=security
+
+void bzero(void *, unsigned long);
+
+template  void foo(T l) {
+  // The warning comes from multiple instances and with
+  // different declarations that have same source location.
+  // One instance should be shown.
+  bzero(l, 1); // expected-warning{{The bzero() function is obsoleted}}
+}
+
+void p(int *p, unsigned *q) {
+  foo(p);
+  foo(q);
+}
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -1134,7 +1134,6 @@
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID ) const {
   ID.Add(getLocation());
   ID.Add(getUniqueingLoc());
-  ID.AddPointer(getUniqueingLoc().isValid() ? getUniqueingDecl() : nullptr);
   ID.AddString(BugType);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits