[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

attributes after an if like:

if (...) [[likely]]

are now applied on the if instead of the following statement.

i added the likely attribute in the necessary places Attr.td, AttrDocs.td.

i added a diagnostics kind for C++2a and for likely not used on an if statement.


Repository:
  rC Clang

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp

Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,22 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema , Stmt *St, const ParsedAttr ,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && A.isCXX11Attribute() &&
+  !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema , Stmt *St, const ParsedAttr ,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +352,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,12 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3858,6 +3858,7 @@
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
+  case ParsedAttr::AT_Likely:
 return true;
   case ParsedAttr::AT_WarnUnusedResult:
 return !ScopeName && AttrName->getName().equals("nodiscard");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
   "use of the %0 attribute is a C++14 extension">, InGroup;
 def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup;
+def ext_cxx2a_attr : Extension<
+  "use of the %0 attribute is a C++2a extension">, InGroup;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,9 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1492,6 +1492,26 @@
   }];
 }
 
+def LikelyDocs : Documentation {
+  let Category = DocCatStmt;
+  let Heading = "likely";
+  let Content = [{
+The ``likely`` (or ``clang::likely``) attribute is used to annotate that a condition is likely to be true
+this is supposed to be used as a hint by the optimizer (not yet implemented)
+
+Here is an example:
+
+.. 

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: JonasToth, aaron.ballman, baloghadamsoftware.
lebedev.ri added projects: clang-tools-extra, OpenMP.
Herald added subscribers: jdoerfert, guansong, rnkovacs, xazax.hun, mgorny.
Herald added a project: clang.

Finally, we are here!

Analyzes OpenMP Structured Blocks and checks that no exception escapes
out of the Structured Block it was thrown in.

As per the OpenMP specification, structured block is an executable statement,
possibly compound, with a single entry at the top and a single exit at the
bottom. Which means, ``throw`` may not be used to to 'exit' out of the
structured block. If an exception is not caught in the same structured block
it was thrown in, the behaviour is undefined / implementation defined,
the program will likely terminate.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59466

Files:
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/ExceptionEscapeCheck.cpp
  clang-tidy/openmp/ExceptionEscapeCheck.h
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tidy/utils/ExceptionAnalyzer.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/openmp-exception-escape.rst
  test/clang-tidy/bugprone-exception-escape-openmp.cpp
  test/clang-tidy/openmp-exception-escape.cpp

Index: test/clang-tidy/openmp-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/openmp-exception-escape.cpp
@@ -0,0 +1,126 @@
+// RUN: %check_clang_tidy %s openmp-exception-escape %t -- -extra-arg=-fopenmp -extra-arg=-fexceptions -config="{CheckOptions: [{key: openmp-exception-escape.IgnoredExceptions, value: 'ignored'}]}" --
+
+int thrower() {
+  throw 42;
+}
+
+class ignored {};
+namespace std {
+class bad_alloc {};
+} // namespace std
+
+void parallel() {
+#pragma omp parallel
+  thrower();
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: An exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+}
+
+void ignore() {
+#pragma omp parallel
+  throw ignored();
+}
+
+void standalone_directive() {
+#pragma omp taskwait
+  throw ignored(); // not structured block
+}
+
+void ignore_alloc() {
+#pragma omp parallel
+  throw std::bad_alloc();
+}
+
+void parallel_caught() {
+#pragma omp parallel
+  {
+try {
+  thrower();
+} catch (...) {
+}
+  }
+}
+
+void for_header(const int a) {
+  // Only the body of the loop counts.
+#pragma omp for
+  for (int i = 0; i < thrower(); i++)
+;
+}
+
+void forloop(const int a) {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+  // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+}
+
+void parallel_forloop(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++)
+  thrower();
+thrower();
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: An exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_forloop_caught(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++) {
+  try {
+thrower();
+  } catch (...) {
+  }
+}
+thrower();
+// CHECK-MESSAGES: :[[@LINE-10]]:9: warning: An exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+  }
+}
+
+void parallel_caught_forloop(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++)
+  thrower();
+try {
+  thrower();
+} catch (...) {
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_outercaught_forloop(const int a) {
+#pragma omp parallel
+  {
+try {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+  thrower();
+} catch (...) {
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_outercaught_forloop_caught(const int a) {
+#pragma omp parallel
+  {
+try {
+#pragma omp for
+  for (int i = 0; i < a; i++) {
+try {
+  thrower();
+} catch (...) {
+}
+  }
+} catch (...) {
+}
+  }
+}
Index: test/clang-tidy/bugprone-exception-escape-openmp.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape-openmp.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-fopenmp -extra-arg=-fexceptions --
+
+int thrower() {
+  throw 42;
+}
+
+void ok_parallel() {
+#pragma omp parallel
+  

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190997.
Szelethus added a comment.

Add a test case for then checker plugin.


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

https://reviews.llvm.org/D57858

Files:
  include/clang/Driver/CC1Options.td
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/analyzer-checker-option-help.c
  test/Analysis/checker-plugins.c

Index: test/Analysis/checker-plugins.c
===
--- test/Analysis/checker-plugins.c
+++ test/Analysis/checker-plugins.c
@@ -104,3 +104,12 @@
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CORRECTED-BOOL-VALUE
 
 // CHECK-CORRECTED-BOOL-VALUE: example.MyChecker:ExampleOption = false
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-checker-option-help \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-HELP
+
+// CHECK-CHECKER-OPTION-HELP: example.MyChecker:ExampleOption  (bool) This is an
+// CHECK-CHECKER-OPTION-HELP-SAME: example checker opt. (default: false)
Index: test/Analysis/analyzer-checker-option-help.c
===
--- /dev/null
+++ test/Analysis/analyzer-checker-option-help.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -analyzer-checker-option-help 2>&1 | FileCheck %s
+
+// CHECK: OVERVIEW: Clang Static Analyzer Checker and Package Option List
+//
+// CHECK: USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config 
+//
+// CHECK:clang -cc1 [CLANG_OPTIONS] -analyzer-config OPTION1=VALUE,
+// CHECK-SAME: -analyzer-config OPTION2=VALUE, ...
+//
+// CHECK:clang [CLANG_OPTIONS] -Xclang -analyzer-config
+// CHECK-SAME: -Xclang
+//
+// CHECK:clang [CLANG_OPTIONS] -Xclang -analyzer-config
+// CHECK-SAME: -Xclang OPTION1=VALUE, -Xclang -analyzer-config
+// CHECK-SAME: -Xclang OPTION2=VALUE, ...
+//
+// CHECK: OPTIONS:
+//
+// CHECK:   alpha.clone.CloneChecker:MinimumCloneComplexity  
+// CHECK-SAME:   (int) Ensures that every clone has at least
+// CHECK:the given complexity. Complexity is here
+// CHECK:defined as the total amount of children
+// CHECK:of a statement. This constraint assumes
+// CHECK:the first statement in the group is representative
+// CHECK:for all other statements in the group in
+// CHECK:terms of complexity. (default: 50)
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
+#include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -538,3 +539,75 @@
   for (const auto *i : EnabledCheckers)
 Out << i->FullName << '\n';
 }
+
+// Some global constants to help with printing.
+constexpr size_t MinLineWidth = 90;
+constexpr size_t PadForOpt = 2;
+constexpr size_t OptionWidth = 50;
+constexpr size_t PadForDesc = PadForOpt + OptionWidth;
+static_assert(MinLineWidth > PadForDesc, "MinLineWidth must be greater!");
+
+static void printCmdLineOption(llvm::formatted_raw_ostream ,
+   StringRef CheckerOrPackageFullName,
+   const CheckerRegistry::CmdLineOption ) {
+  FOut.PadToColumn(PadForOpt) << CheckerOrPackageFullName << ':'
+  << Option.OptionName;
+
+  // If the buffer's length is greater then PadForDesc, print a newline.
+  if (FOut.getColumn() > PadForDesc)
+FOut << '\n';
+
+  FOut.PadToColumn(PadForDesc) << "(" << Option.OptionType << ") ";
+
+  for (char C : Option.Description) {
+if (FOut.getColumn() > MinLineWidth && C == ' ') {
+  FOut << '\n';
+  FOut.PadToColumn(PadForDesc);
+  continue;
+}
+FOut << C;
+  }
+
+  if (!Option.Description.empty())
+FOut << ' ';
+  if (FOut.getColumn() > MinLineWidth) {
+FOut << '\n';
+FOut.PadToColumn(PadForDesc);
+  }
+  FOut << "(default: "
+   << (Option.DefaultValStr.empty() ? "\"\"" : Option.DefaultValStr)
+   << ")\n\n";
+}
+
+void CheckerRegistry::printCheckerOptionList(raw_ostream ) const {
+  Out << "OVERVIEW: Clang Static Analyzer Checker and Package Option List\n\n";
+  Out << "USAGE: clang -cc1 [CLANG_OPTIONS] -analyzer-config "
+ 

[PATCH] D59195: [analyzer] Remove the default value arg from getChecker*Option

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190996.
Szelethus added a comment.

Remove the default argument from the plugin, also add a test case for it.


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

https://reviews.llvm.org/D59195

Files:
  examples/StaticAnalyzer/OptionHandlingPlugin/CheckerOptionHandling.cpp
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/checker-plugins.c
  test/Analysis/invalid-checker-option.c
  unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp

Index: unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
===
--- unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
+++ unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
@@ -51,25 +51,24 @@
   // CheckerTwo one has Option specified as true. It should read true regardless
   // of search mode.
   CheckerOneMock CheckerOne;
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", false));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option"));
   // The package option is overridden with a checker option.
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", false,
-   true));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", true));
   // The Outer package option is overridden by the Inner package option. No
   // package option is specified.
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option2", false,
-   true));
-  // No package option is specified and search in packages is turned off. The
-  // default value should be returned.
-  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option2", false));
   EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option2", true));
+  // No package option is specified and search in packages is turned off. We
+  // should assert here, but we can't test that.
+  //Opts.getCheckerBooleanOption(, "Option2");
+  //Opts.getCheckerBooleanOption(, "Option2");
 
-  // Checker true has no option specified. It should get the default value when
-  // search in parents turned off and false when search in parents turned on.
+  // Checker true has no option specified. It should get false when search in
+  // parents turned on.
   CheckerTwoMock CheckerTwo;
-  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option", false));
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", true));
-  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option", true, true));
+  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option", true));
+  // In any other case, we should assert, that we cannot test unfortunately.
+  //Opts.getCheckerBooleanOption(, "Option");
+  //Opts.getCheckerBooleanOption(, "Option");
 }
 
 TEST(StaticAnalyzerOptions, StringOptions) {
@@ -84,16 +83,14 @@
 
   CheckerOneMock CheckerOne;
   EXPECT_TRUE("StringValue" ==
-Opts.getCheckerStringOption(, "Option", "DefaultValue"));
-  EXPECT_TRUE("DefaultValue" ==
-   Opts.getCheckerStringOption(, "Option2", "DefaultValue"));
+Opts.getCheckerStringOption(, "Option"));
 }
 
 TEST(StaticAnalyzerOptions, SubCheckerOptions) {
   AnalyzerOptions Opts;
   Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
   EXPECT_TRUE("StringValue" == Opts.getCheckerStringOption(
-"Outer.Inner.CheckerOne", "Option", "DefaultValue"));
+"Outer.Inner.CheckerOne", "Option"));
 }
 
 } // end namespace ento
Index: test/Analysis/invalid-checker-option.c
===
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -15,6 +15,27 @@
 // CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
 
 
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ConfigDumper \
+// RUN:   -analyzer-checker=debug.AnalysisOrder \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=yesplease \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CORRECTED-BOOL-VALUE
+
+// CHECK-CORRECTED-BOOL-VALUE: debug.AnalysisOrder:* = false
+//
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ConfigDumper \
+// RUN:   

[PATCH] D59195: [analyzer] Remove the default value arg from getChecker*Option

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190993.
Szelethus added a comment.

Rebase.


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

https://reviews.llvm.org/D59195

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/invalid-checker-option.c
  unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp

Index: unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
===
--- unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
+++ unittests/StaticAnalyzer/AnalyzerOptionsTest.cpp
@@ -51,25 +51,24 @@
   // CheckerTwo one has Option specified as true. It should read true regardless
   // of search mode.
   CheckerOneMock CheckerOne;
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", false));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option"));
   // The package option is overridden with a checker option.
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", false,
-   true));
+  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", true));
   // The Outer package option is overridden by the Inner package option. No
   // package option is specified.
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option2", false,
-   true));
-  // No package option is specified and search in packages is turned off. The
-  // default value should be returned.
-  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option2", false));
   EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option2", true));
+  // No package option is specified and search in packages is turned off. We
+  // should assert here, but we can't test that.
+  //Opts.getCheckerBooleanOption(, "Option2");
+  //Opts.getCheckerBooleanOption(, "Option2");
 
-  // Checker true has no option specified. It should get the default value when
-  // search in parents turned off and false when search in parents turned on.
+  // Checker true has no option specified. It should get false when search in
+  // parents turned on.
   CheckerTwoMock CheckerTwo;
-  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option", false));
-  EXPECT_TRUE(Opts.getCheckerBooleanOption(, "Option", true));
-  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option", true, true));
+  EXPECT_FALSE(Opts.getCheckerBooleanOption(, "Option", true));
+  // In any other case, we should assert, that we cannot test unfortunately.
+  //Opts.getCheckerBooleanOption(, "Option");
+  //Opts.getCheckerBooleanOption(, "Option");
 }
 
 TEST(StaticAnalyzerOptions, StringOptions) {
@@ -84,16 +83,14 @@
 
   CheckerOneMock CheckerOne;
   EXPECT_TRUE("StringValue" ==
-Opts.getCheckerStringOption(, "Option", "DefaultValue"));
-  EXPECT_TRUE("DefaultValue" ==
-   Opts.getCheckerStringOption(, "Option2", "DefaultValue"));
+Opts.getCheckerStringOption(, "Option"));
 }
 
 TEST(StaticAnalyzerOptions, SubCheckerOptions) {
   AnalyzerOptions Opts;
   Opts.Config["Outer.Inner.CheckerOne:Option"] = "StringValue";
   EXPECT_TRUE("StringValue" == Opts.getCheckerStringOption(
-"Outer.Inner.CheckerOne", "Option", "DefaultValue"));
+"Outer.Inner.CheckerOne", "Option"));
 }
 
 } // end namespace ento
Index: test/Analysis/invalid-checker-option.c
===
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -15,6 +15,27 @@
 // CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
 
 
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ConfigDumper \
+// RUN:   -analyzer-checker=debug.AnalysisOrder \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=yesplease \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CORRECTED-BOOL-VALUE
+
+// CHECK-CORRECTED-BOOL-VALUE: debug.AnalysisOrder:* = false
+//
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ConfigDumper \
+// RUN:   -analyzer-checker=optin.performance.Padding \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN:   

[PATCH] D57860: [analyzer] Validate checker option names and values

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190992.
Szelethus added a comment.

Add a test case for checker plugins.


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

https://reviews.llvm.org/D57860

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/checker-plugins.c
  test/Analysis/invalid-checker-option.c

Index: test/Analysis/invalid-checker-option.c
===
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -14,6 +14,63 @@
 // CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages
 // CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
 
+
+// Every other error should be avoidable in compatiblity mode.
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config debug.AnalysisOrder:Everything=false \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'debug.AnalysisOrder'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Everything'
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:Everything=false
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=nothankyou \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'debug.AnalysisOrder:*', that expects a
+// CHECK-INVALID-BOOL-VALUE-SAME: boolean value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=nothankyou
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-INT-VALUE
+
+// CHECK-INVALID-INT-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-INT-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-INVALID-INT-VALUE-SAME: expects an integer value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=sure \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PACKAGE-VALUE
+
+// CHECK-PACKAGE-VALUE: (frontend): invalid input for checker option
+// CHECK-PACKAGE-VALUE-SAME: 'nullability:NoDiagnoseCallsToSystemHeaders', that
+// CHECK-PACKAGE-VALUE-SAME: expects a boolean value
+
 // expected-no-diagnostics
 
 int main() {}
