[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc3c80c38643: [ASTMatchers] adds isComparisonOperator to 
BinaryOperator and… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75800

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2689,6 +2689,20 @@
   notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(IsComparisonOperator, Basic) {
+  StatementMatcher BinCompOperator = binaryOperator(isComparisonOperator());
+  StatementMatcher CXXCompOperator =
+  cxxOperatorCallExpr(isComparisonOperator());
+
+  EXPECT_TRUE(matches("void x() { int a; a == 1; }", BinCompOperator));
+  EXPECT_TRUE(matches("void x() { int a; a > 2; }", BinCompOperator));
+  EXPECT_TRUE(matches("struct S { bool operator==(const S&); };"
+  "void x() { S s1, s2; bool b1 = s1 == s2; }",
+  CXXCompOperator));
+  EXPECT_TRUE(
+  notMatches("void x() { int a; if(a = 0) return; }", BinCompOperator));
+}
+
 TEST(HasInit, Basic) {
   EXPECT_TRUE(
 matches("int x{0};",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -358,6 +358,7 @@
   REGISTER_MATCHER(isClass);
   REGISTER_MATCHER(isClassMessage);
   REGISTER_MATCHER(isClassMethod);
+  REGISTER_MATCHER(isComparisonOperator);
   REGISTER_MATCHER(isConst);
   REGISTER_MATCHER(isConstQualified);
   REGISTER_MATCHER(isConstexpr);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4783,7 +4783,7 @@
 ///(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
 /// \code
 ///   struct S { S& operator=(const S&); };
-///   void x() { S s1, s2; s1 = s2; })
+///   void x() { S s1, s2; s1 = s2; }
 /// \endcode
 AST_POLYMORPHIC_MATCHER(isAssignmentOperator,
 AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
@@ -4791,6 +4791,26 @@
   return Node.isAssignmentOp();
 }
 
+/// Matches comparison operators.
+///
+/// Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+/// \code
+///   if (a == b)
+/// a += b;
+/// \endcode
+///
+/// Example 2: matches s1 < s2
+///(matcher = cxxOperatorCallExpr(isComparisonOperator()))
+/// \code
+///   struct S { bool operator<(const S& other); };
+///   void x(S s1, S s2) { bool b1 = s1 < s2; }
+/// \endcode
+AST_POLYMORPHIC_MATCHER(isComparisonOperator,
+AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
+CXXOperatorCallExpr)) {
+  return Node.isComparisonOp();
+}
+
 /// Matches the left hand side of binary operator expressions.
 ///
 /// Example matches a (matcher = binaryOperator(hasLHS()))
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -118,6 +118,22 @@
   }
   bool isAssignmentOp() const { return isAssignmentOp(getOperator()); }
 
+  static bool isComparisonOp(OverloadedOperatorKind Opc) {
+switch (Opc) {
+case OO_EqualEqual:
+case OO_ExclaimEqual:
+case OO_Greater:
+case OO_GreaterEqual:
+case OO_Less:
+case OO_LessEqual:
+case OO_Spaceship:
+  return true;
+default:
+  return false;
+}
+  }
+  bool isComparisonOp() const { return isComparisonOp(getOperator()); }
+
   /// Is this written as an infix binary operator?
   bool isInfixBinaryOp() const;
 
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -2157,7 +2157,21 @@
 Example 2: matches s1 = s2
(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
   struct S { S operator=(const S); };
-  void x() { S s1, s2; s1 = s2; })
+  void x() { S s1, s2; s1 = s2; }
+
+
+
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1BinaryOperator.html;>BinaryOperatorisComparisonOperator
+Matches comparison operators.
+
+Example 1: matches a == b (matcher = 

[clang-tools-extra] fc3c80c - [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-03-09T00:05:10Z
New Revision: fc3c80c38643aff6c4744433ab485c7550ee77b9

URL: 
https://github.com/llvm/llvm-project/commit/fc3c80c38643aff6c4744433ab485c7550ee77b9
DIFF: 
https://github.com/llvm/llvm-project/commit/fc3c80c38643aff6c4744433ab485c7550ee77b9.diff

LOG: [ASTMatchers] adds isComparisonOperator to BinaryOperator and 
CXXOperatorCallExpr

Reviewers: aaron.ballman, gribozavr2

Reviewed By: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75800

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang/docs/LibASTMatchersReference.html
clang/include/clang/AST/ExprCXX.h
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/Dynamic/Registry.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index 90fcf38f83b7..99214ccf2e79 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -567,7 +567,7 @@ matchRelationalIntegerConstantExpr(StringRef Id) {
   std::string OverloadId = (Id + "-overload").str();
 
   const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
-  isComparisonOperator(), expr().bind(Id),
+  matchers::isComparisonOperator(), expr().bind(Id),
   anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
   hasRHS(matchIntegerConstantExpr(Id))),
 allOf(hasLHS(matchIntegerConstantExpr(Id)),
@@ -943,7 +943,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder 
*Finder) {
   const auto SymRight = matchSymbolicExpr("rhs");
 
   // Match expressions like: x  0xFF == 0xF00.
-  Finder->addMatcher(binaryOperator(isComparisonOperator(),
+  Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
 hasEitherOperand(BinOpCstLeft),
 hasEitherOperand(CstRight))
  .bind("binop-const-compare-to-const"),
@@ -951,14 +951,14 @@ void 
RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
 
   // Match expressions like: x  0xFF == x.
   Finder->addMatcher(
-  binaryOperator(isComparisonOperator(),
+  binaryOperator(matchers::isComparisonOperator(),
  anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft
   .bind("binop-const-compare-to-sym"),
   this);
 
   // Match expressions like: x  10 == x  12.
-  Finder->addMatcher(binaryOperator(isComparisonOperator(),
+  Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
 hasLHS(BinOpCstLeft), 
hasRHS(BinOpCstRight),
 // Already reported as redundant.
 unless(operandsAreEquivalent()))

diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index dca7aa5c2cf7..f0faed7f0f8f 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -2157,7 +2157,21 @@ Narrowing Matchers
 Example 2: matches s1 = s2
(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
   struct S { S operator=(const S); };
-  void x() { S s1, s2; s1 = s2; })
+  void x() { S s1, s2; s1 = s2; }
+
+
+
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1BinaryOperator.html;>BinaryOperatorisComparisonOperator
+Matches 
comparison operators.
+
+Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+  if (a == b)
+a += b;
+
+Example 2: matches s1  s2
+   (matcher = cxxOperatorCallExpr(isComparisonOperator()))
+  struct S { bool operator(const S other); };
+  void x(S s1, S s2) { bool b1 = s1  s2; }
 
 
 
@@ -2616,7 +2630,21 @@ Narrowing Matchers
 Example 2: matches s1 = s2
(matcher = cxxOperatorCallExpr(isAssignmentOperator()))
   struct S { S operator=(const S); };
-  void x() { S s1, s2; s1 = s2; })
+  void x() { S s1, s2; s1 = s2; }
+
+
+
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXOperatorCallExpr.html;>CXXOperatorCallExprisComparisonOperator
+Matches 
comparison operators.
+
+Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+  if (a == b)
+a += b;
+
+Example 2: matches s1  s2
+   (matcher = cxxOperatorCallExpr(isComparisonOperator()))
+  struct S { bool operator(const S other); };
+  void x(S s1, S s2) { bool b1 = s1  s2; }
 
 
 

diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index cea360d12e91..2bd68eec175a 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ 

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 updated this revision to Diff 249019.
boga95 marked 2 inline comments as done.
boga95 added a comment.

Rebase to master.


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

https://reviews.llvm.org/D71524

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/test/Analysis/taint-generic.cpp

Index: clang/test/Analysis/taint-generic.cpp
===
--- clang/test/Analysis/taint-generic.cpp
+++ clang/test/Analysis/taint-generic.cpp
@@ -124,3 +124,127 @@
   foo.myMemberScanf("%d", );
   Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
 }
+
+// Test I/O
+namespace std {
+template 
+class char_traits {};
+
+class ios_base {};
+
+template >
+class basic_ios : public ios_base {};
+
+template >
+class basic_istream : virtual public basic_ios {};
+template >
+class basic_ostream : virtual public basic_ios {};
+
+using istream = basic_istream;
+using ostream = basic_ostream;
+using wistream = basic_istream;
+
+istream >>(istream , int );
+wistream >>(wistream , int );
+
+extern istream cin;
+extern istream wcin;
+
+template >
+class basic_fstream : public basic_istream, public basic_ostream {
+public:
+  basic_fstream(const char *) {}
+};
+template >
+class basic_ifstream : public basic_istream {
+public:
+  basic_ifstream(const char *) {}
+};
+
+using ifstream = basic_ifstream;
+using fstream = basic_ifstream;
+
+template 
+class allocator {};
+
+namespace __cxx11 {
+template <
+class CharT,
+class Traits = std::char_traits,
+class Allocator = std::allocator>
+class basic_string {
+public:
+  const char *c_str();
+  basic_string operator=(const basic_string &);
+
+private:
+  unsigned size;
+  char *ptr;
+};
+} // namespace __cxx11
+
+using string = __cxx11::basic_string;
+
+istream >>(istream , string );
+istream (istream , string );
+} // namespace std
+
+void testCin() {
+  int x, y;
+  std::cin >> x >> y;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testWcin() {
+  int x, y;
+  std::wcin >> x >> y;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void mySink(const std::string &, int, const char *);
+
+void testFstream() {
+  std::string str;
+  std::ifstream file("example.txt");
+  file >> str;
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testIfstream() {
+  std::string str;
+  std::fstream file("example.txt");
+  file >> str;
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testStdstring() {
+  std::string str1;
+  std::cin >> str1;
+
+  std::string  = str1;
+  mySink(str2, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+
+  const std::string  = str1;
+  mySink(str3, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testTaintedThis() {
+  std::string str;
+  mySink(std::string(), 0, str.c_str()); // no-warning
+
+  std::cin >> str;
+  mySink(std::string(), 0, str.c_str()); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testOverloadedAssignmentOp() {
+  std::string str1, str2;
+  std::cin >> str1;
+  str2 = str1;
+  mySink(str2, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testGetline() {
+  std::string str;
+  std::getline(std::cin, str);
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/Taint.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/Taint.cpp
+++ clang/lib/StaticAnalyzer/Checkers/Taint.cpp
@@ -152,6 +152,14 @@
 return isTainted(State, Sym, Kind);
   if (const MemRegion *Reg = V.getAsRegion())
 return isTainted(State, Reg, Kind);
+  if (auto LCV = V.getAs()) {
+if (Optional binding =
+State->getStateManager().getStoreManager().getDefaultBinding(
+*LCV)) {
+  if (SymbolRef Sym = binding->getAsSymbol())
+return isTainted(State, Sym, Kind);
+}
+  }
   return false;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -138,6 +138,9 @@
   bool checkPre(const CallEvent , const FunctionData ,
 CheckerContext ) const;
 
+  /// Add taint sources for operator>> on pre-visit.
+  static bool addOverloadedOpPre(const CallEvent , CheckerContext );
+
   /// Add taint sources on a pre-visit. Returns true on matching.
   bool addSourcesPre(const CallEvent , const FunctionData ,
  CheckerContext ) const;
@@ -154,6 +157,9 @@
   /// and thus, is tainted.
   static bool 

[PATCH] D72635: Allow arbitrary capability name in Thread Safety Analysis

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D72635#1911671 , @aaron.ballman 
wrote:

> However, do we want to diagnose when the capability strings are different?


Hmm, interesting question. The way I think about ordering capabilities is this: 
assuming I have more than one capability in my program, and occasionally I hold 
multiple capabilities at the same time. Then deadlocks can occur, unless I'm 
being careful. One way to avoid deadlocks is to have a partial order on 
capabilities that is a total order on the subsets of capabilities that are 
acquired together. It's not hard to see that this is sufficient, and I even 
think it's necessary. (In the sense that whenever I have a deadlock-free 
program, such an order exists. I have no proof for that, but that's just for 
lack of trying.)

Now if I acquire capabilities of different types, then the order also has to 
incorporate these different types. So my answer would be no, I think we have to 
allow this.

In fact (but that's for another change) I've even thought about warning when a 
cap. is acquired and it's not ordered with another already-held capability 
(under `-beta` of course).


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

https://reviews.llvm.org/D72635



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

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, semantic analysis

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert planned changes to this revision.
jdoerfert added a comment.

I'll try to remove the extra namespace and I'll split the 
`function(is_definition)` matcher out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75779



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

[PATCH] D75827: Add new `attribute push` matcher `function(is_definition)`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: erichkeane, aaron.ballman.
Herald added a subscriber: bollu.
Herald added a project: clang.

This can also be used as a subject for attributes.

TODO: Tests are missing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75827

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7690,6 +7690,26 @@
   }
 }
 
+namespace {
+/// The `attribute push` matcher `function(is_definition)` needs to know if a
+/// function declaration is a definition but we might check before the relevant
+/// parts have been parsed. This RAII helper will set the `WillHaveBody` flag
+/// if appropriate to make the matcher possible.
+struct MarkDeclerationsForParamMatcherRAII {
+  FunctionDecl *FD;
+  bool OldWillHaveBody;
+  MarkDeclerationsForParamMatcherRAII(FunctionDecl *FD, const Declarator )
+  : FD(FD), OldWillHaveBody(FD && FD->willHaveBody()) {
+if (FD && PD.isFunctionDefinition())
+  FD->setWillHaveBody(true);
+  }
+  ~MarkDeclerationsForParamMatcherRAII() {
+if (FD)
+  FD->setWillHaveBody(OldWillHaveBody);
+  }
+};
+} // namespace
+
 /// ProcessDeclAttributes - Given a declarator (PD) with attributes indicated 
in
 /// it, apply them to D.  This is a bit tricky because PD can have attributes
 /// specified in many different places, and we need to find and apply them all.
@@ -7710,7 +7730,10 @@
   ProcessDeclAttributeList(S, D, PD.getAttributes());
 
   // Apply additional attributes specified by '#pragma clang attribute'.
-  AddPragmaAttributes(S, D);
+  {
+MarkDeclerationsForParamMatcherRAII MDFPM(dyn_cast(D), PD);
+AddPragmaAttributes(S, D);
+  }
 }
 
 /// Is the given declaration allowed to use a forbidden type?
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -101,6 +101,10 @@
 [{!S->isStatic() && !S->isConst()}],
 "non-static non-const member functions">;
 
+def FunctionDefinition : SubsetSubjectisThisDeclarationADefinition()}],
+   "function definition">;
+
 def ObjCInstanceMethod : SubsetSubjectisInstanceMethod()}],
"Objective-C instance methods">;
@@ -417,8 +421,11 @@
 def SubRuleForCXXMethod : AttrSubjectMatcherSubRule<"is_member", [CXXMethod]> {
   let LangOpts = [CPlusPlus];
 }
+// function(is_definition)
+def SubRuleForFunctionDefiniton
+: AttrSubjectMatcherSubRule<"is_definition", [FunctionDefinition]> {}
 def SubjectMatcherForFunction : AttrSubjectMatcherRule<"function", [Function], 
[
-  SubRuleForCXXMethod
+  SubRuleForCXXMethod, SubRuleForFunctionDefiniton
 ]>;
 // hasType is abstract, it should be used with one of the sub-rules.
 def SubjectMatcherForType : AttrSubjectMatcherRule<"hasType", [], [


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7690,6 +7690,26 @@
   }
 }
 
+namespace {
+/// The `attribute push` matcher `function(is_definition)` needs to know if a
+/// function declaration is a definition but we might check before the relevant
+/// parts have been parsed. This RAII helper will set the `WillHaveBody` flag
+/// if appropriate to make the matcher possible.
+struct MarkDeclerationsForParamMatcherRAII {
+  FunctionDecl *FD;
+  bool OldWillHaveBody;
+  MarkDeclerationsForParamMatcherRAII(FunctionDecl *FD, const Declarator )
+  : FD(FD), OldWillHaveBody(FD && FD->willHaveBody()) {
+if (FD && PD.isFunctionDefinition())
+  FD->setWillHaveBody(true);
+  }
+  ~MarkDeclerationsForParamMatcherRAII() {
+if (FD)
+  FD->setWillHaveBody(OldWillHaveBody);
+  }
+};
+} // namespace
+
 /// ProcessDeclAttributes - Given a declarator (PD) with attributes indicated in
 /// it, apply them to D.  This is a bit tricky because PD can have attributes
 /// specified in many different places, and we need to find and apply them all.
@@ -7710,7 +7730,10 @@
   ProcessDeclAttributeList(S, D, PD.getAttributes());
 
   // Apply additional attributes specified by '#pragma clang attribute'.
-  AddPragmaAttributes(S, D);
+  {
+MarkDeclerationsForParamMatcherRAII MDFPM(dyn_cast(D), PD);
+AddPragmaAttributes(S, D);
+  }
 }
 
 /// Is the given declaration allowed to use a forbidden type?
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -101,6 +101,10 @@
 [{!S->isStatic() && !S->isConst()}],
 "non-static non-const member functions">;
 

[clang] 073dbaa - Fix GCC warnings. NFC.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2020-03-08T13:00:36-04:00
New Revision: 073dbaae39724ea860b5957fe47ecc1c2a84b197

URL: 
https://github.com/llvm/llvm-project/commit/073dbaae39724ea860b5957fe47ecc1c2a84b197
DIFF: 
https://github.com/llvm/llvm-project/commit/073dbaae39724ea860b5957fe47ecc1c2a84b197.diff

LOG: Fix GCC warnings. NFC.

Added: 


Modified: 
clang/lib/AST/ItaniumMangle.cpp
clang/lib/Index/USRGeneration.cpp

Removed: 




diff  --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 5cc66a0a5778..63e34653637e 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -641,7 +641,7 @@ void CXXNameMangler::mangle(GlobalDecl GD) {
   //::= 
   //::= 
   Out << "_Z";
-  if (const FunctionDecl *FD = dyn_cast(GD.getDecl()))
+  if (isa(GD.getDecl()))
 mangleFunctionEncoding(GD);
   else if (const VarDecl *VD = dyn_cast(GD.getDecl()))
 mangleName(VD);

diff  --git a/clang/lib/Index/USRGeneration.cpp 
b/clang/lib/Index/USRGeneration.cpp
index f3eb653f10fa..7972d0a047c2 100644
--- a/clang/lib/Index/USRGeneration.cpp
+++ b/clang/lib/Index/USRGeneration.cpp
@@ -388,7 +388,7 @@ static const ObjCCategoryDecl *getCategoryContext(const 
NamedDecl *D) {
   if (auto *ICD = dyn_cast(D->getDeclContext()))
 return ICD->getCategoryDecl();
   return nullptr;
-};
+}
 
 void USRGenerator::VisitObjCMethodDecl(const ObjCMethodDecl *D) {
   const DeclContext *container = D->getDeclContext();



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

[PATCH] D72635: Allow arbitrary capability name in Thread Safety Analysis

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D72635#1911495 , @aaronpuchert 
wrote:

> @aaron.ballman, I've just noted that one of the `-Wthread-safety-attributes` 
> warnings says 
> 
>
> > //A// attribute can only be applied in a context annotated with 
> > ‘capability(“mutex”)’ attribute
>
> This should be changed then, right? We never enforced the name anyway.


Yeah, something needs to change here. For this diagnostic specifically, we care 
that the type is a capability-annotated type and if it's not, we want to 
diagnose. However, do we want to diagnose when the capability strings are 
different? e.g.,

  typedef int __attribute__((capability("role"))) ThreadRole;
  typedef int __attribute__((capability("noodle"))) Noodle;
  
  Noodle N;
  ThreadRole Data __attribute__((acquired_before(N)));


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

https://reviews.llvm.org/D72635



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

[PATCH] D75740: [ASTImporter] Corrected import of repeated friend declarations.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Balazs,
This look almost good to me except some comments inline.




Comment at: clang/lib/AST/ASTImporter.cpp:3635
 
+static std::tuple
+getFriendCountAndPosition(FriendDecl *FD) {

It's better to turn the tuple into a named struct (`FriendCountAndPosition`?) 
to make the code more readable.



Comment at: clang/lib/AST/ASTImporter.cpp:3636
+static std::tuple
+getFriendCountAndPosition(FriendDecl *FD) {
+  unsigned int FriendCount = 0;

`const FriendDecl *FD`?



Comment at: clang/lib/AST/ASTImporter.cpp:3639
+  llvm::Optional FriendPosition;
+  auto *RD = cast(FD->getLexicalDeclContext());
+  if (FD->getFriendType()) {

const?



Comment at: clang/lib/AST/ASTImporter.cpp:3640
+  auto *RD = cast(FD->getLexicalDeclContext());
+  if (FD->getFriendType()) {
+QualType TypeOfFriend = FD->getFriendType()->getType().getCanonicalType();

```if (const TypeSourceInfo *FriendType = FD->getFriendType()) {
QualType TypeOfFriend = FriendType->getType().getCanonicalType();
...```
?



Comment at: clang/lib/AST/ASTImporter.cpp:3699
+
+  assert(ImportedEquivalentFriends.size() <= std::get<0>(CountAndPosition) &&
+ "Class with non-matching friends is imported, ODR check wrong?");

Why not strictly equal?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4009
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendType) {
+  auto Code =
+  R"(

clang-tidy wants this to be `const auto* Code`.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:4034
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
+  auto Code =
+  R"(

clang-tidy wants this to be `const auto* Code`.



Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:801
+TEST_F(StructuralEquivalenceRecordTest, SameFriendMultipleTimes) {
+  auto t = makeNamedDecls("struct foo{ friend class X; };",
+  "struct foo{ friend class X; friend class X; };",

We need to place spaces after `foo`.



Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:811
+  Lang_CXX);
+  EXPECT_FALSE(testStructuralMatch(t));
+}

Could you please add a positive test with two `struct foo{ friend class X; 
friend class Y; };",` structures as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75740



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

[PATCH] D75732: [ASTImporter] Added visibility check for variable templates.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75732



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

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis marked 2 inline comments as done.
zinovy.nis added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured 
const here
 };
   }

Quuxplusone wrote:
> zinovy.nis wrote:
> > aaron.ballman wrote:
> > > One more test case to try out (it might be a FIXME because I imagine this 
> > > requires flow control to get right):
> > > ```
> > > A a;
> > > std::move(a);
> > > 
> > > auto lambda = [=] {
> > >   a.foo(); // Use of 'a' after it was moved
> > > }
> > > ```
> > `a` in lambda is `const`, but it's not moved inside lambda, so my warning 
> > is not expected to be shown.
> The "use" of `a` that Aaron was talking about is actually
> 
> auto lambda = [=] { a.foo(); };
>^ here
> 
> where the moved-from object is captured-by-copy into the lambda. Making a 
> copy of a moved-from object should warn. (Your patch shouldn't have affected 
> this AFAICT; I think he's just asking for a test to verify+document the 
> currently expected behavior.)
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp#L342

Seems these tests have already been added.


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

https://reviews.llvm.org/D74692



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

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

In D74692#1911150 , @Quuxplusone wrote:

> Anyway, I still don't see the point of this patch. It seems like you're just 
> duplicating the work of `performance-move-const-arg`. People who want to be 
> shown notes about moves-of-const-args should just enable that check instead.


Well, I understand your point. But moving a const is NOP so `bugprone-*` should 
not be warned at all?


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

https://reviews.llvm.org/D74692



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