Index: test/Analysis/checker-plugins.c
===
--- test/Analysis/checker-plugins.c
+++ test/Analysis/checker-plugins.c
@@ -62,3 +62,35 @@
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-TRUE
 
 // CHECK-CHECKER-OPTION-TRUE: example.MyChecker:ExampleOption = true
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config example.MyChecker:Example=true \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'example.MyChecker'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Example'
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config example.MyChecker:Example=true
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config example.MyChecker:ExampleOption=example \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'example.MyChecker:ExampleOption', that
+// CHECK-INVALID-BOOL-VALUE-SAME: expects a boolean value
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190991.
Szelethus added a comment.

Add a test case for checker plugins.


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

https://reviews.llvm.org/D57922

Files:
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/checker-plugins.c

Index: test/Analysis/checker-plugins.c
===
--- test/Analysis/checker-plugins.c
+++ test/Analysis/checker-plugins.c
@@ -45,3 +45,20 @@
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-OUTPUT-TRUE
 
 // CHECK-CHECKER-OPTION-OUTPUT-TRUE: Example option is set to true
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-checker=debug.ConfigDumper \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION
+
+// CHECK-CHECKER-OPTION: example.MyChecker:ExampleOption = false
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-checker=debug.ConfigDumper \
+// RUN:   -analyzer-config example.MyChecker:ExampleOption=true \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-TRUE
+
+// CHECK-CHECKER-OPTION-TRUE: example.MyChecker:ExampleOption = true
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -3,6 +3,16 @@
 
 // CHECK: [config]
 // CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
+// CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
+// CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreGuardedFields = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:NotesAsWarnings = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:Pedantic = false
+// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
+// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
@@ -18,9 +28,26 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
+// CHECK-NEXT: debug.AnalysisOrder:* = false
+// CHECK-NEXT: debug.AnalysisOrder:Bind = false
+// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
+// CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
+// CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PostCall = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreCall = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:RegionChanges = false
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
@@ -40,7 +67,15 @@
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: model-path = ""
 // CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: nullability:NoDiagnoseCallsToSystemHeaders = false
 // CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
+// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
+// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
+// CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
+// CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true
+// CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false
+// CHECK-NEXT: osx.cocoa.RetainCount:leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: report-in-main-source-file = false
@@ -49,7 +84,8 @@
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks 

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356328: CodeGen: Preserve packed attribute in 
constStructWithPadding. (authored by pcc, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59446?vs=190942=190990#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59446

Files:
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/test/CodeGenCXX/auto-var-init.cpp


Index: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
@@ -85,6 +85,8 @@
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private 
unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr 
constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private 
unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } 
{ i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { 
i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] c"\AA\AA\AA", i32 13371338 } }, align 4
 // ZERO: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] zeroinitializer, i32 13371338 } }, align 4
@@ -778,6 +780,10 @@
 // CHECK-NEXT:  call void 
@llvm.memcpy{{.*}}({{.*}}@__const.test_paddedpackedarray_custom.custom
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
+TEST_UNINIT(unpackedinpacked, unpackedinpacked);
+// CHECK-LABEL: @test_unpackedinpacked_uninit()
+// PATTERN-O0:  call void @llvm.memcpy{{.*}}, i64 9, i1 false)
+
 TEST_UNINIT(paddednested, paddednested);
 // CHECK-LABEL: @test_paddednested_uninit()
 // CHECK:   %uninit = alloca %struct.paddednested, align
Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -1113,7 +1113,7 @@
   }
   if (NestedIntact && Values.size() == STy->getNumElements())
 return constant;
-  return llvm::ConstantStruct::getAnon(Values);
+  return llvm::ConstantStruct::getAnon(Values, STy->isPacked());
 }
 
 /// Replace all padding bytes in a given constant with either a pattern byte or


Index: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
@@ -85,6 +85,8 @@
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] [%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] [%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 

r356328 - CodeGen: Preserve packed attribute in constStructWithPadding.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Sat Mar 16 12:25:39 2019
New Revision: 356328

URL: http://llvm.org/viewvc/llvm-project?rev=356328=rev
Log:
CodeGen: Preserve packed attribute in constStructWithPadding.

Otherwise the object may have an incorrect size due to tail padding.

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

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGenCXX/auto-var-init.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=356328=356327=356328=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Sat Mar 16 12:25:39 2019
@@ -1113,7 +1113,7 @@ static llvm::Constant *constStructWithPa
   }
   if (NestedIntact && Values.size() == STy->getNumElements())
 return constant;
-  return llvm::ConstantStruct::getAnon(Values);
+  return llvm::ConstantStruct::getAnon(Values, STy->isPacked());
 }
 
 /// Replace all padding bytes in a given constant with either a pattern byte or

Modified: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/auto-var-init.cpp?rev=356328=356327=356328=diff
==
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp Sat Mar 16 12:25:39 2019
@@ -85,6 +85,8 @@ struct paddedpacked { char c; int i; } _
 // PATTERN: @__const.test_paddedpackedarray_custom.custom = private 
unnamed_addr constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 // ZERO: @__const.test_paddedpackedarray_custom.custom = private unnamed_addr 
constant %struct.paddedpackedarray { [2 x %struct.paddedpacked] 
[%struct.paddedpacked <{ i8 42, i32 13371337 }>, %struct.paddedpacked <{ i8 43, 
i32 13371338 }>] }, align 1
 struct paddedpackedarray { struct paddedpacked p[2]; };
+// PATTERN-O0: @__const.test_unpackedinpacked_uninit.uninit = private 
unnamed_addr constant <{ { i8, [3 x i8], i32 }, i8 }> <{ { i8, [3 x i8], i32 } 
{ i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, i8 -86 }>, align 1
+struct unpackedinpacked { padded a; char b; } __attribute__((packed));
 // PATTERN-O0: @__const.test_paddednested_uninit.uninit = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 }, { i8, [3 x i8], i32 } { 
i8 -86, [3 x i8] c"\AA\AA\AA", i32 -1431655766 } }, align 4
 // PATTERN: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] c"\AA\AA\AA", i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] c"\AA\AA\AA", i32 13371338 } }, align 4
 // ZERO: @__const.test_paddednested_custom.custom = private unnamed_addr 
constant { { i8, [3 x i8], i32 }, { i8, [3 x i8], i32 } } { { i8, [3 x i8], i32 
} { i8 42, [3 x i8] zeroinitializer, i32 13371337 }, { i8, [3 x i8], i32 } { i8 
43, [3 x i8] zeroinitializer, i32 13371338 } }, align 4
@@ -778,6 +780,10 @@ TEST_CUSTOM(paddedpackedarray, paddedpac
 // CHECK-NEXT:  call void 
@llvm.memcpy{{.*}}({{.*}}@__const.test_paddedpackedarray_custom.custom
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
+TEST_UNINIT(unpackedinpacked, unpackedinpacked);
+// CHECK-LABEL: @test_unpackedinpacked_uninit()
+// PATTERN-O0:  call void @llvm.memcpy{{.*}}, i64 9, i1 false)
+
 TEST_UNINIT(paddednested, paddednested);
 // CHECK-LABEL: @test_paddednested_uninit()
 // CHECK:   %uninit = alloca %struct.paddednested, align


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

[PATCH] D59465: [analyzer] Add example plugin for checker option handling

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity, mgorny.

Repository:
  rC Clang

https://reviews.llvm.org/D59465

Files:
  examples/StaticAnalyzer/CMakeLists.txt
  examples/StaticAnalyzer/OptionHandlingPlugin/CMakeLists.txt
  examples/StaticAnalyzer/OptionHandlingPlugin/CheckerOptionHandling.cpp
  
examples/StaticAnalyzer/OptionHandlingPlugin/OptionHandlingAnalyzerPlugin.exports
  test/Analysis/checker-plugins.c

Index: test/Analysis/checker-plugins.c
===
--- test/Analysis/checker-plugins.c
+++ test/Analysis/checker-plugins.c
@@ -30,3 +30,18 @@
 
 // CHECK-IMPLICITLY-DISABLED-NOT: example.Dependency
 // CHECK-IMPLICITLY-DISABLED-NOT: example.DependendentChecker
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-OUTPUT
+
+// CHECK-CHECKER-OPTION-OUTPUT: Example option is set to false
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/OptionHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.MyChecker \
+// RUN:   -analyzer-config example.MyChecker:ExampleOption=true \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-CHECKER-OPTION-OUTPUT-TRUE
+
+// CHECK-CHECKER-OPTION-OUTPUT-TRUE: Example option is set to true
Index: examples/StaticAnalyzer/OptionHandlingPlugin/OptionHandlingAnalyzerPlugin.exports
===
--- /dev/null
+++ examples/StaticAnalyzer/OptionHandlingPlugin/OptionHandlingAnalyzerPlugin.exports
@@ -0,0 +1,2 @@
+clang_registerCheckers
+clang_analyzerAPIVersionString
Index: examples/StaticAnalyzer/OptionHandlingPlugin/CheckerOptionHandling.cpp
===
--- /dev/null
+++ examples/StaticAnalyzer/OptionHandlingPlugin/CheckerOptionHandling.cpp
@@ -0,0 +1,42 @@
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+struct MyChecker : public Checker {
+  void checkBeginFunction(CheckerContext ) const {}
+};
+
+void registerMyChecker(CheckerManager ) {
+  MyChecker *Checker = Mgr.registerChecker();
+  llvm::outs() << "Example option is set to "
+   << (Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+   Checker, "ExampleOption", false)
+   ? "true"
+   : "false")
+   << '\n';
+}
+
+bool shouldRegisterMyChecker(const LangOptions ) { return true; }
+
+} // end anonymous namespace
+
+// Register plugin!
+extern "C" void clang_registerCheckers(CheckerRegistry ) {
+  registry.addChecker(registerMyChecker, shouldRegisterMyChecker,
+  "example.MyChecker", "Example Description",
+  "example.mychecker.documentation.nonexistent.html");
+
+  registry.addCheckerOption(/*OptionType*/ "bool",
+/*CheckerFullName*/ "example.MyChecker",
+/*OptionName*/ "ExampleOption",
+/*DefaultValStr*/ "false",
+/*Description*/ "This is an example checker opt.");
+}
+
+extern "C" const char clang_analyzerAPIVersionString[] =
+CLANG_ANALYZER_API_VERSION_STRING;
Index: examples/StaticAnalyzer/OptionHandlingPlugin/CMakeLists.txt
===
--- /dev/null
+++ examples/StaticAnalyzer/OptionHandlingPlugin/CMakeLists.txt
@@ -0,0 +1,11 @@
+set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/OptionHandlingAnalyzerPlugin.exports)
+add_llvm_library(OptionHandlingAnalyzerPlugin MODULE CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  target_link_libraries(OptionHandlingAnalyzerPlugin PRIVATE
+clangAnalysis
+clangAST
+clangStaticAnalyzerCore
+LLVMSupport
+)
+endif()
Index: examples/StaticAnalyzer/CMakeLists.txt
===
--- examples/StaticAnalyzer/CMakeLists.txt
+++ examples/StaticAnalyzer/CMakeLists.txt
@@ -1,2 +1,3 @@
 add_subdirectory(SampleAnalyzerPlugin)
 add_subdirectory(CheckerDependencyHandlingPlugin)
+add_subdirectory(OptionHandlingPlugin)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59464: [analyzer] Add an example plugin for checker dependency handling

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity, mgorny.

- Move `examples/analyzer-plugin` to 
`analyzer/StaticAnalyzer/SampleAnalyzerPlugin`
- Create a new example plugin for handling checker dependencies

Since git will blame will be messed up anyways, I also clang-formatted the 
original plugin.


Repository:
  rC Clang

https://reviews.llvm.org/D59464

Files:
  examples/CMakeLists.txt
  examples/StaticAnalyzer/CMakeLists.txt
  examples/StaticAnalyzer/CheckerDependencyHandlingPlugin/CMakeLists.txt
  
examples/StaticAnalyzer/CheckerDependencyHandlingPlugin/CheckerDependencyHandling.cpp
  
examples/StaticAnalyzer/CheckerDependencyHandlingPlugin/CheckerDependencyHandlingAnalyzerPlugin.exports
  examples/StaticAnalyzer/SampleAnalyzerPlugin/CMakeLists.txt
  examples/StaticAnalyzer/SampleAnalyzerPlugin/MainCallChecker.cpp
  examples/StaticAnalyzer/SampleAnalyzerPlugin/SampleAnalyzerPlugin.exports
  examples/analyzer-plugin/CMakeLists.txt
  examples/analyzer-plugin/MainCallChecker.cpp
  examples/analyzer-plugin/SampleAnalyzerPlugin.exports
  test/Analysis/checker-plugins.c

Index: test/Analysis/checker-plugins.c
===
--- test/Analysis/checker-plugins.c
+++ test/Analysis/checker-plugins.c
@@ -1,4 +1,7 @@
-// RUN: %clang_analyze_cc1 -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext -analyzer-checker='example.MainCallChecker' -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext \
+// RUN:   -analyzer-checker='example.MainCallChecker'
+
 // REQUIRES: plugins, examples
 
 // Test that the MainCallChecker example analyzer plugin loads and runs.
@@ -8,3 +11,22 @@
 void caller() {
   main(); // expected-warning {{call to main}}
 }
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerDependencyHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.DependendentChecker \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED
+
+// CHECK-IMPLICITLY-ENABLED: example.Dependency
+// CHECK-IMPLICITLY-ENABLED: example.DependendentChecker
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -load %llvmshlibdir/CheckerDependencyHandlingAnalyzerPlugin%pluginext\
+// RUN:   -analyzer-checker=example.DependendentChecker \
+// RUN:   -analyzer-disable-checker=example.Dependency \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED
+
+// CHECK-IMPLICITLY-DISABLED-NOT: example.Dependency
+// CHECK-IMPLICITLY-DISABLED-NOT: example.DependendentChecker
Index: examples/StaticAnalyzer/SampleAnalyzerPlugin/SampleAnalyzerPlugin.exports
===
--- /dev/null
+++ examples/StaticAnalyzer/SampleAnalyzerPlugin/SampleAnalyzerPlugin.exports
@@ -0,0 +1,2 @@
+clang_registerCheckers
+clang_analyzerAPIVersionString
Index: examples/StaticAnalyzer/SampleAnalyzerPlugin/MainCallChecker.cpp
===
--- examples/StaticAnalyzer/SampleAnalyzerPlugin/MainCallChecker.cpp
+++ examples/StaticAnalyzer/SampleAnalyzerPlugin/MainCallChecker.cpp
@@ -1,13 +1,13 @@
-#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
-#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
-class MainCallChecker : public Checker < check::PreStmt > {
+class MainCallChecker : public Checker> {
   mutable std::unique_ptr BT;
 
 public:
@@ -15,7 +15,8 @@
 };
 } // end anonymous namespace
 
-void MainCallChecker::checkPreStmt(const CallExpr *CE, CheckerContext ) const {
+void MainCallChecker::checkPreStmt(const CallExpr *CE,
+   CheckerContext ) const {
   const Expr *Callee = CE->getCallee();
   const FunctionDecl *FD = C.getSVal(Callee).getAsFunctionDecl();
 
@@ -24,7 +25,7 @@
 
   // Get the name of the callee.
   IdentifierInfo *II = FD->getIdentifier();
-  if (!II)   // if no identifier, not a simple C function
+  if (!II) // if no identifier, not a simple C function
 return;
 
   if (II->isStr("main")) {
@@ -43,12 +44,11 @@
 }
 
 // Register plugin!
-extern "C"
-void clang_registerCheckers (CheckerRegistry ) {
+extern "C" void clang_registerCheckers(CheckerRegistry ) {
   registry.addChecker(
   "example.MainCallChecker", "Disallows calls to functions called main",
   "");
 }
 
-extern "C"
-const char clang_analyzerAPIVersionString[] = 

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:6421
+///
+/// Prerequisite: the executable directive must not be standalone directive.
+///

aaron.ballman wrote:
> What happens if this prereq is not met? Does the matcher return false, or 
> does it do something worse (like assert)?
From https://reviews.llvm.org/D59214#change-5GOe7CiMwWxl
```
const Stmt *OMPExecutableDirective::getStructuredBlock() const {
  assert(!isStandaloneDirective() &&
 "Standalone Executable Directives don't have Structured Blocks.");
```
It really doesn't make sense to return false/nullptr,
it would be like trying to find ForLoop in VarDecl, they are just really 
distinct types.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59463



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

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.

aaron.ballman wrote:
> These markings are a bit strange, can you explain them to me?
It is weird, but i think this is the right solution.
See `isAllowedToContainClause()` narrower.
This `asOMPClauseKind()` allows to pass a whole matcher, and then distill the 
inner output node type.
I.e. now i can spell 
`ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()))`
(and the `ompDefaultClause()` won't actually be used for matching!), instead of 
doing something like e.g.
`ompExecutableDirective(isAllowedToContainClause(OMPC_default))`
which looks horrible, and will likely not work well with `clang-query`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112



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

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:6421
+///
+/// Prerequisite: the executable directive must not be standalone directive.
+///

What happens if this prereq is not met? Does the matcher return false, or does 
it do something worse (like assert)?



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:513-515
+  REGISTER_MATCHER(isStandaloneDirective);
+  REGISTER_MATCHER(isOMPStructuredBlock);
+  REGISTER_MATCHER(hasStructuredBlock);

Alphabetical order, please.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59463



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

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/ASTTypeTraits.h:81
 
+  /// \{
+  /// Return the AST node kind of this ASTNodeKind.

These markings are a bit strange, can you explain them to me?



Comment at: include/clang/ASTMatchers/ASTMatchers.h:6389
 
+/// Given OpenMP directive, matches the first clause (out of all specified),
+/// that matches InnerMatcher.

Given OpenMP -> Given an OpenMP



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:509-512
+  REGISTER_MATCHER(hasClause);
+  REGISTER_MATCHER(ompDefaultClause);
+  REGISTER_MATCHER(isNoneKind);
+  REGISTER_MATCHER(isAllowedToContainClause);

Please keep these alphabetically sorted.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112



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

[PATCH] D59453: [ASTMatchers][OpenMP] Add base ompExecutableDirective() matcher.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a nit.




Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:508
   REGISTER_MATCHER(withInitializer);
+  REGISTER_MATCHER(ompExecutableDirective);
 }

Please keep this list alphabetically sorted.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59453



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431935 , @delcypher wrote:

> Would one of you be able to file a bug against Z3 to fix this? I am no longer 
> in a position to contribute to Z3 so I can't do this.


I've opened https://github.com/Z3Prover/z3/issues/2184 .

In D54978#1431936 , @delcypher wrote:

> This output means you built Z3 from source that was not in a git repository. 
> In this case the header file should look the same for both Z3's CMake and 
> Python build systems.


That's strange, I have been building from a git repository.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> 2. Instead of parsing `Z3_FULL_VERSION`, we can parse `Z3_MAJOR_VERSION`, 
> `Z3_MINOR_VERSION` and `Z3_BUILD_NUMBER` which are also available in the same 
> header.


Since the differences in version string depending on the build system are 
intended behavior, it seems (2) would be preferable?


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

https://reviews.llvm.org/D54978



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

[PATCH] D59463: [ASTMatchers][OpenMP] OpenMP Structured-block-related matchers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: gribozavr, aaron.ballman, JonasToth, 
george.karpenkov.
lebedev.ri added projects: clang, OpenMP.
Herald added subscribers: jdoerfert, guansong.

Exposes the interface being added in D59214  
for ASTMatchers.


Repository:
  rC Clang

https://reviews.llvm.org/D59463

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2274,6 +2274,57 @@
 notMatches("int main2() {}", functionDecl(isMain(;
 }
 
+TEST(OMPExecutableDirective, isStandaloneDirective) {
+  auto Matcher = ompExecutableDirective(isStandaloneDirective());
+
+  const std::string Source0 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp taskyield
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source1, Matcher));
+}
+
+TEST(Stmt, isOMPStructuredBlock) {
+  const std::string Source0 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(
+  matchesWithOpenMP(Source0, stmt(nullStmt(), isOMPStructuredBlock(;
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+{;}
+})";
+  EXPECT_TRUE(
+  notMatchesWithOpenMP(Source1, stmt(nullStmt(), isOMPStructuredBlock(;
+  EXPECT_TRUE(
+  matchesWithOpenMP(Source1, stmt(compoundStmt(), isOMPStructuredBlock(;
+}
+
+TEST(OMPExecutableDirective, hasStructuredBlock) {
+  const std::string Source0 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(
+  Source0, ompExecutableDirective(hasStructuredBlock(nullStmt();
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+{;}
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(
+  Source1, ompExecutableDirective(hasStructuredBlock(nullStmt();
+  EXPECT_TRUE(matchesWithOpenMP(
+  Source1, ompExecutableDirective(hasStructuredBlock(compoundStmt();
+}
+
 TEST(OMPExecutableDirective, hasClause) {
   auto Matcher = ompExecutableDirective(hasClause(anything()));
 
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -510,6 +510,9 @@
   REGISTER_MATCHER(ompDefaultClause);
   REGISTER_MATCHER(isNoneKind);
   REGISTER_MATCHER(isAllowedToContainClause);
+  REGISTER_MATCHER(isStandaloneDirective);
+  REGISTER_MATCHER(isOMPStructuredBlock);
+  REGISTER_MATCHER(hasStructuredBlock);
 }
 
 RegistryMaps::~RegistryMaps() = default;
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -6386,6 +6386,58 @@
 extern const internal::VariadicDynCastAllOfMatcher
 ompExecutableDirective;
 
+/// Given OpenMP directive, matches if it is an OpenMP standalone directive,
+/// i.e. an executable directive with no structured block.
+///
+/// Example:
+///
+/// Given ``ompExecutableDirective(isStandaloneDirective()))``:
+///
+/// \code
+///   #pragma omp parallel   // <- no match, not standalone
+///   {}
+///   #pragma omp taskyield  // <- match, is standalone
+/// \endcode
+AST_MATCHER(OMPExecutableDirective, isStandaloneDirective) {
+  return Node.isStandaloneDirective();
+}
+
+/// Will match the Stmt AST node that is marked as being the structured block
+/// of an OpenMP executable directive.
+///
+/// Example:
+///
+/// Given ``stmt(isOMPStructuredBlock()))``:
+///
+/// \code
+///#pragma omp parallel
+///; // <- will match `;`
+/// \endcode
+AST_MATCHER(Stmt, isOMPStructuredBlock) { return Node.isOMPStructuredBlock(); }
+
+/// Given OpenMP executable directive, matches if the InnerMatcher matches the
+/// structured block of this OpenMP executable directive.
+///
+/// Prerequisite: the executable directive must not be standalone directive.
+///
+/// Example:
+///
+/// Given ``ompExecutableDirective(hasStructuredBlock(nullStmt()))``:
+///
+/// \code
+///#pragma omp parallel
+///; // <- will match `;`
+/// \endcode
+///
+/// \code
+///#pragma omp parallel
+///{}// <- no match
+/// \endcode
+AST_MATCHER_P(OMPExecutableDirective, hasStructuredBlock,
+  internal::Matcher, InnerMatcher) {
+  return InnerMatcher.matches(*Node.getStructuredBlock(), Finder, Builder);
+}
+
 /// Given OpenMP directive, matches the first clause (out of all specified),
 /// that matches InnerMatcher.
 ///
Index: docs/LibASTMatchersReference.html

[PATCH] D57860: [analyzer] Validate checker option names and values

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190981.
Szelethus added a comment.

Rebase.


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

https://reviews.llvm.org/D57860

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/invalid-checker-option.c

Index: test/Analysis/invalid-checker-option.c
===
--- test/Analysis/invalid-checker-option.c
+++ test/Analysis/invalid-checker-option.c
@@ -14,6 +14,63 @@
 // CHECK-NON-EXISTENT-CHECKER: (frontend): no analyzer checkers or packages
 // CHECK-NON-EXISTENT-CHECKER-SAME: are associated with 'RetainOneTwoThree'
 
+
+// Every other error should be avoidable in compatiblity mode.
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config debug.AnalysisOrder:Everything=false \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-NON-EXISTENT-CHECKER-OPTION
+
+// CHECK-NON-EXISTENT-CHECKER-OPTION: (frontend): checker 'debug.AnalysisOrder'
+// CHECK-NON-EXISTENT-CHECKER-OPTION-SAME: has no option called 'Everything'
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:Everything=false
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=nothankyou \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-BOOL-VALUE
+
+// CHECK-INVALID-BOOL-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-BOOL-VALUE-SAME: 'debug.AnalysisOrder:*', that expects a
+// CHECK-INVALID-BOOL-VALUE-SAME: boolean value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config debug.AnalysisOrder:*=nothankyou
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-INT-VALUE
+
+// CHECK-INVALID-INT-VALUE: (frontend): invalid input for checker option
+// CHECK-INVALID-INT-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-INVALID-INT-VALUE-SAME: expects an integer value
+
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config-compatibility-mode=true \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=surpriseme
+
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=sure \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PACKAGE-VALUE
+
+// CHECK-PACKAGE-VALUE: (frontend): invalid input for checker option
+// CHECK-PACKAGE-VALUE-SAME: 'nullability:NoDiagnoseCallsToSystemHeaders', that
+// CHECK-PACKAGE-VALUE-SAME: expects a boolean value
+
 // expected-no-diagnostics
 
 int main() {}
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -310,18 +310,61 @@
   Dependencies.emplace_back(FullName, Dependency);
 }
 
+/// Insert the checker/package option to AnalyzerOptions' config table, and
+/// validate it, if the user supplied it on the command line.
+static void insertAndValidate(StringRef FullName,
+  const CheckerRegistry::CmdLineOption ,
+  AnalyzerOptions ,
+  DiagnosticsEngine ) {
+
+  std::string FullOption = (FullName + ":" + Option.OptionName).str();
+
+  auto It = AnOpts.Config.insert({FullOption, Option.DefaultValStr});
+
+  // Insertation was successful -- CmdLineOption's constructor will validate
+  // whether values received from plugins or TableGen files are correct.
+  if (It.second)
+return;
+
+  // Insertion failed, the user supplied this package/checker option on the
+  // command line. If the supplied value is invalid, we'll emit an error.
+
+  StringRef SuppliedValue = It.first->getValue();
+
+  if (Option.OptionType == "bool") {
+if (SuppliedValue != "true" && SuppliedValue != "false") {
+  if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
+Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+<< FullOption << "a boolean value";
+  }
+}
+return;
+  }
+
+  if (Option.OptionType == "int") {
+int Tmp;
+bool HasFailed = SuppliedValue.getAsInteger(0, Tmp);
+if (HasFailed) {
+  if (AnOpts.ShouldEmitErrorsOnInvalidConfigValue) {
+Diags.Report(diag::err_analyzer_checker_option_invalid_input)
+<< FullOption << "an integer 

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190979.
Szelethus added a comment.

Rebase.


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

https://reviews.llvm.org/D57922

Files:
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/analyzer-config.c

Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -3,6 +3,16 @@
 
 // CHECK: [config]
 // CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
+// CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
+// CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreGuardedFields = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:NotesAsWarnings = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:Pedantic = false
+// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
+// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
@@ -18,9 +28,26 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
+// CHECK-NEXT: debug.AnalysisOrder:* = false
+// CHECK-NEXT: debug.AnalysisOrder:Bind = false
+// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
+// CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
+// CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PostCall = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreCall = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:RegionChanges = false
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
@@ -40,7 +67,15 @@
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: model-path = ""
 // CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: nullability:NoDiagnoseCallsToSystemHeaders = false
 // CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
+// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
+// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
+// CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
+// CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true
+// CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false
+// CHECK-NEXT: osx.cocoa.RetainCount:leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: report-in-main-source-file = false
@@ -49,7 +84,8 @@
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
+// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 49
+// CHECK-NEXT: num-entries = 85
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -313,12 +313,16 @@
 template 
 static void
 insertOptionToCollection(StringRef FullName, T ,
- const CheckerRegistry::CmdLineOption &) {
+ const CheckerRegistry::CmdLineOption &,
+ AnalyzerOptions ) {
   auto It = binaryFind(Collection, FullName);
   assert(It != Collection.end() &&
  "Failed to find the checker while attempting to add a command line "
  "option to it!");
 
+  AnOpts.Config.insert(
+  {(FullName + ":" + Option.OptionName).str(), Option.DefaultValStr});
+
   It->CmdLineOptions.emplace_back(std::move(Option));
 }
 
@@ -326,14 +330,14 @@
  

[PATCH] D57855: [analyzer] Reimplement checker options

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 190977.
Szelethus edited the summary of this revision.
Szelethus added a comment.

- Moved every non-related change to smaller patches, this should ease **a lot** 
on reviewers.
- Now processing options once all checkers are added to the registry. This is 
important, because I use binary searches to find the checkers and packages that 
need to be modified -- if a plugin however called 
`CheckerRegistry::add*Option`, it would've cause an assertion failure.

In D57855#1392560 , @xazax.hun wrote:

> We have `examples/analyzer-plugin`. I would prefer to add an example option 
> to the example plugin so people do see how to do this when they are 
> registering a checker from a plugin.


Coming in a separate patch later!


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

https://reviews.llvm.org/D57855

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/disable-all-checks.c
  test/Analysis/invalid-checker-option.c
  utils/TableGen/ClangSACheckersEmitter.cpp

Index: utils/TableGen/ClangSACheckersEmitter.cpp
===
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -90,6 +90,26 @@
   .str();
 }
 
+/// Retrieves the type from a CmdOptionTypeEnum typed Record object. Note that
+/// the class itself has to be modified for adding a new option type in
+/// CheckerBase.td.
+static std::string getCheckerOptionType(const Record ) {
+  if (BitsInit *BI = R.getValueAsBitsInit("Type")) {
+switch(getValueFromBitsInit(BI, R)) {
+case 0:
+  return "int";
+case 1:
+  return "string";
+case 2:
+  return "bool";
+}
+  }
+  PrintFatalError(R.getLoc(),
+  "unable to parse command line option type for "
+  + getCheckerFullName());
+  return "";
+}
+
 static void printChecker(llvm::raw_ostream , const Record ) {
 OS << "CHECKER(" << "\"";
 OS.write_escaped(getCheckerFullName()) << "\", ";
@@ -134,6 +154,45 @@
   OS << "#endif // GET_PACKAGES\n"
 "\n";
 
+  // Emit a package option.
+  //
+  // PACKAGE_OPTION(OPTIONTYPE, PACKAGENAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  // This is important for validating user input. Note that
+  // it's a string, rather than an actual type: since we can
+  // load checkers runtime, we can't use template hackery for
+  // sorting this out compile-time.
+  //   - PACKAGENAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config PACKAGENAME:OPTIONNAME=VALUE
+  OS << "\n"
+"#ifdef GET_PACKAGE_OPTIONS\n";
+  for (const Record *Package : packages) {
+
+if (Package->isValueUnset("PackageOptions"))
+  continue;
+
+std::vector PackageOptions = Package
+   ->getValueAsListOfDefs("PackageOptions");
+for (Record *PackageOpt : PackageOptions) {
+  OS << "PACKAGE_OPTION(\"";
+  OS.write_escaped(getCheckerOptionType(*PackageOpt)) << "\", \"";
+  OS.write_escaped(getPackageFullName(Package)) << "\", ";
+  OS << '\"' << getStringValue(*PackageOpt, "CmdFlag") << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*PackageOpt, "Desc")) << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*PackageOpt, "DefaultVal")) << "\"";
+  OS << ")\n";
+}
+  }
+  OS << "#endif // GET_PACKAGE_OPTIONS\n"
+"\n";
+
   // Emit checkers.
   //
   // CHECKER(FULLNAME, CLASS, HELPTEXT)
@@ -160,15 +219,15 @@
   //   - DEPENDENCY: The full name of the checker FULLNAME depends on.
   OS << "\n"
 "#ifdef GET_CHECKER_DEPENDENCIES\n";
-  for (const Record *checker : checkers) {
-if (checker->isValueUnset("Dependencies"))
+  for (const Record *Checker : checkers) {
+if (Checker->isValueUnset("Dependencies"))
   continue;
 
 for (const Record *Dependency :
-checker->getValueAsListOfDefs("Dependencies")) {
+Checker->getValueAsListOfDefs("Dependencies")) {
   OS << "CHECKER_DEPENDENCY(";
   OS << '\"';
-  OS.write_escaped(getCheckerFullName(checker)) << "\", ";
+  OS.write_escaped(getCheckerFullName(Checker)) << "\", ";
   OS << '\"';
   OS.write_escaped(getCheckerFullName(Dependency)) << '\"';
   OS << ")\n";
@@ -176,5 +235,45 @@
   }
   OS << "\n"
   

[PATCH] D59461: [analyzer] Fix an assertion failure if plugins added dependencies

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Don't let the testcase fool you, it's merely added because there wasn't any for 
that particular case, I still need to add the plugin, but that moves quite a 
few files around, so I might separate that to another patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59461



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

[PATCH] D59461: [analyzer] Fix an assertion failure if plugins added dependencies

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D59459: [analyzer][NFC] Prefer binary 
searches in CheckerRegistry.

Ideally, there is no reason behind not being able to depend on checkers that 
come from a different plugin (or on builtin checkers) -- however, this is only 
possible if all checkers are added to the registry before resolving checker 
dependencies. Since I used a binary search in my `addDependency` method, this 
also resulted in an assertion failure (due to `CheckerRegistry::Checkers` not 
being sorted), since the function used by plugins to register their checkers 
(`clang_registerCheckers`) calls `addDependency`.

This patch resolves this issue by only noting which dependencies have to 
established when `addDependency` is called, and resolves them at a later stage 
when no more checkers are added to the registry.


Repository:
  rC Clang

https://reviews.llvm.org/D59461

Files:
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/checker-dependencies.c


Index: test/Analysis/checker-dependencies.c
===
--- test/Analysis/checker-dependencies.c
+++ test/Analysis/checker-dependencies.c
@@ -1,3 +1,20 @@
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=nullability.NullReturnedFromNonnull
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=osx.cocoa.RetainCount \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED
+
+// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCountBase
+// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCount
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=osx.cocoa.RetainCount \
+// RUN:   -analyzer-disable-checker=osx.cocoa.RetainCountBase \
+// RUN:   -analyzer-list-enabled-checkers \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED
+
+// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCountBase
+// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCount
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -180,6 +180,8 @@
 #undef CHECKER_DEPENDENCY
 #undef GET_CHECKER_DEPENDENCIES
 
+  resolveDependencies();
+
   // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
   // command line.
   for (const std::pair  : AnOpts.CheckersControlList) {
@@ -281,17 +283,25 @@
   }
 }
 
-void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
-  auto CheckerIt = binaryFind(Checkers, FullName);
-  assert(CheckerIt != Checkers.end() &&
- "Failed to find the checker while attempting to set up its "
- "dependencies!");
+void CheckerRegistry::resolveDependencies() {
+  for (const std::pair  : Dependencies) {
+auto CheckerIt = binaryFind(Checkers, Entry.first);
+assert(CheckerIt != Checkers.end() &&
+   "Failed to find the checker while attempting to set up its "
+   "dependencies!");
 
-  auto DependencyIt = binaryFind(Checkers, Dependency);
-  assert(DependencyIt != Checkers.end() &&
- "Failed to find the dependency of a checker!");
+auto DependencyIt = binaryFind(Checkers, Entry.second);
+assert(DependencyIt != Checkers.end() &&
+   "Failed to find the dependency of a checker!");
+
+CheckerIt->Dependencies.emplace_back(&*DependencyIt);
+  }
 
-  CheckerIt->Dependencies.emplace_back(&*DependencyIt);
+  Dependencies.clear();
+}
+
+void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
+  Dependencies.emplace_back(FullName, Dependency);
 }
 
 void CheckerRegistry::initializeManager(CheckerManager ) const {
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -195,6 +195,12 @@
   CheckerInfoList Checkers;
   llvm::StringMap PackageSizes;
 
+  /// Contains all (Dependendent checker, Dependency) pairs. We need this, as
+  /// we'll resolve dependencies after all checkers were added first.
+  llvm::SmallVector, 0> Dependencies;
+
+  void resolveDependencies();
+
   DiagnosticsEngine 
   AnalyzerOptions 
   const LangOptions 


Index: test/Analysis/checker-dependencies.c
===
--- test/Analysis/checker-dependencies.c
+++ test/Analysis/checker-dependencies.c
@@ -1,3 +1,20 @@
 // RUN: 

[PATCH] D59459: [analyzer][NFC] Prefer binary searches in CheckerRegistry

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: xazax.hun, NoQ, baloghadamsoftware, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D59458: [analyzer][NFC] Clang-format 
CheckerRegistry.

Repository:
  rC Clang

https://reviews.llvm.org/D59459

Files:
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -48,6 +48,28 @@
 using CheckerNameLT = FullNameLT;
 } // end of anonymous namespace
 
+template 
+static
+typename std::conditional::value,
+  typename CheckerOrPackageInfoList::const_iterator,
+  typename CheckerOrPackageInfoList::iterator>::type
+binaryFind(CheckerOrPackageInfoList , StringRef FullName) {
+
+  using CheckerOrPackage = typename CheckerOrPackageInfoList::value_type;
+  using CheckerOrPackageFullNameLT = FullNameLT;
+
+  assert(std::is_sorted(Collection.begin(), Collection.end(),
+CheckerOrPackageFullNameLT{}) &&
+ "In order to efficiently gather checkers/packages, this function "
+ "expects them to be already sorted!");
+
+  typename CheckerOrPackageInfoList::value_type Info(FullName);
+
+  return llvm::lower_bound(
+  Collection, Info,
+  FullNameLT{});
+}
+
 static constexpr char PackageSeparator = '.';
 
 static bool isInPackage(const CheckerRegistry::CheckerInfo ,
@@ -69,16 +91,7 @@
 
 CheckerRegistry::CheckerInfoListRange
 CheckerRegistry::getMutableCheckersForCmdLineArg(StringRef CmdLineArg) {
-
-  assert(std::is_sorted(Checkers.begin(), Checkers.end(), CheckerNameLT{}) &&
- "In order to efficiently gather checkers, this function expects them "
- "to be already sorted!");
-
-  // Use a binary search to find the possible start of the package.
-  CheckerRegistry::CheckerInfo PackageInfo(nullptr, nullptr, CmdLineArg, "",
-   "");
-  auto It = std::lower_bound(Checkers.begin(), Checkers.end(), PackageInfo,
- CheckerNameLT{});
+  auto It = binaryFind(Checkers, CmdLineArg);
 
   if (!isInPackage(*It, CmdLineArg))
 return {Checkers.end(), Checkers.end()};
@@ -268,24 +281,17 @@
   }
 }
 
-void CheckerRegistry::addDependency(StringRef FullName, StringRef dependency) {
-  auto CheckerThatNeedsDeps = [](const CheckerInfo ) {
-return Chk.FullName == FullName;
-  };
-  auto Dependency = [](const CheckerInfo ) {
-return Chk.FullName == dependency;
-  };
-
-  auto CheckerIt = llvm::find_if(Checkers, CheckerThatNeedsDeps);
+void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) {
+  auto CheckerIt = binaryFind(Checkers, FullName);
   assert(CheckerIt != Checkers.end() &&
  "Failed to find the checker while attempting to set up its "
  "dependencies!");
 
-  auto DependencyIt = llvm::find_if(Checkers, Dependency);
+  auto DependencyIt = binaryFind(Checkers, Dependency);
   assert(DependencyIt != Checkers.end() &&
  "Failed to find the dependency of a checker!");
 
-  CheckerIt->Dependencies.push_back(&*DependencyIt);
+  CheckerIt->Dependencies.emplace_back(&*DependencyIt);
 }
 
 void CheckerRegistry::initializeManager(CheckerManager ) const {
Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -108,8 +108,8 @@
   State_Enabled
 };
 
-InitializationFunction Initialize;
-ShouldRegisterFunction ShouldRegister;
+InitializationFunction Initialize = nullptr;
+ShouldRegisterFunction ShouldRegister = nullptr;
 StringRef FullName;
 StringRef Desc;
 StringRef DocumentationUri;
@@ -129,6 +129,9 @@
 StringRef Name, StringRef Desc, StringRef DocsUri)
 : Initialize(Fn), ShouldRegister(sfn), FullName(Name), Desc(Desc),
   DocumentationUri(DocsUri) {}
+
+// Used for lower_bound.
+explicit CheckerInfo(StringRef FullName) : FullName(FullName) {}
   };
 
   using StateFromCmdLine = CheckerInfo::StateFromCmdLine;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 190973.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

Rebased, NFC.
Moved matchers into D59453 +D57112 
.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113

Files:
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/openmp-use-default-none.rst
  test/clang-tidy/openmp-use-default-none.cpp

Index: test/clang-tidy/openmp-use-default-none.cpp
===
--- /dev/null
+++ test/clang-tidy/openmp-use-default-none.cpp
@@ -0,0 +1,160 @@
+// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c++ -fopenmp -fopenmp-version=40
+// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c   -fopenmp -fopenmp-version=40
+
+////
+// Null cases.
+////
+
+// 'for' directive can not have 'default' clause, no diagnostics.
+void n0(const int a) {
+#pragma omp for
+  for (int b = 0; b < a; b++)
+;
+}
+
+////
+// Single-directive positive cases.
+////
+
+// 'parallel' directive.
+
+// 'parallel' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p0_0() {
+#pragma omp parallel
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'parallel' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p0_1() {
+#pragma omp parallel default(none)
+  ;
+}
+
+// 'parallel' directive can have 'default' clause, and said clause is specified,
+// but with 'shared' kind, which is not 'none', diagnose.
+void p0_2() {
+#pragma omp parallel default(shared)
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead.
+  // CHECK-NOTES: :[[@LINE-3]]:22: note: Existing 'default' clause is specified here.
+}
+
+// 'task' directive.
+
+// 'task' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p1_0() {
+#pragma omp task
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'task' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'task' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p1_1() {
+#pragma omp task default(none)
+  ;
+}
+
+// 'task' directive can have 'default' clause, and said clause is specified,
+// but with 'shared' kind, which is not 'none', diagnose.
+void p1_2() {
+#pragma omp task default(shared)
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'task' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead.
+  // CHECK-NOTES: :[[@LINE-3]]:18: note: Existing 'default' clause is specified here.
+}
+
+// 'teams' directive. (has to be inside of 'target' directive)
+
+// 'teams' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p2_0() {
+#pragma omp target
+#pragma omp teams
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'teams' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'teams' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p2_1() {
+#pragma omp target
+#pragma omp teams default(none)
+  ;
+}
+
+// 'teams' directive can have 'default' clause, and said clause is specified,
+// but with 'shared' kind, which is not 'none', diagnose.
+void p2_2() {
+#pragma omp target
+#pragma omp teams default(shared)
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'teams' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead.
+  // CHECK-NOTES: :[[@LINE-3]]:19: note: Existing 'default' clause is specified here.
+}
+
+// 'taskloop' directive.
+
+// 'taskloop' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p3_0(const int a) {
+#pragma omp taskloop
+  for (int b = 0; b < a; b++)
+;
+  // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'taskloop' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'taskloop' directive can have 'default' clause, and said clause is specified,
+// with 'none' 

[PATCH] D59457: [analyzer][NFC] Use capital variable names in CheckerRegistry

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, baloghadamsoftware, xazax.hun, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, Charusso, jdoerfert, gamesh411, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity.

There are barely any lines I haven't changed in these files, so I think I could 
might as well leave it in an LLVM coding style conforming state. I also renamed 
2 functions and moved `addDependency` out of line to ease on followup patches.


Repository:
  rC Clang

https://reviews.llvm.org/D59457

Files:
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -28,35 +28,41 @@
 
 using RegisterCheckersFn = void (*)(CheckerRegistry &);
 
-static bool isCompatibleAPIVersion(const char *versionString) {
-  // If the version string is null, it's not an analyzer plugin.
-  if (!versionString)
+static bool isCompatibleAPIVersion(const char *VersionString) {
+  // If the version string is null, its not an analyzer plugin.
+  if (!VersionString)
 return false;
 
   // For now, none of the static analyzer API is considered stable.
   // Versions must match exactly.
-  return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
+  return strcmp(VersionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
 }
 
-static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
-  const CheckerRegistry::CheckerInfo ) {
-  return a.FullName < b.FullName;
-}
+namespace {
+template 
+struct FullNameLT {
+  bool operator()(const T , const T ) {
+return Lhs.FullName < Rhs.FullName;
+  }
+};
+
+using CheckerNameLT = FullNameLT;
+} // end of anonymous namespace
 
 static constexpr char PackageSeparator = '.';
 
-static bool isInPackage(const CheckerRegistry::CheckerInfo ,
-StringRef packageName) {
+static bool isInPackage(const CheckerRegistry::CheckerInfo ,
+StringRef PackageName) {
   // Does the checker's full name have the package as a prefix?
-  if (!checker.FullName.startswith(packageName))
+  if (!Checker.FullName.startswith(PackageName))
 return false;
 
   // Is the package actually just the name of a specific checker?
-  if (checker.FullName.size() == packageName.size())
+  if (Checker.FullName.size() == PackageName.size())
 return true;
 
   // Is the checker in the package (or a subpackage)?
-  if (checker.FullName[packageName.size()] == PackageSeparator)
+  if (Checker.FullName[PackageName.size()] == PackageSeparator)
 return true;
 
   return false;
@@ -65,91 +71,95 @@
 CheckerRegistry::CheckerInfoListRange
 CheckerRegistry::getMutableCheckersForCmdLineArg(StringRef CmdLineArg) {
 
-  assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
+  assert(std::is_sorted(Checkers.begin(), Checkers.end(), CheckerNameLT{}) &&
  "In order to efficiently gather checkers, this function expects them "
  "to be already sorted!");
 
   // Use a binary search to find the possible start of the package.
   CheckerRegistry::CheckerInfo
-  packageInfo(nullptr, nullptr, CmdLineArg, "", "");
-  auto it = std::lower_bound(Checkers.begin(), Checkers.end(),
- packageInfo, checkerNameLT);
+  PackageInfo(nullptr, nullptr, CmdLineArg, "", "");
+  auto It = std::lower_bound(Checkers.begin(), Checkers.end(),
+ PackageInfo, CheckerNameLT{});
 
-  if (!isInPackage(*it, CmdLineArg))
+  if (!isInPackage(*It, CmdLineArg))
 return { Checkers.end(), Checkers.end() };
 
   // See how large the package is.
   // If the package doesn't exist, assume the option refers to a single
   // checker.
-  size_t size = 1;
-  llvm::StringMap::const_iterator packageSize =
-  Packages.find(CmdLineArg);
+  size_t Size = 1;
+  llvm::StringMap::const_iterator PackageSize =
+  PackageSizes.find(CmdLineArg);
 
-  if (packageSize != Packages.end())
-size = packageSize->getValue();
+  if (PackageSize != PackageSizes.end())
+Size = PackageSize->getValue();
 
-  return { it, it + size };
+  return { It, It + Size };
 }
 
 CheckerRegistry::CheckerRegistry(
- ArrayRef plugins, DiagnosticsEngine ,
+ ArrayRef Plugins, DiagnosticsEngine ,
  AnalyzerOptions , const LangOptions ,
  ArrayRef>
- checkerRegistrationFns)
-  : Diags(diags), AnOpts(AnOpts), LangOpts(LangOpts) {
+ CheckerRegistrationFns)
+  : Diags(Diags), AnOpts(AnOpts), LangOpts(LangOpts) {
 
   // Register builtin checkers.
 #define GET_CHECKERS
 #define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI)\
   addChecker(register##CLASS, 

[PATCH] D59458: [analyzer][NFC] Clang-format CheckerRegistry

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, baloghadamsoftware, xazax.hun, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

Repository:
  rC Clang

https://reviews.llvm.org/D59458

Files:
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -11,8 +11,8 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
-#include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -39,8 +39,7 @@
 }
 
 namespace {
-template 
-struct FullNameLT {
+template  struct FullNameLT {
   bool operator()(const T , const T ) {
 return Lhs.FullName < Rhs.FullName;
   }
@@ -76,13 +75,13 @@
  "to be already sorted!");
 
   // Use a binary search to find the possible start of the package.
-  CheckerRegistry::CheckerInfo
-  PackageInfo(nullptr, nullptr, CmdLineArg, "", "");
-  auto It = std::lower_bound(Checkers.begin(), Checkers.end(),
- PackageInfo, CheckerNameLT{});
+  CheckerRegistry::CheckerInfo PackageInfo(nullptr, nullptr, CmdLineArg, "",
+   "");
+  auto It = std::lower_bound(Checkers.begin(), Checkers.end(), PackageInfo,
+ CheckerNameLT{});
 
   if (!isInPackage(*It, CmdLineArg))
-return { Checkers.end(), Checkers.end() };
+return {Checkers.end(), Checkers.end()};
 
   // See how large the package is.
   // If the package doesn't exist, assume the option refers to a single
@@ -94,15 +93,14 @@
   if (PackageSize != PackageSizes.end())
 Size = PackageSize->getValue();
 
-  return { It, It + Size };
+  return {It, It + Size};
 }
 
 CheckerRegistry::CheckerRegistry(
- ArrayRef Plugins, DiagnosticsEngine ,
- AnalyzerOptions , const LangOptions ,
- ArrayRef>
- CheckerRegistrationFns)
-  : Diags(Diags), AnOpts(AnOpts), LangOpts(LangOpts) {
+ArrayRef Plugins, DiagnosticsEngine ,
+AnalyzerOptions , const LangOptions ,
+ArrayRef> CheckerRegistrationFns)
+: Diags(Diags), AnOpts(AnOpts), LangOpts(LangOpts) {
 
   // Register builtin checkers.
 #define GET_CHECKERS
@@ -135,22 +133,21 @@
   Diags.Report(diag::warn_incompatible_analyzer_plugin_api)
   << llvm::sys::path::filename(Plugin);
   Diags.Report(diag::note_incompatible_analyzer_plugin_api)
-  << CLANG_ANALYZER_API_VERSION_STRING
-  << PluginAPIVersion;
+  << CLANG_ANALYZER_API_VERSION_STRING << PluginAPIVersion;
   continue;
 }
 
 // Register its checkers.
 RegisterCheckersFn RegisterPluginCheckers =
-reinterpret_cast(Lib.getAddressOfSymbol(
- "clang_registerCheckers"));
+reinterpret_cast(
+Lib.getAddressOfSymbol("clang_registerCheckers"));
 if (RegisterPluginCheckers)
   RegisterPluginCheckers(*this);
   }
 
   // Register statically linked checkers, that aren't generated from the tblgen
-  // file, but rather passed their registry function as a parameter in 
-  // checkerRegistrationFns. 
+  // file, but rather passed their registry function as a parameter in
+  // checkerRegistrationFns.
 
   for (const auto  : CheckerRegistrationFns)
 Fn(*this);
@@ -174,7 +171,7 @@
   // command line.
   for (const std::pair  : AnOpts.CheckersControlList) {
 CheckerInfoListRange CheckerForCmdLineArg =
- getMutableCheckersForCmdLineArg(Opt.first);
+getMutableCheckersForCmdLineArg(Opt.first);
 
 if (CheckerForCmdLineArg.begin() == CheckerForCmdLineArg.end()) {
   Diags.Report(diag::err_unknown_analyzer_checker) << Opt.first;
@@ -182,22 +179,23 @@
 }
 
 for (CheckerInfo  : CheckerForCmdLineArg) {
-  checker.State = Opt.second ? StateFromCmdLine::State_Enabled :
-   StateFromCmdLine::State_Disabled;
+  checker.State = Opt.second ? StateFromCmdLine::State_Enabled
+ : StateFromCmdLine::State_Disabled;
 }
   }
 }
 
 /// Collects dependencies in \p ret, returns false on failure.
-static bool collectDependenciesImpl(
-  const CheckerRegistry::ConstCheckerInfoList ,
-  const LangOptions ,
-  CheckerRegistry::CheckerInfoSet );
+static bool

[PATCH] D57571: [clang-tidy] A new OpenMP module

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 190972.
lebedev.ri added a comment.
Herald added a subscriber: jdoerfert.

Rebased, no changes.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57571

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/index.rst

Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -73,6 +73,7 @@
means "C++11") language constructs.
 ``mpi-``   Checks related to MPI (Message Passing Interface).
 ``objc-``  Checks related to Objective-C coding conventions.
+``openmp-``Checks related to OpenMP API.
 ``performance-``   Checks that target performance-related issues.
 ``portability-``   Checks that target portability-related issues that don't
relate to any particular coding style.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,10 @@
 Improvements to clang-tidy
 --
 
+- New OpenMP module.
+
+  For checks specific to `OpenMP `_ API.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/tool/CMakeLists.txt
===
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -30,6 +30,7 @@
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule
Index: clang-tidy/plugin/CMakeLists.txt
===
--- clang-tidy/plugin/CMakeLists.txt
+++ clang-tidy/plugin/CMakeLists.txt
@@ -21,6 +21,7 @@
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
+  clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule
Index: clang-tidy/openmp/OpenMPTidyModule.cpp
===
--- /dev/null
+++ clang-tidy/openmp/OpenMPTidyModule.cpp
@@ -0,0 +1,35 @@
+//===--- OpenMPTidyModule.cpp - clang-tidy===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+
+namespace clang {
+namespace tidy {
+namespace openmp {
+
+/// This module is for OpenMP-specific checks.
+class OpenMPModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories ) override {
+  }
+};
+
+// Register the OpenMPTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add
+X("openmp-module", "Adds OpenMP-specific checks.");
+
+} // namespace openmp
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the OpenMPModule.
+volatile int OpenMPModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/openmp/CMakeLists.txt
===
--- /dev/null
+++ clang-tidy/openmp/CMakeLists.txt
@@ -0,0 +1,11 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyOpenMPModule
+  OpenMPTidyModule.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangTidy
+  )
Index: clang-tidy/ClangTidyForceLinker.h
===
--- clang-tidy/ClangTidyForceLinker.h
+++ clang-tidy/ClangTidyForceLinker.h
@@ -77,6 +77,11 @@
 MPIModuleAnchorSource;
 #endif
 
+// This anchor is used to force the linker to link the OpenMPModule.
+extern volatile int OpenMPModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED OpenMPModuleAnchorDestination =
+OpenMPModuleAnchorSource;
+
 // This anchor is used to force the linker to link the PerformanceModule.
 extern volatile int PerformanceModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED PerformanceModuleAnchorDestination =
Index: clang-tidy/CMakeLists.txt
===
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -49,6 +49,7 @@
   add_subdirectory(mpi)
 endif()
 add_subdirectory(objc)
+add_subdirectory(openmp)
 add_subdirectory(performance)
 add_subdirectory(plugin)
 

[PATCH] D59455: Thread safety analysis: Add note for unlock kind mismatch

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 190968.
aaronpuchert added a comment.

Factor out some common code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59455

Files:
  include/clang/Analysis/Analyses/ThreadSafety.h
  lib/Analysis/ThreadSafety.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  test/Sema/warn-thread-safety-analysis.c
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -726,26 +726,26 @@
 }
 
 void shared_bad_3() {
-  sls_mu.Lock();
+  sls_mu.Lock(); // expected-note {{mutex acquired here}}
   sls_mu.ReaderUnlock(); // \
 // expected-warning {{releasing mutex 'sls_mu' using shared access, expected exclusive access}}
 }
 
 void shared_bad_4() {
-  sls_mu.ReaderLock();
+  sls_mu.ReaderLock();  // expected-note {{mutex acquired here}}
   sls_mu.ExclusiveUnlock(); // \
 // expected-warning {{releasing mutex 'sls_mu' using exclusive access, expected shared access}}
 }
 
 void shared_bad_5() {
-  sls_mu.Lock();
+  sls_mu.Lock();  // expected-note {{mutex acquired here}}
   sls_mu.PromoteShared(); // \
 // expected-warning {{releasing mutex 'sls_mu' using shared access, expected exclusive access}}
   sls_mu.ExclusiveUnlock();
 }
 
 void shared_bad_6() {
-  sls_mu.ReaderLock();
+  sls_mu.ReaderLock();  // expected-note {{mutex acquired here}}
   sls_mu.DemoteExclusive(); // \
 // expected-warning {{releasing mutex 'sls_mu' using exclusive access, expected shared access}}
   sls_mu.ReaderUnlock();
Index: test/Sema/warn-thread-safety-analysis.c
===
--- test/Sema/warn-thread-safety-analysis.c
+++ test/Sema/warn-thread-safety-analysis.c
@@ -117,11 +117,11 @@
   (void)(*d_ == 1);
   mutex_unlock(foo_.mu_);
 
-  mutex_exclusive_lock();
+  mutex_exclusive_lock();// expected-note {{mutex acquired here}}
   mutex_shared_unlock(); // expected-warning {{releasing mutex 'mu1' using shared access, expected exclusive access}}
   mutex_exclusive_unlock();  // expected-warning {{releasing mutex 'mu1' that was not held}}
 
-  mutex_shared_lock();
+  mutex_shared_lock();  // expected-note {{mutex acquired here}}
   mutex_exclusive_unlock(); // expected-warning {{releasing mutex 'mu1' using exclusive access, expected shared access}}
   mutex_shared_unlock();// expected-warning {{releasing mutex 'mu1' that was not held}}
 
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -1642,6 +1642,13 @@
 return ONS;
   }
 
+  OptionalNotes makeLockedHereNote(SourceLocation LocLocked, StringRef Kind) {
+return LocLocked.isValid()
+   ? getNotes(PartialDiagnosticAt(
+ LocLocked, S.PDiag(diag::note_locked_here) << Kind))
+   : getNotes();
+  }
+
  public:
   ThreadSafetyReporter(Sema , SourceLocation FL, SourceLocation FEL)
 : S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1679,13 +1686,15 @@
 
   void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
  LockKind Expected, LockKind Received,
+ SourceLocation LocLocked,
  SourceLocation Loc) override {
 if (Loc.isInvalid())
   Loc = FunLocation;
 PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch)
  << Kind << LockName << Received
  << Expected);
-Warnings.emplace_back(std::move(Warning), getNotes());
+Warnings.emplace_back(std::move(Warning),
+  makeLockedHereNote(LocLocked, Kind));
   }
 
   void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation LocLocked,
@@ -1694,12 +1703,8 @@
   Loc = FunLocation;
 PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_double_lock)
  << Kind << LockName);
-OptionalNotes Notes =
-LocLocked.isValid()
-? getNotes(PartialDiagnosticAt(
-  LocLocked, S.PDiag(diag::note_locked_here) << Kind))
-: getNotes();
-Warnings.emplace_back(std::move(Warning), std::move(Notes));
+Warnings.emplace_back(std::move(Warning),
+  makeLockedHereNote(LocLocked, Kind));
   }
 
   void handleMutexHeldEndOfScope(StringRef Kind, Name LockName,
@@ -1726,13 +1731,8 @@
 
 PartialDiagnosticAt Warning(LocEndOfScope, S.PDiag(DiagID) << Kind
<< LockName);
-if (LocLocked.isValid()) {
-  PartialDiagnosticAt Note(LocLocked, 

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 190966.
aaronpuchert added a comment.

Don't suggest adding `static` if there is a non-prototype declaration. This 
required a minor refactoring: we let `ShouldWarnAboutMissingPrototype` return 
any kind of declaration it finds and check for the number of parameters at the 
call site.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-missing-prototypes.c
  test/Sema/warn-missing-variable-declarations.c
  test/SemaCXX/warn-missing-variable-declarations.cpp

Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-variable-declarations -fdiagnostics-parseable-fixits -std=c++17 %s 2>&1 | FileCheck %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:3}:"static "
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
 
 template int var_template = 0;
 template constexpr int const_var_template = 0;
Index: test/Sema/warn-missing-variable-declarations.c
===
--- test/Sema/warn-missing-variable-declarations.c
+++ test/Sema/warn-missing-variable-declarations.c
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -Wmissing-variable-declarations -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-variable-declarations -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
 
 int vbad2;
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
@@ -8,6 +10,7 @@
 struct {
   int mgood1;
 } vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:1-[[@LINE-3]]:1}:"static "
 
 int vbad4;
 int vbad4 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad4'}}
Index: test/Sema/warn-missing-prototypes.c
===
--- test/Sema/warn-missing-prototypes.c
+++ test/Sema/warn-missing-prototypes.c
@@ -4,15 +4,20 @@
 int f();
 
 int f(int x) { return x; } // expected-warning{{no previous prototype for function 'f'}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
 
 static int g(int x) { return x; }
 
 int h(int x) { return x; } // expected-warning{{no previous prototype for function 'h'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
 
 static int g2();
 
 int g2(int x) { return x; }
 
+extern int g3(int x) { return x; } // expected-warning{{no previous prototype for function 'g3'}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
+
 void test(void);
 
 int h3();
@@ -40,4 +45,4 @@
 void not_a_prototype_test(); // expected-note{{this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function}}
 void not_a_prototype_test() { } // expected-warning{{no previous prototype for function 'not_a_prototype_test'}}
 
-// CHECK: fix-it:"{{.*}}":{40:27-40:27}:"void"
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:27-[[@LINE-3]]:27}:"void"
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11805,7 +11805,11 @@
   prev = prev->getPreviousDecl();
 
 if (!prev)
-  Diag(var->getLocation(), diag::warn_missing_variable_declarations) << var;
+  Diag(var->getLocation(), diag::warn_missing_variable_declarations)
+  << 

[PATCH] D57112: [ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 190965.
lebedev.ri marked 3 inline comments as done.
lebedev.ri retitled this revision from "[ASTTypeTraits] OMPClause handling" to 
"[ASTTypeTraits][ASTMatchers][OpenMP] OMPClause handling".
lebedev.ri edited the summary of this revision.
lebedev.ri added reviewers: aaron.ballman, george.karpenkov.
lebedev.ri added a comment.
Herald added a subscriber: guansong.

Split off matchers from from D57113 , added 
tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112

Files:
  docs/LibASTMatchersReference.html
  include/clang/AST/ASTTypeTraits.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTTypeTraits.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1780,5 +1780,38 @@
   EXPECT_TRUE(notMatchesWithOpenMP(SourceBad, Matcher));
 }
 
+TEST(OMPDefaultClause, Matches) {
+  auto Matcher = ompExecutableDirective(hasClause(ompDefaultClause()));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2274,5 +2274,117 @@
 notMatches("int main2() {}", functionDecl(isMain(;
 }
 
+TEST(OMPExecutableDirective, hasClause) {
+  auto Matcher = ompExecutableDirective(hasClause(anything()));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+}
+
+TEST(OMPDefaultClause, isNoneKind) {
+  auto Matcher =
+  ompExecutableDirective(hasClause(ompDefaultClause(isNoneKind(;
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source4, Matcher));
+}
+
+TEST(OMPExecutableDirective, isAllowedToContainClause) {
+  auto Matcher =
+  ompExecutableDirective(isAllowedToContainClause(ompDefaultClause()));
+
+  const std::string Source0 = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source3, Matcher));
+
+  const std::string Source4 = R"(void x(int x) {
+#pragma omp parallel num_threads(x)
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(Source4, Matcher));
+
+  const std::string Source5 = R"(void x() {
+#pragma omp taskyield
+})";
+  

[PATCH] D59455: Thread safety analysis: Add note for unlock kind mismatch

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Similar to D56967 , we add the existing 
diag::note_locked_here to tell
the user where we saw the locking that isn't matched correctly.


Repository:
  rC Clang

https://reviews.llvm.org/D59455

Files:
  include/clang/Analysis/Analyses/ThreadSafety.h
  lib/Analysis/ThreadSafety.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  test/Sema/warn-thread-safety-analysis.c
  test/SemaCXX/warn-thread-safety-analysis.cpp

Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -726,26 +726,26 @@
 }
 
 void shared_bad_3() {
-  sls_mu.Lock();
+  sls_mu.Lock(); // expected-note {{mutex acquired here}}
   sls_mu.ReaderUnlock(); // \
 // expected-warning {{releasing mutex 'sls_mu' using shared access, expected exclusive access}}
 }
 
 void shared_bad_4() {
-  sls_mu.ReaderLock();
+  sls_mu.ReaderLock();  // expected-note {{mutex acquired here}}
   sls_mu.ExclusiveUnlock(); // \
 // expected-warning {{releasing mutex 'sls_mu' using exclusive access, expected shared access}}
 }
 
 void shared_bad_5() {
-  sls_mu.Lock();
+  sls_mu.Lock();  // expected-note {{mutex acquired here}}
   sls_mu.PromoteShared(); // \
 // expected-warning {{releasing mutex 'sls_mu' using shared access, expected exclusive access}}
   sls_mu.ExclusiveUnlock();
 }
 
 void shared_bad_6() {
-  sls_mu.ReaderLock();
+  sls_mu.ReaderLock();  // expected-note {{mutex acquired here}}
   sls_mu.DemoteExclusive(); // \
 // expected-warning {{releasing mutex 'sls_mu' using exclusive access, expected shared access}}
   sls_mu.ReaderUnlock();
Index: test/Sema/warn-thread-safety-analysis.c
===
--- test/Sema/warn-thread-safety-analysis.c
+++ test/Sema/warn-thread-safety-analysis.c
@@ -117,11 +117,11 @@
   (void)(*d_ == 1);
   mutex_unlock(foo_.mu_);
 
-  mutex_exclusive_lock();
+  mutex_exclusive_lock();// expected-note {{mutex acquired here}}
   mutex_shared_unlock(); // expected-warning {{releasing mutex 'mu1' using shared access, expected exclusive access}}
   mutex_exclusive_unlock();  // expected-warning {{releasing mutex 'mu1' that was not held}}
 
-  mutex_shared_lock();
+  mutex_shared_lock();  // expected-note {{mutex acquired here}}
   mutex_exclusive_unlock(); // expected-warning {{releasing mutex 'mu1' using exclusive access, expected shared access}}
   mutex_shared_unlock();// expected-warning {{releasing mutex 'mu1' that was not held}}
 
Index: lib/Sema/AnalysisBasedWarnings.cpp
===
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -1642,6 +1642,13 @@
 return ONS;
   }
 
+  OptionalNotes makeLockedHereNote(SourceLocation LocLocked, StringRef Kind) {
+return LocLocked.isValid()
+   ? getNotes(PartialDiagnosticAt(
+ LocLocked, S.PDiag(diag::note_locked_here) << Kind))
+   : getNotes();
+  }
+
  public:
   ThreadSafetyReporter(Sema , SourceLocation FL, SourceLocation FEL)
 : S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1679,13 +1686,15 @@
 
   void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
  LockKind Expected, LockKind Received,
+ SourceLocation LocLocked,
  SourceLocation Loc) override {
 if (Loc.isInvalid())
   Loc = FunLocation;
 PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch)
  << Kind << LockName << Received
  << Expected);
-Warnings.emplace_back(std::move(Warning), getNotes());
+Warnings.emplace_back(std::move(Warning),
+  makeLockedHereNote(LocLocked, Kind));
   }
 
   void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation LocLocked,
@@ -1694,12 +1703,8 @@
   Loc = FunLocation;
 PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_double_lock)
  << Kind << LockName);
-OptionalNotes Notes =
-LocLocked.isValid()
-? getNotes(PartialDiagnosticAt(
-  LocLocked, S.PDiag(diag::note_locked_here) << Kind))
-: getNotes();
-Warnings.emplace_back(std::move(Warning), std::move(Notes));
+Warnings.emplace_back(std::move(Warning),
+  makeLockedHereNote(LocLocked, Kind));
   }
 
   void handleMutexHeldEndOfScope(StringRef Kind, Name LockName,
Index: lib/Analysis/ThreadSafety.cpp
===

[PATCH] D59453: [ASTMatchers][OpenMP] Add base ompExecutableDirective() matcher.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: gribozavr, aaron.ballman, JonasToth, 
george.karpenkov.
lebedev.ri added a project: clang.
Herald added subscribers: jdoerfert, guansong.

Split off from D57113 .


Repository:
  rC Clang

https://reviews.llvm.org/D59453

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/ASTMatchersTest.h

Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -234,6 +234,18 @@
   return matchesConditionallyWithCuda(Code, AMatcher, false, "-std=c++11");
 }
 
+template 
+testing::AssertionResult matchesWithOpenMP(const std::string ,
+   const T ) {
+  return matchesConditionally(Code, AMatcher, true, "-fopenmp");
+}
+
+template 
+testing::AssertionResult notMatchesWithOpenMP(const std::string ,
+  const T ) {
+  return matchesConditionally(Code, AMatcher, false, "-fopenmp");
+}
+
 template 
 testing::AssertionResult
 matchAndVerifyResultConditionally(const std::string , const T ,
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1765,5 +1765,20 @@
   EXPECT_FALSE(matchesObjC(ObjCStringNoPool, autoreleasePoolStmt()));
 }
 
+TEST(OMPExecutableDirective, Matches) {
+  auto Matcher = stmt(ompExecutableDirective());
+
+  const std::string SourceGood = R"(void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(matchesWithOpenMP(SourceGood, Matcher));
+
+  const std::string SourceBad = R"(void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(SourceBad, Matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -505,6 +505,7 @@
   REGISTER_MATCHER(voidType);
   REGISTER_MATCHER(whileStmt);
   REGISTER_MATCHER(withInitializer);
+  REGISTER_MATCHER(ompExecutableDirective);
 }
 
 RegistryMaps::~RegistryMaps() = default;
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -845,5 +845,8 @@
 AST_POLYMORPHIC_SUPPORTED_TYPES(BlockPointerType, MemberPointerType,
 PointerType, ReferenceType));
 
+const internal::VariadicDynCastAllOfMatcher
+ompExecutableDirective;
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -60,6 +60,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
+#include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -6369,6 +6370,25 @@
   return false;
 }
 
+////
+// OpenMP handling.
+////
+
+/// Matches any ``#pragma omp`` executable directive.
+///
+/// Example:
+///
+/// \code
+///   #pragma omp parallel
+///   #pragma omp for <...>
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+ompExecutableDirective;
+
+////
+// End OpenMP handling.
+////
+
 } // namespace ast_matchers
 } // namespace clang
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1375,6 +1375,16 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtompExecutableDirectiveMatcherhttps://clang.llvm.org/doxygen/classclang_1_1OMPExecutableDirective.html;>OMPExecutableDirective...
+Matches any ``#pragma omp`` executable directive.
+
+Example:
+
+  #pragma omp parallel
+  #pragma omp for ...
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Stmt.html;>StmtopaqueValueExprMatcherhttps://clang.llvm.org/doxygen/classclang_1_1OpaqueValueExpr.html;>OpaqueValueExpr...
 Matches opaque value expressions. They are used as helpers
 to reference another expressions and 

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: lib/AST/StmtOpenMP.cpp:40
+  if (auto *LD = dyn_cast(this))
+return LD->getBody();
+  return getInnermostCapturedStmt()->getCapturedStmt();

lebedev.ri wrote:
> riccibruno wrote:
> > lebedev.ri wrote:
> > > @riccibruno
> > > `getBody()` exists in `const`-only variant
> > > `getInnermostCapturedStmt()` does exist in both the `const` and 
> > > non-`const` variants,
> > > but the `const` variant does `const_cast` and defers to non-`const` 
> > > variant.
> > > `getCapturedStmt()` is good though.
> > > 
> > > So at best i can add
> > > ```
> > > Stmt *getStructuredBlock() {
> > >   return const_cast(const_cast > > *>(this)->getStructuredBlock());
> > > }
> > > ```
> > > I'm not sure it's worth it?
> > Or perhaps something like the following to avoid one `const_cast`:
> > 
> > ```
> > Stmt *getStructuredBlock() {
> >   const OMPExecutableDirective *ConstThis = this;
> >   return const_cast(ConstThis->getStructuredBlock());
> > }
> > ```
> > 
> > My point is that in itself this is a minor thing, but not defining 
> > consistently both versions (when appropriate) inevitably leads to annoying 
> > errors. For example suppose that you only define the const version of 
> > `getStructuredBlock()`, and then there is some random function `bar` with 
> > the signature `void bar(Stmt *S)`.
> > 
> > You then try:
> > ```
> > // Intentionally not `const Stmt *` since we will set some random flag in S.
> > void bar(Stmt *S);
> > 
> > // Non-const since we are modifying it.
> > OMPExecutableDirective *OED = something;
> > bar(OED->getStructuredBlock())
> > ```
> > 
> > which fails because `OED->getStructuredBlock()` has type `const Stmt*`, 
> > even though every other pointee type is non-const qualified.
> > 
> > So unless it is intentional to forbid modifying the statement returned by 
> > `getStructuredBlock()`, defining both versions of `getStructuredBlock()` 
> > avoid this problem. Yes it is annoying to have to define the two versions 
> > but as far as I know this is inherent to C++ unfortunately.
> Hmm, okay, will add during next update / before landing, whichever happens 
> next.
Sounds good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/AST/StmtOpenMP.cpp:40
+  if (auto *LD = dyn_cast(this))
+return LD->getBody();
+  return getInnermostCapturedStmt()->getCapturedStmt();

riccibruno wrote:
> lebedev.ri wrote:
> > @riccibruno
> > `getBody()` exists in `const`-only variant
> > `getInnermostCapturedStmt()` does exist in both the `const` and non-`const` 
> > variants,
> > but the `const` variant does `const_cast` and defers to non-`const` variant.
> > `getCapturedStmt()` is good though.
> > 
> > So at best i can add
> > ```
> > Stmt *getStructuredBlock() {
> >   return const_cast(const_cast > *>(this)->getStructuredBlock());
> > }
> > ```
> > I'm not sure it's worth it?
> Or perhaps something like the following to avoid one `const_cast`:
> 
> ```
> Stmt *getStructuredBlock() {
>   const OMPExecutableDirective *ConstThis = this;
>   return const_cast(ConstThis->getStructuredBlock());
> }
> ```
> 
> My point is that in itself this is a minor thing, but not defining 
> consistently both versions (when appropriate) inevitably leads to annoying 
> errors. For example suppose that you only define the const version of 
> `getStructuredBlock()`, and then there is some random function `bar` with the 
> signature `void bar(Stmt *S)`.
> 
> You then try:
> ```
> // Intentionally not `const Stmt *` since we will set some random flag in S.
> void bar(Stmt *S);
> 
> // Non-const since we are modifying it.
> OMPExecutableDirective *OED = something;
> bar(OED->getStructuredBlock())
> ```
> 
> which fails because `OED->getStructuredBlock()` has type `const Stmt*`, even 
> though every other pointee type is non-const qualified.
> 
> So unless it is intentional to forbid modifying the statement returned by 
> `getStructuredBlock()`, defining both versions of `getStructuredBlock()` 
> avoid this problem. Yes it is annoying to have to define the two versions but 
> as far as I know this is inherent to C++ unfortunately.
Hmm, okay, will add during next update / before landing, whichever happens next.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: lib/AST/StmtOpenMP.cpp:40
+  if (auto *LD = dyn_cast(this))
+return LD->getBody();
+  return getInnermostCapturedStmt()->getCapturedStmt();

lebedev.ri wrote:
> @riccibruno
> `getBody()` exists in `const`-only variant
> `getInnermostCapturedStmt()` does exist in both the `const` and non-`const` 
> variants,
> but the `const` variant does `const_cast` and defers to non-`const` variant.
> `getCapturedStmt()` is good though.
> 
> So at best i can add
> ```
> Stmt *getStructuredBlock() {
>   return const_cast(const_cast *>(this)->getStructuredBlock());
> }
> ```
> I'm not sure it's worth it?
Or perhaps something like the following to avoid one `const_cast`:

```
Stmt *getStructuredBlock() {
  const OMPExecutableDirective *ConstThis = this;
  return const_cast(ConstThis->getStructuredBlock());
}
```

My point is that in itself this is a minor thing, but not defining consistently 
both versions (when appropriate) inevitably leads to annoying errors. For 
example suppose that you only define the const version of 
`getStructuredBlock()`, and then there is some random function `bar` with the 
signature `void bar(Stmt *S)`.

You then try:
```
// Intentionally not `const Stmt *` since we will set some random flag in S.
void bar(Stmt *S);

// Non-const since we are modifying it.
OMPExecutableDirective *OED = something;
bar(OED->getStructuredBlock())
```

which fails because `OED->getStructuredBlock()` has type `const Stmt*`, even 
though every other pointee type is non-const qualified.

So unless it is intentional to forbid modifying the statement returned by 
`getStructuredBlock()`, defining both versions of `getStructuredBlock()` avoid 
this problem. Yes it is annoying to have to define the two versions but as far 
as I know this is inherent to C++ unfortunately.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

[JF Bastien via Phabricator via cfe-commits]

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

Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59446



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

r356323 - [analyzer] ConditionBRVisitor: Unknown condition evaluation support

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Mar 16 06:47:55 2019
New Revision: 356323

URL: http://llvm.org/viewvc/llvm-project?rev=356323=rev
Log:
[analyzer] ConditionBRVisitor: Unknown condition evaluation support

Summary:
If the constraint information is not changed between two program states the
analyzer has not learnt new information and made no report. But it is
possible to happen because we have no information at all. The new approach
evaluates the condition to determine if that is the case and let the user
know we just `Assuming...` some value.

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ

Subscribers: llvm-commits, xazax.hun, baloghadamsoftware, szepet, a.sidorin,
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gsd, gerazo

Tags: #clang, #llvm

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/macros.cpp
cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356323=356322=356323=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 
06:47:55 2019
@@ -1815,12 +1815,6 @@ std::shared_ptr
 ConditionBRVisitor::VisitNodeImpl(const ExplodedNode *N,
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
-  ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PreviousState = N->getFirstPred()->getState();
-
-  // If the constraint information does not changed there is no assumption.
-  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState))
-return nullptr;
 
   // If an assumption was made on a branch, it should be caught
   // here by looking at the state transition.
@@ -1910,6 +1904,18 @@ std::shared_ptr
 ConditionBRVisitor::VisitTrueTest(const Expr *Cond, bool tookTrue,
   BugReporterContext , BugReport ,
   const ExplodedNode *N) {
+  ProgramStateRef CurrentState = N->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
+  const LocationContext *LCtx = N->getLocationContext();
+
+  // If the constraint information is changed between the current and the
+  // previous program state we assuming the newly seen constraint information.
+  // If we cannot evaluate the condition (and the constraints are the same)
+  // the analyzer has no information about the value and just assuming it.
+  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState) 
&&
+  CurrentState->getSVal(Cond, LCtx).isValid())
+return nullptr;
+
   // These will be modified in code below, but we need to preserve the original
   //  values in case we want to throw the generic message.
   const Expr *CondTmp = Cond;
@@ -1945,7 +1951,6 @@ ConditionBRVisitor::VisitTrueTest(const
 
   // Condition too complex to explain? Just say something so that the user
   // knew we've made some path decision at this point.
-  const LocationContext *LCtx = N->getLocationContext();
   PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
   if (!Loc.isValid() || !Loc.asLocation().isValid())
 return nullptr;

Modified: cfe/trunk/test/Analysis/diagnostics/macros.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macros.cpp?rev=356323=356322=356323=diff
==
--- cfe/trunk/test/Analysis/diagnostics/macros.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/macros.cpp Sat Mar 16 06:47:55 2019
@@ -30,7 +30,8 @@ void testnullptrMacro(int *p) {
 
 // There are no path notes on the comparison to float types.
 void testDoubleMacro(double d) {
-  if (d == DBL_MAX) { // expected-note {{Taking true branch}}
+  if (d == DBL_MAX) { // expected-note {{Assuming 'd' is equal to DBL_MAX}}
+  // expected-note@-1 {{Taking true branch}}
 
 char *p = NULL; // expected-note {{'p' initialized to a null pointer 
value}}
 *p = 7; // expected-warning {{Dereference of null pointer (loaded 
from variable 'p')}}

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=356323=356322=356323=diff
==
--- cfe/trunk/test/Analysis/uninit-vals.m (original)
+++ cfe/trunk/test/Analysis/uninit-vals.m Sat Mar 16 06:47:55 2019
@@ -164,7 +164,8 @@ void PR14765_test() {
// expected-note@-1{{TRUE}}
 
   testObj->origin = makePoint(0.0, 0.0);
-  if (testObj->size > 0) { ; } // expected-note{{Taking false 

[PATCH] D54978: Move the SMT API to LLVM

[Dan Liew via Phabricator via cfe-commits]

delcypher added a comment.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> Hi all,
>
> Sorry for the massive delay, but I just updated the `FindZ3` script to 
> retrieve the version from the lib. I changed it to use `try_run` instead of 
> `try_compile` so we can get the version number.
>
> I tried to use @brzycki code to get the version from the header, however, 
> it's not working for Z3 4.8.4. In Z3 4.8.3 the FULL_VERSION is a nice `"Z3 
> 4.8.3.0"` but in version 4.8.4 it's `"Z3 4.8.4.10272 d6df51951f4c master 
> z3-4.8.4"` and cmake fails with the following message:
>
>   -- Could NOT find Z3: Found unsuitable version "#define Z3_FULL_VERSION
> "Z3 4.8.4.10272 d6df51951f4c master z3-4.8.4"", but required is at least 
> "4.7.1" (found /home/mgadelha/z3/bin/libz3.so)
>


This output means you built Z3 from source that was in a git repository and you 
used Z3's python build system.

In D54978#1431786 , @ddcc wrote:

> Thanks for making the changes! Is this being parsed from e.g. 
> `/usr/include/z3_version.h`? I checked their code, and I have a local build 
> of z3 4.8.5.0, but I'm not seeing those changes to the version string:
>
>   #define Z3_FULL_VERSION"Z3 4.8.5.0"
>


This output means you built Z3 from source that was not in a git repository. In 
this case the header file should look the same for both Z3's CMake and Python 
build systems.


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

https://reviews.llvm.org/D54978



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/AST/StmtOpenMP.cpp:40
+  if (auto *LD = dyn_cast(this))
+return LD->getBody();
+  return getInnermostCapturedStmt()->getCapturedStmt();

@riccibruno
`getBody()` exists in `const`-only variant
`getInnermostCapturedStmt()` does exist in both the `const` and non-`const` 
variants,
but the `const` variant does `const_cast` and defers to non-`const` variant.
`getCapturedStmt()` is good though.

So at best i can add
```
Stmt *getStructuredBlock() {
  return const_cast(const_cast(this)->getStructuredBlock());
}
```
I'm not sure it's worth it?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D54978: Move the SMT API to LLVM

[Dan Liew via Phabricator via cfe-commits]

delcypher added a comment.

In D54978#1431788 , @ddcc wrote:

> The only relevant commit that I can find is 
> https://github.com/Z3Prover/z3/commit/2cb4223979cc94e2ebc4e49a9e83adbdcd2b6979
>  , but it first landed in z3 4.6.0. It looks like it's specific to CMake 
> though, so is it different if you use the python build? I haven't tried the 
> CMake build.


Hmm this looks like there are some bugs in the different Z3 build systems.

So for the CMake build the value of `Z3_FULL_VERSION_STR` is used to populate 
the `Z3_FULL_VERSION` macro in the header file. Initially this is 
`"${Z3_VERSION_MAJOR}.${Z3_VERSION_MINOR}.${Z3_VERSION_PATCH}.${Z3_VERSION_TWEAK}"`
 but then
If the git has is available we append to it 
https://github.com/Z3Prover/z3/blob/c0f7afacc497e7e2e1a2a28bcebf1173933f5bab/CMakeLists.txt#L96
and if the git description is available we append to it 
https://github.com/Z3Prover/z3/blob/c0f7afacc497e7e2e1a2a28bcebf1173933f5bab/CMakeLists.txt#L109
This was my attempt at mimicking the `get_full_version_string()` function ( 
https://github.com/Z3Prover/z3/blob/038f992ff485d0bf93f962dc9e45330ff1e2e47d/scripts/mk_util.py#L3065
 ) when I added Z3's CMake build system.

Unfortunately this doesn't quite match what Z3's python build system does.

- In the python build system the `VER_REVISION` global (used to set 
`Z3_REVISION_NUMBER` macro and is the forth integer in `Z3_FULL_VERSION`, i.e. 
`d` in `a.b.c.d`)  tries to count the number of commits reachable from the 
current git `HEAD` (see 
https://github.com/Z3Prover/z3/blob/038f992ff485d0bf93f962dc9e45330ff1e2e47d/scripts/mk_util.py#L561
 ). The CMake build system doesn't mimic this. Personally I don't think the 
python build system makes sense here at all given that this integer goes in the 
4th position in the version output (i.e. `d` in `a.b.c.d`).

- In the CMake build system the 4th integer in `Z3_FULL_VERSION` is always 
whatever `Z3_VERSION_TWEAK` is set to in the root `CMakeLists.txt` file. So 
far, I think has always been `0`.

So the bug here is that the CMake and python build systems don't agree on the 
meaning of the 4th version field. Personally I think the implementation of Z3's 
CMake build system makes more sense given where it is placed in the version 
string but that means the name `Z3_REVISION_NUMBER` macro name isn't very 
appropriate.

None of this really mattered until the version header file became public

Would one of you be able to file a bug against Z3 to fix this? I am no longer 
in a position to contribute to Z3 so I can't do this.


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

https://reviews.llvm.org/D54978



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: include/clang/AST/StmtOpenMP.h:274
+  /// Prerequisite: Executable Directive must not be Standalone directive.
+  const Stmt *getStructuredBlock() const;
 };

Uh, and what about the non-const version of `getStructuredBlock` ? I think that 
you just have to do the usual `const_cast` dance to implement the non-const 
version in term of the const version.

Apart from this I have no other comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

r356322 - [analyzer] ConditionBRVisitor: Remove GDM checking

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Mar 16 04:55:07 2019
New Revision: 356322

URL: http://llvm.org/viewvc/llvm-project?rev=356322=rev
Log:
[analyzer] ConditionBRVisitor: Remove GDM checking

Summary:
Removed the `GDM` checking what could prevent reports made by this visitor.
Now we rely on constraint changes instead.
(It reapplies 356318 with a feature from 356319 because build-bot failure.)

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ

Subscribers: cfe-commits, jdoerfert, gerazo, xazax.hun, baloghadamsoftware,
szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp

Tags: #clang

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

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h?rev=356322=356321=356322=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
Sat Mar 16 04:55:07 2019
@@ -79,6 +79,9 @@ public:
   ConstraintManager() = default;
   virtual ~ConstraintManager();
 
+  virtual bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const = 0;
+
   virtual ProgramStateRef assume(ProgramStateRef state,
  DefinedSVal Cond,
  bool Assumption) = 0;

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=356322=356321=356322=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
Sat Mar 16 04:55:07 2019
@@ -588,11 +588,15 @@ public:
   ProgramStateRef getPersistentStateWithGDM(ProgramStateRef FromState,
ProgramStateRef GDMState);
 
-  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) {
+  bool haveEqualConstraints(ProgramStateRef S1, ProgramStateRef S2) const {
+return ConstraintMgr->haveEqualConstraints(S1, S2);
+  }
+
+  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) const {
 return S1->Env == S2->Env;
   }
 
-  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) {
+  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) const {
 return S1->store == S2->store;
   }
 

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=356322=356321=356322=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Sat Mar 16 04:55:07 2019
@@ -220,6 +220,11 @@ public:
 OS << nl;
   }
 
+  bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const override {
+return S1->get() == S2->get();
+  }
+
   bool canReasonAbout(SVal X) const override {
 const TargetInfo  = getBasicVals().getContext().getTargetInfo();
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356322=356321=356322=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 
04:55:07 2019
@@ -1816,13 +1816,10 @@ ConditionBRVisitor::VisitNodeImpl(const
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
   ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PrevState = N->getFirstPred()->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
 
-  // Compare the GDMs of the state, because that is where constraints
-  // are managed.  Note that ensure that we only look at nodes that
-  // were generated by the analyzer engine proper, not 

r356321 - Revert "[analyzer] ConditionBRVisitor: Remove GDM checking"

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Mar 16 03:44:49 2019
New Revision: 356321

URL: http://llvm.org/viewvc/llvm-project?rev=356321=rev
Log:
Revert "[analyzer] ConditionBRVisitor: Remove GDM checking"

This reverts commit f962485adad9d646511fd3240c0408d9554e6784.

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h?rev=356321=356320=356321=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
Sat Mar 16 03:44:49 2019
@@ -79,9 +79,6 @@ public:
   ConstraintManager() = default;
   virtual ~ConstraintManager();
 
-  virtual bool haveEqualConstraints(ProgramStateRef S1,
-ProgramStateRef S2) const = 0;
-
   virtual ProgramStateRef assume(ProgramStateRef state,
  DefinedSVal Cond,
  bool Assumption) = 0;

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=356321=356320=356321=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
Sat Mar 16 03:44:49 2019
@@ -588,15 +588,11 @@ public:
   ProgramStateRef getPersistentStateWithGDM(ProgramStateRef FromState,
ProgramStateRef GDMState);
 
-  bool haveEqualConstraints(ProgramStateRef S1, ProgramStateRef S2) const {
-return ConstraintMgr->haveEqualConstraints(S1, S2);
-  }
-
-  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) const {
+  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) {
 return S1->Env == S2->Env;
   }
 
-  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) const {
+  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) {
 return S1->store == S2->store;
   }
 

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=356321=356320=356321=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Sat Mar 16 03:44:49 2019
@@ -220,11 +220,6 @@ public:
 OS << nl;
   }
 
-  bool haveEqualConstraints(ProgramStateRef S1,
-ProgramStateRef S2) const override {
-return S1->get() == S2->get();
-  }
-
   bool canReasonAbout(SVal X) const override {
 const TargetInfo  = getBasicVals().getContext().getTargetInfo();
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356321=356320=356321=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 
03:44:49 2019
@@ -1816,10 +1816,13 @@ ConditionBRVisitor::VisitNodeImpl(const
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
   ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PreviousState = N->getFirstPred()->getState();
+  ProgramStateRef PrevState = N->getFirstPred()->getState();
 
-  // If the constraint information does not changed there is no assumption.
-  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState))
+  // Compare the GDMs of the state, because that is where constraints
+  // are managed.  Note that ensure that we only look at nodes that
+  // were generated by the analyzer engine proper, not checkers.
+  if (CurrentState->getGDM().getRoot() ==
+  PrevState->getGDM().getRoot())
 return nullptr;
 
   // If an assumption was made on a branch, it should be caught

Modified: 

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis created this revision.
zinovy.nis added reviewers: alexfh, bkramer.
zinovy.nis added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a reviewer: serge-sans-paille.
Herald added a project: clang.

This patch adds a new `--diff[=2]` option to `run-clang-tidy.py` to run 
`clang-tidy` checks against the given diff only, instead of the whole file.

Usage:

  git show -U2 | /some/path/1/run-clang-tidy.py --diff=1 -clang-tidy-binary 
/some/path/2/clang-tidy.exe -p out/Debug -checks=-*,performance-* option_regex


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59449

Files:
  clang-tidy/tool/run-clang-tidy.py

Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -7,7 +7,6 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #======#
-# FIXME: Integrate with clang-tidy-diff.py
 
 """
 Parallel clang-tidy runner
@@ -56,6 +55,10 @@
 else:
 import queue as queue
 
+quote = ""
+if sys.platform != 'win32':
+  quote = "'"
+
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
   result = './'
@@ -73,7 +76,7 @@
   return os.path.normpath(os.path.join(directory, f))
 
 
-def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
+def get_tidy_invocation(f, lines, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, extra_arg, extra_arg_before, quiet,
 config):
   """Gets a command line for clang-tidy."""
@@ -83,6 +86,12 @@
   else:
 # Show warnings in all in-project headers by default.
 start.append('-header-filter=^' + build_path + '/.*')
+
+  if lines is not None:
+line_json = json.dumps([{"name": f, "lines": lines}], separators=(',', ':'))
+# Run clang-tidy on files containing changes.
+start.append('-line-filter=' + quote + line_json + quote)
+
   if checks:
 start.append('-checks=' + checks)
   if tmpdir is not None:
@@ -155,8 +164,9 @@
 def run_tidy(args, tmpdir, build_path, queue, lock, failed_files):
   """Takes filenames out of queue and runs clang-tidy on them."""
   while True:
-name = queue.get()
-invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
+(name, lines) = queue.get()
+invocation = get_tidy_invocation(name, lines,
+ args.clang_tidy_binary, args.checks,
  tmpdir, build_path, args.header_filter,
  args.extra_arg, args.extra_arg_before,
  args.quiet, args.config)
@@ -172,11 +182,42 @@
 queue.task_done()
 
 
+def get_changed_lines(prefix_len):
+  iregex = '^%s$' % r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)'
+  lines_by_file = {}
+  filename = None
+  for line in sys.stdin:
+match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % prefix_len, line)
+if match:
+  filename = match.group(2)
+if filename is None:
+  continue
+
+if not re.match(iregex, filename, re.IGNORECASE):
+  continue
+
+match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
+if match:
+  start_line = int(match.group(1))
+  line_count = 1
+  if match.group(3):
+line_count = int(match.group(3))
+  if line_count == 0:
+continue
+  end_line = start_line + line_count - 1
+  lines_by_file.setdefault(filename, []).append([start_line, end_line])
+
+  return lines_by_file
+
+
 def main():
   parser = argparse.ArgumentParser(description='Runs clang-tidy over all files '
'in a compilation database. Requires '
'clang-tidy and clang-apply-replacements in '
'$PATH.')
+  parser.add_argument('--diff', default=None, const=2, nargs='?', type=int,
+  help='check only the diff read from stdin.'
+  ' Strip the smallest prefix containing DIFF[=2] slashes')
   parser.add_argument('-clang-tidy-binary', metavar='PATH',
   default='clang-tidy',
   help='path to clang-tidy binary')
@@ -244,11 +285,6 @@
 print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)
 
-  # Load the database and extract all files.
-  database = json.load(open(os.path.join(build_path, db_path)))
-  files = [make_absolute(entry['file'], entry['directory'])
-   for entry in database]
-
   max_task = args.j
   if max_task == 0:
 max_task = multiprocessing.cpu_count()
@@ -260,6 +296,16 @@
 
   # Build up a big regexy filter from all command line arguments.
   file_name_re = re.compile('|'.join(args.files))
+  files = None
+  changed_lines = None
+  if args.diff is not None:
+

r356320 - Revert "[analyzer] ConditionBRVisitor: Unknown condition evaluation support"

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Mar 16 03:06:06 2019
New Revision: 356320

URL: http://llvm.org/viewvc/llvm-project?rev=356320=rev
Log:
Revert "[analyzer] ConditionBRVisitor: Unknown condition evaluation support"

This reverts commit 0fe67a61cd4aec13c7969a179517f1cc06ab05cd.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/macros.cpp
cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356320=356319=356320=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 
03:06:06 2019
@@ -1815,6 +1815,12 @@ std::shared_ptr
 ConditionBRVisitor::VisitNodeImpl(const ExplodedNode *N,
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
+  ProgramStateRef CurrentState = N->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
+
+  // If the constraint information does not changed there is no assumption.
+  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState))
+return nullptr;
 
   // If an assumption was made on a branch, it should be caught
   // here by looking at the state transition.
@@ -1883,8 +1889,6 @@ std::shared_ptr Con
 break;
   }
 
-  Cond = Cond->IgnoreParens();
-
   // However, when we encounter a logical operator as a branch condition,
   // then the condition is actually its RHS, because LHS would be
   // the condition for the logical operator terminator.
@@ -1904,18 +1908,6 @@ std::shared_ptr
 ConditionBRVisitor::VisitTrueTest(const Expr *Cond, bool tookTrue,
   BugReporterContext , BugReport ,
   const ExplodedNode *N) {
-  ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PreviousState = N->getFirstPred()->getState();
-  const LocationContext *LCtx = N->getLocationContext();
-
-  // If the constraint information is changed between the current and the
-  // previous program state we assuming the newly seen constraint information.
-  // If we cannot evaluate the condition (and the constraints are the same)
-  // the analyzer has no information about the value and just assuming it.
-  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState) 
&&
-  CurrentState->getSVal(Cond, LCtx).isValid())
-return nullptr;
-
   // These will be modified in code below, but we need to preserve the original
   //  values in case we want to throw the generic message.
   const Expr *CondTmp = Cond;
@@ -1951,6 +1943,7 @@ ConditionBRVisitor::VisitTrueTest(const
 
   // Condition too complex to explain? Just say something so that the user
   // knew we've made some path decision at this point.
+  const LocationContext *LCtx = N->getLocationContext();
   PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
   if (!Loc.isValid() || !Loc.asLocation().isValid())
 return nullptr;

Modified: cfe/trunk/test/Analysis/diagnostics/macros.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macros.cpp?rev=356320=356319=356320=diff
==
--- cfe/trunk/test/Analysis/diagnostics/macros.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/macros.cpp Sat Mar 16 03:06:06 2019
@@ -30,8 +30,7 @@ void testnullptrMacro(int *p) {
 
 // There are no path notes on the comparison to float types.
 void testDoubleMacro(double d) {
-  if (d == DBL_MAX) { // expected-note {{Assuming 'd' is equal to DBL_MAX}}
-  // expected-note@-1 {{Taking true branch}}
+  if (d == DBL_MAX) { // expected-note {{Taking true branch}}
 
 char *p = NULL; // expected-note {{'p' initialized to a null pointer 
value}}
 *p = 7; // expected-warning {{Dereference of null pointer (loaded 
from variable 'p')}}

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=356320=356319=356320=diff
==
--- cfe/trunk/test/Analysis/uninit-vals.m (original)
+++ cfe/trunk/test/Analysis/uninit-vals.m Sat Mar 16 03:06:06 2019
@@ -164,8 +164,7 @@ void PR14765_test() {
// expected-note@-1{{TRUE}}
 
   testObj->origin = makePoint(0.0, 0.0);
-  if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is 
false}}
-   // expected-note@-1{{Taking false branch}}
+  if (testObj->size > 0) { ; } // expected-note{{Taking false branch}}
 
   // FIXME: Assigning to 'testObj->origin' kills the default binding for the
 

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> 2. Instead of parsing `Z3_FULL_VERSION`, we can parse `Z3_MAJOR_VERSION`, 
> `Z3_MINOR_VERSION` and `Z3_BUILD_NUMBER` which are also available in the same 
> header.


Sounds like this might be the way to go, since the format seems to be more 
stable.


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

https://reviews.llvm.org/D54978



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

r356319 - [analyzer] ConditionBRVisitor: Unknown condition evaluation support

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Mar 16 02:24:30 2019
New Revision: 356319

URL: http://llvm.org/viewvc/llvm-project?rev=356319=rev
Log:
[analyzer] ConditionBRVisitor: Unknown condition evaluation support

Summary: If the constraint information is not changed between two program 
states the analyzer has not learnt new information and made no report. But it 
is possible to happen because we have no information at all. The new approach 
evaluates the condition to determine if that is the case and let the user know 
we just 'Assuming...' some value.

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, gsd, gerazo

Tags: #clang

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/macros.cpp
cfe/trunk/test/Analysis/uninit-vals.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356319=356318=356319=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 
02:24:30 2019
@@ -1815,12 +1815,6 @@ std::shared_ptr
 ConditionBRVisitor::VisitNodeImpl(const ExplodedNode *N,
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
-  ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PreviousState = N->getFirstPred()->getState();
-
-  // If the constraint information does not changed there is no assumption.
-  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState))
-return nullptr;
 
   // If an assumption was made on a branch, it should be caught
   // here by looking at the state transition.
@@ -1889,6 +1883,8 @@ std::shared_ptr Con
 break;
   }
 
+  Cond = Cond->IgnoreParens();
+
   // However, when we encounter a logical operator as a branch condition,
   // then the condition is actually its RHS, because LHS would be
   // the condition for the logical operator terminator.
@@ -1908,6 +1904,18 @@ std::shared_ptr
 ConditionBRVisitor::VisitTrueTest(const Expr *Cond, bool tookTrue,
   BugReporterContext , BugReport ,
   const ExplodedNode *N) {
+  ProgramStateRef CurrentState = N->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
+  const LocationContext *LCtx = N->getLocationContext();
+
+  // If the constraint information is changed between the current and the
+  // previous program state we assuming the newly seen constraint information.
+  // If we cannot evaluate the condition (and the constraints are the same)
+  // the analyzer has no information about the value and just assuming it.
+  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState) 
&&
+  CurrentState->getSVal(Cond, LCtx).isValid())
+return nullptr;
+
   // These will be modified in code below, but we need to preserve the original
   //  values in case we want to throw the generic message.
   const Expr *CondTmp = Cond;
@@ -1943,7 +1951,6 @@ ConditionBRVisitor::VisitTrueTest(const
 
   // Condition too complex to explain? Just say something so that the user
   // knew we've made some path decision at this point.
-  const LocationContext *LCtx = N->getLocationContext();
   PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
   if (!Loc.isValid() || !Loc.asLocation().isValid())
 return nullptr;

Modified: cfe/trunk/test/Analysis/diagnostics/macros.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macros.cpp?rev=356319=356318=356319=diff
==
--- cfe/trunk/test/Analysis/diagnostics/macros.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/macros.cpp Sat Mar 16 02:24:30 2019
@@ -30,7 +30,8 @@ void testnullptrMacro(int *p) {
 
 // There are no path notes on the comparison to float types.
 void testDoubleMacro(double d) {
-  if (d == DBL_MAX) { // expected-note {{Taking true branch}}
+  if (d == DBL_MAX) { // expected-note {{Assuming 'd' is equal to DBL_MAX}}
+  // expected-note@-1 {{Taking true branch}}
 
 char *p = NULL; // expected-note {{'p' initialized to a null pointer 
value}}
 *p = 7; // expected-warning {{Dereference of null pointer (loaded 
from variable 'p')}}

Modified: cfe/trunk/test/Analysis/uninit-vals.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=356319=356318=356319=diff
==
--- cfe/trunk/test/Analysis/uninit-vals.m (original)
+++ 

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> I'm sorry for not having a positive answer here, but I'm not in favor of this 
> change. The style rule looks like it introduces arbitrary complexity at a 
> point where I don't understand at all  how it matters. We cannot possibly 
> support all style guides that come up with random variance. I do not think 
> this option pulls its weight.

So is this a personal opinion or based on a technical reason (other than the 
normal mantra about risk & cost),  Could we give @reuk some positive feedback 
about what he needs to change in this review to get this accepted? He has the 
other boxes checked about public style etc..

It may not be your preference but its not without some merit for those of us 
who like to fine tune our style, if that can be done with minimal impact to 
other styles and is well covered by tests, then can we at least have some 
discussion first

@reuk please lets continue to fix this revision so it builds as I want to pull 
it into

https://github.com/mydeveloperday/clang-experimental

As an aside, Wouldn't it be great if JUCE was a public style so you could just 
say

BasedOnStyle: JUCE

Where that style defines what JUCE uses? I've always found this odd, I've never 
understood why we only have Google,WebKit,Mozilla,LLVM, I would have thought it 
would have been nice to have other big guns Microsoft,Qt generally its only 
going to be a single function setting the defaults.

I actually think a change like that would really help people so they don't have 
to work out all the individual styles, it would help keep .clang-format files 
small and neat and not full of a bunch of different options.

Even if JUCE was based on another style say "Google" the code should really be 
saying

if (Style.isGoogleDerivedStyle())

  ..

and not

Style == FormatStyle::Google

When its specific to JUCE it would say

if (Style.isJUCEDerivedStyle())

  ..

I know when using LLVM, that being able to have a .clang-format file that just 
says

BasedOnStyle: LLVM

is really nice, I don't have to go work out what the LLVM style is, I just get 
what the project defines, I think it would be great that anything that passes 
the public style test for submissions should really be able to add that kind of 
configuration.


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

https://reviews.llvm.org/D55170



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

[PATCH] D54811: [analyzer] ConditionBRVisitor: Remove GDM checking

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356318: [analyzer] ConditionBRVisitor: Remove GDM checking 
(authored by Charusso, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

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

https://reviews.llvm.org/D54811

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp


Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -588,11 +588,15 @@
   ProgramStateRef getPersistentStateWithGDM(ProgramStateRef FromState,
ProgramStateRef GDMState);
 
-  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) {
+  bool haveEqualConstraints(ProgramStateRef S1, ProgramStateRef S2) const {
+return ConstraintMgr->haveEqualConstraints(S1, S2);
+  }
+
+  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) const {
 return S1->Env == S2->Env;
   }
 
-  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) {
+  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) const {
 return S1->store == S2->store;
   }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
@@ -220,6 +220,11 @@
 OS << nl;
   }
 
+  bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const override {
+return S1->get() == S2->get();
+  }
+
   bool canReasonAbout(SVal X) const override {
 const TargetInfo  = getBasicVals().getContext().getTargetInfo();
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -79,6 +79,9 @@
   ConstraintManager() = default;
   virtual ~ConstraintManager();
 
+  virtual bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const = 0;
+
   virtual ProgramStateRef assume(ProgramStateRef state,
  DefinedSVal Cond,
  bool Assumption) = 0;
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1816,13 +1816,10 @@
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
   ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PrevState = N->getFirstPred()->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
 
-  // Compare the GDMs of the state, because that is where constraints
-  // are managed.  Note that ensure that we only look at nodes that
-  // were generated by the analyzer engine proper, not checkers.
-  if (CurrentState->getGDM().getRoot() ==
-  PrevState->getGDM().getRoot())
+  // If the constraint information does not changed there is no assumption.
+  if (BRC.getStateManager().haveEqualConstraints(CurrentState, PreviousState))
 return nullptr;
 
   // If an assumption was made on a branch, it should be caught
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -230,6 +230,11 @@
   // Implementation for interface from ConstraintManager.
   //===--===//
 
+  bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const override {
+return S1->get() == S2->get();
+  }
+
   bool canReasonAbout(SVal X) const override;
 
   ConditionTruthVal checkNull(ProgramStateRef State, SymbolRef Sym) override;


Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -588,11 +588,15 @@
   ProgramStateRef 

r356318 - [analyzer] ConditionBRVisitor: Remove GDM checking

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Mar 16 02:16:16 2019
New Revision: 356318

URL: http://llvm.org/viewvc/llvm-project?rev=356318=rev
Log:
[analyzer] ConditionBRVisitor: Remove GDM checking

Summary: Removed the `GDM` checking what could prevent reports made by this 
visitor. Now we rely on constraint changes instead.

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ

Subscribers: jdoerfert, gerazo, xazax.hun, baloghadamsoftware, szepet, 
a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp

Tags: #clang

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

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h?rev=356318=356317=356318=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
Sat Mar 16 02:16:16 2019
@@ -79,6 +79,9 @@ public:
   ConstraintManager() = default;
   virtual ~ConstraintManager();
 
+  virtual bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const = 0;
+
   virtual ProgramStateRef assume(ProgramStateRef state,
  DefinedSVal Cond,
  bool Assumption) = 0;

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=356318=356317=356318=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
Sat Mar 16 02:16:16 2019
@@ -588,11 +588,15 @@ public:
   ProgramStateRef getPersistentStateWithGDM(ProgramStateRef FromState,
ProgramStateRef GDMState);
 
-  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) {
+  bool haveEqualConstraints(ProgramStateRef S1, ProgramStateRef S2) const {
+return ConstraintMgr->haveEqualConstraints(S1, S2);
+  }
+
+  bool haveEqualEnvironments(ProgramStateRef S1, ProgramStateRef S2) const {
 return S1->Env == S2->Env;
   }
 
-  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) {
+  bool haveEqualStores(ProgramStateRef S1, ProgramStateRef S2) const {
 return S1->store == S2->store;
   }
 

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=356318=356317=356318=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Sat Mar 16 02:16:16 2019
@@ -220,6 +220,11 @@ public:
 OS << nl;
   }
 
+  bool haveEqualConstraints(ProgramStateRef S1,
+ProgramStateRef S2) const override {
+return S1->get() == S2->get();
+  }
+
   bool canReasonAbout(SVal X) const override {
 const TargetInfo  = getBasicVals().getContext().getTargetInfo();
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=356318=356317=356318=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Sat Mar 16 
02:16:16 2019
@@ -1816,13 +1816,10 @@ ConditionBRVisitor::VisitNodeImpl(const
   BugReporterContext , BugReport ) {
   ProgramPoint progPoint = N->getLocation();
   ProgramStateRef CurrentState = N->getState();
-  ProgramStateRef PrevState = N->getFirstPred()->getState();
+  ProgramStateRef PreviousState = N->getFirstPred()->getState();
 
-  // Compare the GDMs of the state, because that is where constraints
-  // are managed.  Note that ensure that we only look at nodes that
-  // were generated by the analyzer engine proper, not checkers.
-  if (CurrentState->getGDM().getRoot() ==
-  PrevState->getGDM().getRoot())
+  

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm happy to be wrong, but In current master, SpaceBeforeCpp11BracedList is a 
boolean not an enum  (but I think I've seen a review changing it somewhere 
which maybe isn't landed)

https://reviews.llvm.org/source/llvm-github/browse/master/clang/include/clang/Format/Format.h$1578


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

https://reviews.llvm.org/D55170



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

[PATCH] D59448: [WebAssembly] Change wasm.throw intrinsic's first argument to immediate

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: cfe-commits, sunfish, jgravelle-google, sbc100.
Herald added a project: clang.

After D57825  (r355981), intrinsic arguments 
that are marked as `ImmArg`
only can take immediate values.


Repository:
  rC Clang

https://reviews.llvm.org/D59448

Files:
  test/CodeGen/builtins-wasm.c


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -38,10 +38,10 @@
   // WEBASSEMBLY64: call void @llvm.wasm.data.drop(i32 3)
 }
 
-void throw(unsigned int tag, void *obj) {
-  return __builtin_wasm_throw(tag, obj);
-  // WEBASSEMBLY32: call void @llvm.wasm.throw(i32 %{{.*}}, i8* %{{.*}})
-  // WEBASSEMBLY64: call void @llvm.wasm.throw(i32 %{{.*}}, i8* %{{.*}})
+void throw(void *obj) {
+  return __builtin_wasm_throw(0, obj);
+  // WEBASSEMBLY32: call void @llvm.wasm.throw(i32 0, i8* %{{.*}})
+  // WEBASSEMBLY64: call void @llvm.wasm.throw(i32 0, i8* %{{.*}})
 }
 
 void rethrow_in_catch(void) {


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -38,10 +38,10 @@
   // WEBASSEMBLY64: call void @llvm.wasm.data.drop(i32 3)
 }
 
-void throw(unsigned int tag, void *obj) {
-  return __builtin_wasm_throw(tag, obj);
-  // WEBASSEMBLY32: call void @llvm.wasm.throw(i32 %{{.*}}, i8* %{{.*}})
-  // WEBASSEMBLY64: call void @llvm.wasm.throw(i32 %{{.*}}, i8* %{{.*}})
+void throw(void *obj) {
+  return __builtin_wasm_throw(0, obj);
+  // WEBASSEMBLY32: call void @llvm.wasm.throw(i32 0, i8* %{{.*}})
+  // WEBASSEMBLY64: call void @llvm.wasm.throw(i32 0, i8* %{{.*}})
 }
 
 void rethrow_in_catch(void) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits