[PATCH] D57114: Remove Expr sugar decorating the CXXUuidofExpr node.

[Bill Wendling via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC352307: Remove Expr sugar decorating the CXXUuidofExpr node. 
(authored by void, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57114?vs=183180=183750#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57114

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/PR40395.cpp


Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
Index: test/SemaCXX/PR40395.cpp
===
--- test/SemaCXX/PR40395.cpp
+++ test/SemaCXX/PR40395.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify 
%s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}


Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
Index: test/SemaCXX/PR40395.cpp
===
--- test/SemaCXX/PR40395.cpp
+++ test/SemaCXX/PR40395.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify %s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r352307 - Remove Expr sugar decorating the CXXUuidofExpr node.

[Bill Wendling via cfe-commits]

Author: void
Date: Sat Jan 26 23:24:03 2019
New Revision: 352307

URL: http://llvm.org/viewvc/llvm-project?rev=352307=rev
Log:
Remove Expr sugar decorating the CXXUuidofExpr node.

Summary: Sugar, like ConstantExpr, causes an infinite expansion of the template 
object.

Reviewers: rsmith, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: riccibruno, aaron.ballman, cfe-commits, tzik, rnk

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

Added:
cfe/trunk/test/SemaCXX/PR40395.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=352307=352306=352307=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Sat Jan 26 23:24:03 2019
@@ -6308,7 +6308,7 @@ ExprResult Sema::CheckTemplateArgument(N
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)

Added: cfe/trunk/test/SemaCXX/PR40395.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR40395.cpp?rev=352307=auto
==
--- cfe/trunk/test/SemaCXX/PR40395.cpp (added)
+++ cfe/trunk/test/SemaCXX/PR40395.cpp Sat Jan 26 23:24:03 2019
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify 
%s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}


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

[PATCH] D50563: Fixed frontend clang tests in windows read-only container

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(fwiw using %T is discouraged nowadays, see 
https://llvm.org/docs/CommandGuide/lit.html#pre-defined-substitutions / 
https://reviews.llvm.org/D35396 )


Repository:
  rC Clang

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

https://reviews.llvm.org/D50563



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

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber marked 2 inline comments as done.
bernhardmgruber added a comment.

Thank you for the feedback!

@JonasToth I added tests for `decltype` and i can rewrite all signatures where 
`decltype` is not the top level expression. The reason is, that the source 
range of a `clang::DecltypeType` is not accurate, as it does not include the 
expression within the parenthesis following the `decltype` keyword. There is 
even a FIXME somewhere in the corresponding source file.

@aaron.ballman I am not sure what you mean and maybe you have not understood my 
issue correctly.

In D56160#1369986 , @aaron.ballman 
wrote:

> > ...
>
> I think you may be able to skip the lookup (which could get expensive) and 
> instead rely on the fact that the user must have explicitly written that type 
> as an elaborated type specifier when trying to calculate the source range for 
> the return type. I suspect what's happening right now is that 
> `findReturnTypeAndCVSourceRange()` isn't noticing the `struct` specifier and 
> that's why it's getting dropped. If the user wrote it as an elaborated type 
> specifier, we should probably retain that when shifting it to a trailing 
> return type.


Given the following source code before the rewriting:

  struct Object { long long value; };
  class C {
int Object;
struct Object m();
  };
  Object C::m() { return {0}; }

The member function `struct Object m();` needs to have a `struct` before 
`Object`, because otherwise, the return type would conflict with the member 
variable of the same name. On the contrary, the out-of-line definition `Object 
C::m() { return {0}; }` does not need the `struct`, as the scope of the return 
type does not include the member variables of class `C`. However, rewriting the 
out-of-line definition from `Object C::m() { return {0}; }` to `auto C::m() -> 
Object { return {0}; }` changes the scope of the return type, which now 
includes the member variable `int Object;` and results in a compilation error.
I do not understand what you meant by

> the user must have explicitly written that type as an elaborated type 
> specifier

because in case of the out-of-line defintion, the user is not required to do 
so. Also, when my check rewrites `struct Object m();`, it correctly produces 
`auto m() -> struct Object;` (`findReturnTypeAndCVSourceRange()` includes the 
`struct`).

I tried to circumvent the problem doing something like (F is the matched 
`FunctionDecl`)

  if (auto M = dyn_cast(F)) {
if (auto Name = M->getDeclaredReturnType().getBaseTypeIdentifier()) {
  auto result = M->getDeclContext()->lookup(Name);
  if (!result.empty()) {
// cannot do rewrite, collision
  }
}
  }

but then I noticed, the lookup is only performed in the scope of the member 
function's class, not including e.g. inherited classes. So if we extend the 
example with

  class D : public C {
  ::Object g();
  };
  Object D::g() {

(note that instead of `struct Object` we can also qualify the type with the 
namespace it comes from), then the `DeclContext` of the out-of-line definition 
of the member function `g` is empty (it least, I do not get an output when i 
call `dumpDeclContext`). So maybe the `DeclContext` is not the right tool for 
the job. How else can I query all visible names in which a given object (in 
this case the matched (member) function) is declared? So I can check that the 
name of the return type is not already taken in the scope of the trailing 
return type.

Here is btw. the function case:

  struct Object { long long value; };
  Object j1(unsigned Object) { return {Object * 2}; }

After the rewrite, the return type conflicts with the parameter name.

I appreciate any input! I will continue to explore the problem. Maybe I can get 
it working by inspecting a multitude of `DeclContext`s.


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

https://reviews.llvm.org/D56160



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

r352299 - [FIX] Adjust CXX microsoft abi dynamic cast test to r352293

[Johannes Doerfert via cfe-commits]

Author: jdoerfert
Date: Sat Jan 26 16:22:10 2019
New Revision: 352299

URL: http://llvm.org/viewvc/llvm-project?rev=352299=rev
Log:
[FIX] Adjust CXX microsoft abi dynamic cast test to r352293

Modified:
cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-typeid.cpp

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp?rev=352299=352298=352299=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-dynamic-cast.cpp Sat Jan 26 
16:22:10 2019
@@ -60,7 +60,7 @@ T* test5(A* x) { return dynamic_cast
 // CHECK-NEXT:   [[VBOFFP:%.*]] = getelementptr inbounds i32, i32* [[VBTBL]], 
i32 1
 // CHECK-NEXT:   [[VBOFFS:%.*]] = load i32, i32* [[VBOFFP]], align 4
 // CHECK-NEXT:   [[ADJ:%.*]] = getelementptr inbounds i8, i8* [[VOIDP]], i32 
[[VBOFFS]]
-// CHECK-NEXT:   [[CALL:%.*]] = tail call i8* @__RTDynamicCast(i8* [[ADJ]], 
i32 [[VBOFFS]], i8* {{.*}}bitcast (%rtti.TypeDescriptor7* @"??_R0?AUA@@@8" to 
i8*), i8* {{.*}}bitcast (%rtti.TypeDescriptor7* @"??_R0?AUT@@@8" to i8*), i32 0)
+// CHECK-NEXT:   [[CALL:%.*]] = tail call i8* @__RTDynamicCast(i8* nonnull 
[[ADJ]], i32 [[VBOFFS]], i8* {{.*}}bitcast (%rtti.TypeDescriptor7* 
@"??_R0?AUA@@@8" to i8*), i8* {{.*}}bitcast (%rtti.TypeDescriptor7* 
@"??_R0?AUT@@@8" to i8*), i32 0)
 // CHECK-NEXT:   [[RES:%.*]] = bitcast i8* [[CALL]] to %struct.T*
 // CHECK-NEXT:   br label
 // CHECK:[[RET:%.*]] = phi %struct.T*
@@ -100,7 +100,7 @@ void* test8(A* x) { return dynamic_cast<
 // CHECK-NEXT:   [[VBOFFP:%.*]] = getelementptr inbounds i32, i32* [[VBTBL]], 
i32 1
 // CHECK-NEXT:   [[VBOFFS:%.*]] = load i32, i32* [[VBOFFP]], align 4
 // CHECK-NEXT:   [[ADJ:%.*]] = getelementptr inbounds i8, i8* [[VOIDP]], i32 
[[VBOFFS]]
-// CHECK-NEXT:   [[RES:%.*]] = tail call i8* @__RTCastToVoid(i8* [[ADJ]])
+// CHECK-NEXT:   [[RES:%.*]] = tail call i8* @__RTCastToVoid(i8* nonnull 
[[ADJ]])
 // CHECK-NEXT:   br label
 // CHECK:[[RET:%.*]] = phi i8*
 // CHECK-NEXT:   ret i8* [[RET]]

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-typeid.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-typeid.cpp?rev=352299=352298=352299=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-typeid.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-typeid.cpp Sat Jan 26 16:22:10 2019
@@ -36,7 +36,7 @@ const std::type_info* test3_typeid() { r
 // CHECK-NEXT:   [[VBSLOT:%.*]] = getelementptr inbounds i32, i32* [[VBTBL]], 
i32 1
 // CHECK-NEXT:   [[VBASE_OFFS:%.*]] = load i32, i32* [[VBSLOT]], align 4
 // CHECK-NEXT:   [[ADJ:%.*]] = getelementptr inbounds i8, i8* [[THIS]], i32 
[[VBASE_OFFS]]
-// CHECK-NEXT:   [[RT:%.*]] = tail call i8* @__RTtypeid(i8* [[ADJ]])
+// CHECK-NEXT:   [[RT:%.*]] = tail call i8* @__RTtypeid(i8* nonnull [[ADJ]])
 // CHECK-NEXT:   [[RET:%.*]] = bitcast i8* [[RT]] to %struct.type_info*
 // CHECK-NEXT:   ret %struct.type_info* [[RET]]
 


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

[PATCH] D56974: [SemaCXX] Fix ICE with structure bindings to members of template

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

LGTM, thanks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56974



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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

In D56935#1370861 , @philip.pfaffe 
wrote:

> It would be good to check, since the bots won't! Otherwise this looks good.


Thanks!

I attempted to test on Windows, but noticed that for a shared library CMake 
already reports "Loadable modules not supported on this platform", and couldn't 
create a real plugin. In any case, I attempted to load an arbitrary DLL to see 
if errors are reported correctly and subsequently improved that. If a library 
can't be loaded, it will not fail silently now.

If you're happy with this version, I would need you to land this for me (I do 
not have commit access). Many thanks!


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

https://reviews.llvm.org/D56935



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

LLVM buildmaster will be restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 183729.
mgorny added a comment.

Ok, here's my proposition of using trinary enum.


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

https://reviews.llvm.org/D56554

Files:
  ELF/Config.h
  ELF/Driver.cpp
  ELF/Writer.cpp
  docs/ld.lld.1
  test/ELF/gnustack.s

Index: test/ELF/gnustack.s
===
--- test/ELF/gnustack.s
+++ test/ELF/gnustack.s
@@ -10,6 +10,9 @@
 # RUN: ld.lld %t1 -o %t -z noexecstack
 # RUN: llvm-readobj --program-headers -s %t | FileCheck --check-prefix=RW %s
 
+# RUN: ld.lld %t1 -o %t -z nognustack
+# RUN: llvm-readobj --program-headers -s %t | FileCheck --check-prefix=NOGNUSTACK %s
+
 # RW:  Type: PT_GNU_STACK
 # RW-NEXT: Offset: 0x0
 # RW-NEXT: VirtualAddress: 0x0
@@ -35,5 +38,7 @@
 # RWX-NEXT: ]
 # RWX-NEXT: Alignment: 0
 
+# NOGNUSTACK-NOT: Type: PT_GNU_STACK
+
 .globl _start
 _start:
Index: docs/ld.lld.1
===
--- docs/ld.lld.1
+++ docs/ld.lld.1
@@ -509,6 +509,10 @@
 .Dv DF_1_NOOPEN
 flag to indicate that the object may not be opened by
 .Xr dlopen 3 .
+.It Cm nognustack
+Do not emit the
+.Dv PT_GNU_STACK
+segment.
 .It Cm norelro
 Do not indicate that portions of the object shold be mapped read-only
 after initial relocation processing.
Index: ELF/Writer.cpp
===
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -1976,14 +1976,16 @@
   if (OutputSection *Cmd = findSection(".openbsd.randomdata"))
 AddHdr(PT_OPENBSD_RANDOMIZE, Cmd->getPhdrFlags())->add(Cmd);
 
-  // PT_GNU_STACK is a special section to tell the loader to make the
-  // pages for the stack non-executable. If you really want an executable
-  // stack, you can pass -z execstack, but that's not recommended for
-  // security reasons.
-  unsigned Perm = PF_R | PF_W;
-  if (Config->ZExecstack)
-Perm |= PF_X;
-  AddHdr(PT_GNU_STACK, Perm)->p_memsz = Config->ZStackSize;
+  if (Config->ZGnustack != GnuStackKind::None) {
+// PT_GNU_STACK is a special section to tell the loader to make the
+// pages for the stack non-executable. If you really want an executable
+// stack, you can pass -z execstack, but that's not recommended for
+// security reasons.
+unsigned Perm = PF_R | PF_W;
+if (Config->ZGnustack == GnuStackKind::Exec)
+  Perm |= PF_X;
+AddHdr(PT_GNU_STACK, Perm)->p_memsz = Config->ZStackSize;
+  }
 
   // PT_OPENBSD_WXNEEDED is a OpenBSD-specific header to mark the executable
   // is expected to perform W^X violations, such as calling mprotect(2) or
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -345,6 +345,20 @@
   return Default;
 }
 
+static GnuStackKind getZGnuStack(opt::InputArgList ) {
+  for (auto *Arg : Args.filtered_reverse(OPT_z)) {
+if (StringRef("execstack") == Arg->getValue())
+  return GnuStackKind::Exec;
+else if (StringRef("noexecstack") == Arg->getValue())
+  return GnuStackKind::NoExec;
+else if (StringRef("nognustack") == Arg->getValue())
+  return GnuStackKind::None;
+  }
+
+  // default
+  return GnuStackKind::NoExec;
+}
+
 static bool isKnownZFlag(StringRef S) {
   return S == "combreloc" || S == "copyreloc" || S == "defs" ||
  S == "execstack" || S == "global" || S == "hazardplt" ||
@@ -352,6 +366,7 @@
  S == "keep-text-section-prefix" || S == "lazy" || S == "muldefs" ||
  S == "nocombreloc" || S == "nocopyreloc" || S == "nodefaultlib" ||
  S == "nodelete" || S == "nodlopen" || S == "noexecstack" ||
+ S == "nognustack" ||
  S == "nokeep-text-section-prefix" || S == "norelro" || S == "notext" ||
  S == "now" || S == "origin" || S == "relro" || S == "retpolineplt" ||
  S == "rodynamic" || S == "text" || S == "wxneeded" ||
@@ -868,8 +883,8 @@
   Args.hasFlag(OPT_warn_symbol_ordering, OPT_no_warn_symbol_ordering, true);
   Config->ZCombreloc = getZFlag(Args, "combreloc", "nocombreloc", true);
   Config->ZCopyreloc = getZFlag(Args, "copyreloc", "nocopyreloc", true);
-  Config->ZExecstack = getZFlag(Args, "execstack", "noexecstack", false);
   Config->ZGlobal = hasZOption(Args, "global");
+  Config->ZGnustack = getZGnuStack(Args);
   Config->ZHazardplt = hasZOption(Args, "hazardplt");
   Config->ZInitfirst = hasZOption(Args, "initfirst");
   Config->ZInterpose = hasZOption(Args, "interpose");
Index: ELF/Config.h
===
--- ELF/Config.h
+++ ELF/Config.h
@@ -61,6 +61,9 @@
 // For tracking ARM Float Argument PCS
 enum class ARMVFPArgKind { Default, Base, VFP, ToolChain };
 
+// For -z *stack
+enum class GnuStackKind { None, Exec, NoExec };
+
 struct SymbolVersion {
   llvm::StringRef Name;
   bool IsExternCpp;
@@ -184,8 +187,8 @@
   bool WriteAddends;
   bool ZCombreloc;
   bool ZCopyreloc;
-  bool 

[PATCH] D52790: [analyzer][PlistMacroExpansion] New flag to convert macro expansions to events

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

Szelethus abandoned this revision.
Szelethus added a comment.

As we currently support the actual `macro_expansions` format in CodeChecker, 
and AFAIK you guys at Apple prefer relying on Xcode 
(http://lists.llvm.org/pipermail/cfe-dev/2018-September/059231.html), I'd like 
to focus on more pressing matters.


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

https://reviews.llvm.org/D52790



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

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352292: [analyzer] Add CheckerManager::getChecker, make sure 
that a registry function… (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55429?vs=182727=183727#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55429

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  cfe/trunk/test/Analysis/analyzer-checker-config.c
  cfe/trunk/test/Analysis/free.c
  cfe/trunk/test/Analysis/outofbound.c
  cfe/trunk/test/Analysis/undef-buffers.c

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -141,7 +141,7 @@
   using CheckerDtor = CheckerFn;
 
 //===--===//
-// registerChecker
+// Checker registration.
 //===--===//
 
   /// Used to register checkers.
@@ -153,8 +153,7 @@
   CHECKER *registerChecker(AT &&... Args) {
 CheckerTag tag = getTag();
 CheckerRef  = CheckerTags[tag];
-if (ref)
-  return static_cast(ref); // already registered.
+assert(!ref && "Checker already registered, use getChecker!");
 
 CHECKER *checker = new CHECKER(std::forward(Args)...);
 checker->Name = CurrentCheckName;
@@ -164,8 +163,17 @@
 return checker;
   }
 
+  template 
+  CHECKER *getChecker() {
+CheckerTag tag = getTag();
+assert(CheckerTags.count(tag) != 0 &&
+   "Requested checker is not registered! Maybe you should add it as a "
+   "dependency in Checkers.td?");
+return static_cast(CheckerTags[tag]);
+  }
+
 //===--===//
-// Functions for running checkers for AST traversing..
+// Functions for running checkers for AST traversing.
 //===--===//
 
   /// Run checkers handling Decls.
Index: cfe/trunk/test/Analysis/free.c
===
--- cfe/trunk/test/Analysis/free.c
+++ cfe/trunk/test/Analysis/free.c
@@ -1,5 +1,11 @@
-// RUN: %clang_analyze_cc1 -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-store=region -analyzer-checker=core,unix.Malloc -fblocks -verify -analyzer-config unix.Malloc:Optimistic=true %s
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc
+//
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -analyzer-store=region \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 typedef __typeof(sizeof(int)) size_t;
 void free(void *);
 void *alloca(size_t);
Index: cfe/trunk/test/Analysis/undef-buffers.c
===
--- cfe/trunk/test/Analysis/undef-buffers.c
+++ cfe/trunk/test/Analysis/undef-buffers.c
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,core.uninitialized -analyzer-store=region -verify -analyzer-config unix:Optimistic=true %s
+// RUN: %clang_analyze_cc1 -analyzer-store=region -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix \
+// RUN:   -analyzer-checker=core.uninitialized \
+// RUN:   -analyzer-config unix:Optimistic=true
+
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);
Index: cfe/trunk/test/Analysis/analyzer-checker-config.c
===
--- cfe/trunk/test/Analysis/analyzer-checker-config.c
+++ cfe/trunk/test/Analysis/analyzer-checker-config.c
@@ -4,7 +4,7 @@
 // RUN: not 

r352292 - [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 13:41:50 2019
New Revision: 352292

URL: http://llvm.org/viewvc/llvm-project?rev=352292=rev
Log:
[analyzer] Add CheckerManager::getChecker, make sure that a registry function 
registers no more than 1 checker

This patch effectively fixes the almost decade old checker naming issue.
The solution is to assert when CheckerManager::getChecker is called on an
unregistered checker, and assert when CheckerManager::registerChecker is called
on a checker that is already registered.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
cfe/trunk/test/Analysis/analyzer-checker-config.c
cfe/trunk/test/Analysis/free.c
cfe/trunk/test/Analysis/outofbound.c
cfe/trunk/test/Analysis/undef-buffers.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h?rev=352292=352291=352292=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h Sat Jan 26 
13:41:50 2019
@@ -141,7 +141,7 @@ public:
   using CheckerDtor = CheckerFn;
 
 
//===--===//
-// registerChecker
+// Checker registration.
 
//===--===//
 
   /// Used to register checkers.
@@ -153,8 +153,7 @@ public:
   CHECKER *registerChecker(AT &&... Args) {
 CheckerTag tag = getTag();
 CheckerRef  = CheckerTags[tag];
-if (ref)
-  return static_cast(ref); // already registered.
+assert(!ref && "Checker already registered, use getChecker!");
 
 CHECKER *checker = new CHECKER(std::forward(Args)...);
 checker->Name = CurrentCheckName;
@@ -164,8 +163,17 @@ public:
 return checker;
   }
 
+  template 
+  CHECKER *getChecker() {
+CheckerTag tag = getTag();
+assert(CheckerTags.count(tag) != 0 &&
+   "Requested checker is not registered! Maybe you should add it as a "
+   "dependency in Checkers.td?");
+return static_cast(CheckerTags[tag]);
+  }
+
 
//===--===//
-// Functions for running checkers for AST traversing..
+// Functions for running checkers for AST traversing.
 
//===--===//
 
   /// Run checkers handling Decls.

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=352292=352291=352292=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Sat Jan 26 
13:41:50 2019
@@ -2485,7 +2485,7 @@ bool ento::shouldRegisterCStringModeling
 
 #define REGISTER_CHECKER(name) 
\
   void ento::register##name(CheckerManager ) { 
\
-CStringChecker *checker = mgr.registerChecker();   
\
+CStringChecker *checker = mgr.getChecker();
\
 checker->Filter.Check##name = true;
\
 checker->Filter.CheckName##name = mgr.getCurrentCheckName();   
\
   }
\

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp?rev=352292=352291=352292=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp Sat Jan 26 
13:41:50 2019

[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

In D56554#1369606 , @mgorny wrote:

> In D56554#1369592 , @ruiu wrote:
>
> > No, I'm suggesting you add execstack, noexecstack and nognustack as a 
> > tri-state -z flag. Does this sound good?
>
>
> Do you mean that of those three options, the last one specified should take 
> precedence?


(and, if yes, could you suggest how to implement it? `getZOption()` doesn't 
seem suitable.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56554



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

[clang-tools-extra] r352290 - Fix a lit test failure after D54438

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 13:22:58 2019
New Revision: 352290

URL: http://llvm.org/viewvc/llvm-project?rev=352290=rev
Log:
Fix a lit test failure after D54438

Modified:
clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp?rev=352290=352289=352290=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/static-analyzer-config.cpp Sat Jan 
26 13:22:58 2019
@@ -1,5 +1,5 @@
 // REQUIRES: static-analyzer
-// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' 
-config='{CheckOptions: [{ key: "clang-analyzer-unix.Malloc:Optimistic", value: 
true}]}' -- | FileCheck %s
+// RUN: clang-tidy %s -checks='-*,clang-analyzer-unix.Malloc' 
-config='{CheckOptions: [{ key: 
"clang-analyzer-unix.DynamicMemoryModeling:Optimistic", value: true}]}' -- | 
FileCheck %s
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);


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

[PATCH] D56932: [Driver] [NetBSD] Pass default library search paths to linker

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Gentle ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56932



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

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

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

Szelethus added a comment.

In D54438#1372324 , @NoQ wrote:

> *gets hyped for the upcoming patchlanding party*


Oh yeah, super excited about this! It was a blast!

> In D54438#1329425 , @Szelethus wrote:
> 
>> Hmmm, I left plenty of room for improvement in the tblgen generation, we 
>> could generate compile-time errors on cycles within the dependency graph, 
>> try to include the generated file only once, but these clearly are very 
>> low-prio issues, so I'll do it later. I'll have to revisit the entire thing 
>> soon enough.
> 
> 
> Hmm. The dependency cycle check sounds cool as long as we do actually have 
> problems with dependency cycles. I guess we could just enable/disable the 
> whole cycle of checkers all together? This might even be a useful feature.

That sounds pretty cool actually! For all the crap I give people about 
documentation, I'm struggling quite a bit with this one (will probably end up 
making one for the entirety of `Frontend/`), but will definitely include this 
in there.

> What do you mean by "include the generated file only once"?

Never mind. It was thinking aloud, which I later realized is nonsense.

> Aha, ok, so the current approach to conflict resolution is to only enable the 
> checker if none of its dependencies were disabled on the command line. So if, 
> say, `DependentChecker` depends on `BaseChecker`, once an 
> -analyzer-disable-checker `BaseChecker` flag is passed, it needs to be 
> reverted via `-analyzer-checker BaseChecker` before the `-analyzer-checker 
> DependentChecker` directive would take effect, regardless of in which order 
> it is with respect to the `BaseChecker`-enabling/disabling directives.

Exactly.

> So we kinda choose to err on the side of disabling in ambiguous situations. 
> Which kinda makes sense because the disable-argument is more rare and 
> indicates an irritation of the user. But it is also kinda inconsistent with 
> how the options interact in absence of dependencies: "the flag that was 
> passed last overrides all previous flags". And we can kinda fix this 
> inconsistency by introducing a different behavior:
> 
> - whenever an `-analyzer-checker` flag is passed, remove the "disabled" marks 
> from all checkers it depends on;
> - whenever an `-analyzer-disable-checker` flag is passed, remove the 
> "enabled" marks from all checkers that depend on it.
> 
> This approach still ensures that the set of enabled checkers is consistent 
> (i.e., users are still not allowed to shoot themselves in the foot by running 
> a checker without its dependencies), but it also looks respects every flag in 
> an intuitive manner. WDYT?

Sounds great! Got nothing against that.

Hmm, I didn't really add tests about the dependency related potential implicit 
disabling, not even a warning, so there still is more work to be done.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54438



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

[PATCH] D57106: [AST] Introduce GenericSelectionExpr::Association

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/Expr.h:5084
+  /// storage of Stmt * and TypeSourceInfo * in GenericSelectionExpr.
+  template  class AssociationIteratorTy {
+friend class GenericSelectionExpr;

riccibruno wrote:
> aaron.ballman wrote:
> > riccibruno wrote:
> > > aaron.ballman wrote:
> > > > riccibruno wrote:
> > > > > dblaikie wrote:
> > > > > > Worth using any of the iterator helpers LLVM has? (iterator_facade 
> > > > > > or the like)
> > > > > I did try to use `iteratore_facade` but for some reason I was getting 
> > > > > strange overload resolution failures with it.
> > > > > 
> > > > > In the end it did not save much and so I just rewrote the 
> > > > > boiler-plate (especially given that if we end up going with an input 
> > > > > iterator there is not going to be much boiler-plate).
> > > > Does using the `iterator_facade_base` help now that we're back to an 
> > > > input iterator? It seems like that should be able to get rid of some of 
> > > > the boilerplate.
> > > I must be holding it wrong; for some reason the post-fix operator ++ is 
> > > not getting found when I use `iterator_facade_base`. It also forces me to 
> > > define `operator==` as a member instead of a non-member function. Do you 
> > > mind terribly if I don't use it ? It only at best avoid me to write 
> > > `operator!=` and `operator++(int)`.
> > It also removes all of the typedefs and `operator->()`, so it does remove 
> > quite a bit of boilerplate. You shouldn't have to do anything special to 
> > get it to locate the postfix operator++ though (so long as you use public 
> > inheritance), which makes me wonder what's going on for your use. I would 
> > like to understand more about why this base class doesn't work here when it 
> > seems to work fine for the other uses in the code base.
> Right. Let me try again then. I will still have to provide `operator->` 
> though since the implementation from `iterator_facade_base` is not adequate 
> as far as I can tell.
Yeah, I think you're right about `operator->()`; because of the odd semantics 
of the Association object, the facade implementation won't work there because 
it would return a pointer to a temporary.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57106



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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 183715.
melver added a comment.

- Improve error reporting. While testing on Windows, noticed that Clang wants 
the error to be checked otherwise crashed quite verbosely.


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

https://reviews.llvm.org/D56935

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1322,6 +1322,8 @@
 
   Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
 
+  Opts.PassPlugins = Args.getAllArgValues(OPT_fpass_plugin_EQ);
+
   return Success;
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5066,6 +5066,13 @@
 A->claim();
   }
 
+  // Forward -fpass-plugin=name.so to -cc1.
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ)) {
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-fpass-plugin=") + A->getValue()));
+A->claim();
+  }
+
   // Setup statistics file output.
   SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
   if (!StatsFile.empty())
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -37,6 +37,7 @@
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Passes/PassBuilder.h"
+#include "llvm/Passes/PassPlugin.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -962,6 +963,17 @@
 
   PassBuilder PB(TM.get(), PGOOpt);
 
+  // Attempt to load pass plugins and register their callbacks with PB.
+  for (auto  : CodeGenOpts.PassPlugins) {
+auto PassPlugin = PassPlugin::Load(PluginFN);
+if (PassPlugin) {
+  PassPlugin->registerPassBuilderCallbacks(PB);
+} else {
+  Diags.Report(diag::err_fe_unable_to_load_plugin)
+  << PluginFN << toString(PassPlugin.takeError());
+}
+  }
+
   LoopAnalysisManager LAM(CodeGenOpts.DebugPassManager);
   FunctionAnalysisManager FAM(CodeGenOpts.DebugPassManager);
   CGSCCAnalysisManager CGAM(CodeGenOpts.DebugPassManager);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1613,6 +1613,9 @@
 def fno_rwpi : Flag<["-"], "fno-rwpi">, Group;
 def fplugin_EQ : Joined<["-"], "fplugin=">, Group, 
Flags<[DriverOption]>, MetaVarName<"">,
   HelpText<"Load the named plugin (dynamic shared object)">;
+def fpass_plugin_EQ : Joined<["-"], "fpass-plugin=">,
+  Group, Flags<[CC1Option]>, MetaVarName<"">,
+  HelpText<"Load pass plugin from a dynamic shared object file (only with new 
pass manager).">;
 def fpreserve_as_comments : Flag<["-"], "fpreserve-as-comments">, 
Group;
 def fno_preserve_as_comments : Flag<["-"], "fno-preserve-as-comments">, 
Group, Flags<[CC1Option]>,
   HelpText<"Do not preserve comments in inline assembly">;
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -288,6 +288,9 @@
 
   std::vector DefaultFunctionAttrs;
 
+  /// List of dynamic shared object files to be loaded as pass plugins.
+  std::vector PassPlugins;
+
 public:
   // Define accessors/mutators for code generation options of enumeration type.
 #define CODEGENOPT(Name, Bits, Default)


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1322,6 +1322,8 @@
 
   Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
 
+  Opts.PassPlugins = Args.getAllArgValues(OPT_fpass_plugin_EQ);
+
   return Success;
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5066,6 +5066,13 @@
 A->claim();
   }
 
+  // Forward -fpass-plugin=name.so to -cc1.
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ)) {
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-fpass-plugin=") + A->getValue()));
+A->claim();
+  }
+
   // Setup statistics file output.
   SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, 

[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D57230#1372523 , @NoQ wrote:

> In D57230#1372488 , @xazax.hun wrote:
>
> > In D57230#1372275 , @NoQ wrote:
> >
> > >
> >
> >
> > Do you have success reducing false positives using creduce? My problem 
> > usually is that we cannot tell if a reduction rendered a false positive 
> > into a true positive.
>
>
> False positives - no. Improvements and regressions - totally! Just run two 
> different clangs in the creduce test and check that there's a difference in 
> results.


Oh, I see. Great idea, I never did this. Will look into it.


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

https://reviews.llvm.org/D57230



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

[PATCH] D57106: [AST] Introduce GenericSelectionExpr::Association

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked an inline comment as done.
riccibruno added inline comments.



Comment at: include/clang/AST/Expr.h:5084
+  /// storage of Stmt * and TypeSourceInfo * in GenericSelectionExpr.
+  template  class AssociationIteratorTy {
+friend class GenericSelectionExpr;

aaron.ballman wrote:
> riccibruno wrote:
> > aaron.ballman wrote:
> > > riccibruno wrote:
> > > > dblaikie wrote:
> > > > > Worth using any of the iterator helpers LLVM has? (iterator_facade or 
> > > > > the like)
> > > > I did try to use `iteratore_facade` but for some reason I was getting 
> > > > strange overload resolution failures with it.
> > > > 
> > > > In the end it did not save much and so I just rewrote the boiler-plate 
> > > > (especially given that if we end up going with an input iterator there 
> > > > is not going to be much boiler-plate).
> > > Does using the `iterator_facade_base` help now that we're back to an 
> > > input iterator? It seems like that should be able to get rid of some of 
> > > the boilerplate.
> > I must be holding it wrong; for some reason the post-fix operator ++ is not 
> > getting found when I use `iterator_facade_base`. It also forces me to 
> > define `operator==` as a member instead of a non-member function. Do you 
> > mind terribly if I don't use it ? It only at best avoid me to write 
> > `operator!=` and `operator++(int)`.
> It also removes all of the typedefs and `operator->()`, so it does remove 
> quite a bit of boilerplate. You shouldn't have to do anything special to get 
> it to locate the postfix operator++ though (so long as you use public 
> inheritance), which makes me wonder what's going on for your use. I would 
> like to understand more about why this base class doesn't work here when it 
> seems to work fine for the other uses in the code base.
Right. Let me try again then. I will still have to provide `operator->` though 
since the implementation from `iterator_facade_base` is not adequate as far as 
I can tell.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57106



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

[PATCH] D57106: [AST] Introduce GenericSelectionExpr::Association

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/Expr.h:5084
+  /// storage of Stmt * and TypeSourceInfo * in GenericSelectionExpr.
+  template  class AssociationIteratorTy {
+friend class GenericSelectionExpr;

riccibruno wrote:
> aaron.ballman wrote:
> > riccibruno wrote:
> > > dblaikie wrote:
> > > > Worth using any of the iterator helpers LLVM has? (iterator_facade or 
> > > > the like)
> > > I did try to use `iteratore_facade` but for some reason I was getting 
> > > strange overload resolution failures with it.
> > > 
> > > In the end it did not save much and so I just rewrote the boiler-plate 
> > > (especially given that if we end up going with an input iterator there is 
> > > not going to be much boiler-plate).
> > Does using the `iterator_facade_base` help now that we're back to an input 
> > iterator? It seems like that should be able to get rid of some of the 
> > boilerplate.
> I must be holding it wrong; for some reason the post-fix operator ++ is not 
> getting found when I use `iterator_facade_base`. It also forces me to define 
> `operator==` as a member instead of a non-member function. Do you mind 
> terribly if I don't use it ? It only at best avoid me to write `operator!=` 
> and `operator++(int)`.
It also removes all of the typedefs and `operator->()`, so it does remove quite 
a bit of boilerplate. You shouldn't have to do anything special to get it to 
locate the postfix operator++ though (so long as you use public inheritance), 
which makes me wonder what's going on for your use. I would like to understand 
more about why this base class doesn't work here when it seems to work fine for 
the other uses in the code base.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57106



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

[PATCH] D56989: [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

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

Szelethus added a comment.

Oops, I ran into the issue of `check-clang-analysis` not actually running out 
unit tests. I decided not to revert this patch and just commit the fix: 
rC352284 . Any objections?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56989



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

[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D57230#1372488 , @xazax.hun wrote:

> In D57230#1372275 , @NoQ wrote:
>
> >
>
>
> Do you have success reducing false positives using creduce? My problem 
> usually is that we cannot tell if a reduction rendered a false positive into 
> a true positive.


False positives - no. Improvements and regressions - totally! Just run two 
different clangs in the creduce test and check that there's a difference in 
results.


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

https://reviews.llvm.org/D57230



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

r352284 - [analyzer] Fix an bug where statically linked, but not registered checkers weren't recognized

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 09:27:40 2019
New Revision: 352284

URL: http://llvm.org/viewvc/llvm-project?rev=352284=rev
Log:
[analyzer] Fix an bug where statically linked, but not registered checkers 
weren't recognized

My last patch, D56989, moved the validation of whether a checker exists into
its constructor, but we do support statically linked (and non-plugin) checkers
that were do not have an entry in Checkers.td. However, the handling of this
happens after the creation of the CheckerRegistry object.

This patch fixes this bug by moving even this functionality into
CheckerRegistry's constructor.

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h?rev=352284=352283=352284=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h Sat Jan 
26 09:27:40 2019
@@ -81,8 +81,11 @@ namespace ento {
 /// "core.builtin", or the full name "core.builtin.NoReturnFunctionChecker".
 class CheckerRegistry {
 public:
-  CheckerRegistry(ArrayRef plugins, DiagnosticsEngine ,
-  AnalyzerOptions , const LangOptions );
+  CheckerRegistry(
+  ArrayRef plugins, DiagnosticsEngine ,
+  AnalyzerOptions , const LangOptions ,
+  ArrayRef>
+  checkerRegistrationFns = {});
 
   /// Initialization functions perform any necessary setup for a checker.
   /// They should include a call to CheckerManager::registerChecker.

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp?rev=352284=352283=352284=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp Sat Jan 26 
09:27:40 2019
@@ -33,10 +33,8 @@ std::unique_ptr ento::cr
 DiagnosticsEngine ) {
   auto checkerMgr = llvm::make_unique(context, opts);
 
-  CheckerRegistry allCheckers(plugins, diags, opts, context.getLangOpts());
-
-  for (const auto  : checkerRegistrationFns)
-Fn(allCheckers);
+  CheckerRegistry allCheckers(plugins, diags, opts, context.getLangOpts(),
+  checkerRegistrationFns);
 
   allCheckers.initializeManager(*checkerMgr);
   allCheckers.validateCheckerOptions();

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp?rev=352284=352283=352284=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp Sat Jan 26 
09:27:40 2019
@@ -91,10 +91,11 @@ CheckerRegistry::getMutableCheckersForCm
   return { it, it + size };
 }
 
-CheckerRegistry::CheckerRegistry(ArrayRef plugins,
- DiagnosticsEngine ,
- AnalyzerOptions ,
- const LangOptions )
+CheckerRegistry::CheckerRegistry(
+ ArrayRef plugins, DiagnosticsEngine ,
+ AnalyzerOptions , const LangOptions ,
+ ArrayRef>
+ checkerRegistrationFns)
   : Diags(diags), AnOpts(AnOpts), LangOpts(LangOpts) {
 
   // Register builtin checkers.
@@ -137,6 +138,13 @@ CheckerRegistry::CheckerRegistry(ArrayRe
   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. 
+
+  for (const auto  : checkerRegistrationFns)
+Fn(*this);
+
   // Sort checkers for efficient collection.
   // FIXME: Alphabetical sort puts 'experimental' in the middle.
   // Would it be better to name it '~experimental' or something else


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

[PATCH] D56989: [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352282: [analyzer][NFC] Keep track of whether enabling a 
checker was explictly… (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56989?vs=182724=183705#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56989

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

Index: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -38,12 +38,66 @@
   return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
 }
 
+static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
+  const CheckerRegistry::CheckerInfo ) {
+  return a.FullName < b.FullName;
+}
+
+static constexpr char PackageSeparator = '.';
+
+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))
+return false;
+
+  // Is the package actually just the name of a specific checker?
+  if (checker.FullName.size() == packageName.size())
+return true;
+
+  // Is the checker in the package (or a subpackage)?
+  if (checker.FullName[packageName.size()] == PackageSeparator)
+return true;
+
+  return false;
+}
+
+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);
+
+  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);
+
+  if (packageSize != Packages.end())
+size = packageSize->getValue();
+
+  return { it, it + size };
+}
+
 CheckerRegistry::CheckerRegistry(ArrayRef plugins,
  DiagnosticsEngine ,
  AnalyzerOptions ,
  const LangOptions )
   : Diags(diags), AnOpts(AnOpts), LangOpts(LangOpts) {
 
+  // Register builtin checkers.
 #define GET_CHECKERS
 #define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI)\
   addChecker(register##CLASS, shouldRegister##CLASS, FULLNAME, HELPTEXT,   \
@@ -52,6 +106,7 @@
 #undef CHECKER
 #undef GET_CHECKERS
 
+  // Register checkers from plugins.
   for (ArrayRef::iterator i = plugins.begin(), e = plugins.end();
i != e; ++i) {
 // Get access to the plugin.
@@ -81,73 +136,38 @@
 if (registerPluginCheckers)
   registerPluginCheckers(*this);
   }
-}
-
-static constexpr char PackageSeparator = '.';
-
-static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
-  const CheckerRegistry::CheckerInfo ) {
-  return a.FullName < b.FullName;
-}
 
-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))
-return false;
+  // Sort checkers for efficient collection.
+  // FIXME: Alphabetical sort puts 'experimental' in the middle.
+  // Would it be better to name it '~experimental' or something else
+  // that's ASCIIbetically last?
+  llvm::sort(Checkers, checkerNameLT);
 
-  // Is the package actually just the name of a specific checker?
-  if (checker.FullName.size() == packageName.size())
-return true;
+  // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the
+  // command line.
+  for (const std::pair  : AnOpts.CheckersControlList) {
+CheckerInfoListRange checkersForCmdLineArg =
+ getMutableCheckersForCmdLineArg(opt.first);
 
-  // Is the checker in the package (or a subpackage)?
-  if (checker.FullName[packageName.size()] == PackageSeparator)
-return true;
+if (checkersForCmdLineArg.begin() == checkersForCmdLineArg.end()) {
+  Diags.Report(diag::err_unknown_analyzer_checker) << opt.first;
+  

r352282 - [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 08:35:33 2019
New Revision: 352282

URL: http://llvm.org/viewvc/llvm-project?rev=352282=rev
Log:
[analyzer][NFC] Keep track of whether enabling a checker was explictly 
specified in command line arguments

I added a new enum to CheckerInfo, so we can easily track whether the check is
explicitly enabled, explicitly disabled, or isn't specified in this regard.
Checkers belonging in the latter category may be implicitly enabled through
dependencies in the followup patch. I also made sure that this is done within
CheckerRegisty's constructor, leading to very significant simplifications in
its query-like methods.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h?rev=352282=352281=352282=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h Sat Jan 
26 08:35:33 2019
@@ -90,11 +90,25 @@ public:
   using ShouldRegisterFunction = bool (*)(const LangOptions &);
 
   struct CheckerInfo {
+enum class StateFromCmdLine {
+  // This checker wasn't explicitly enabled or disabled.
+  State_Unspecified,
+  // This checker was explicitly disabled.
+  State_Disabled,
+  // This checker was explicitly enabled.
+  State_Enabled
+};
+
 InitializationFunction Initialize;
 ShouldRegisterFunction ShouldRegister;
 StringRef FullName;
 StringRef Desc;
 StringRef DocumentationUri;
+StateFromCmdLine State = StateFromCmdLine::State_Unspecified;
+
+bool isEnabled(const LangOptions ) const {
+  return State == StateFromCmdLine::State_Enabled && ShouldRegister(LO);
+}
 
 CheckerInfo(InitializationFunction Fn, ShouldRegisterFunction sfn,
 StringRef Name, StringRef Desc, StringRef DocsUri)
@@ -102,7 +116,9 @@ public:
   DocumentationUri(DocsUri) {}
   };
 
+  using StateFromCmdLine = CheckerInfo::StateFromCmdLine;
   using CheckerInfoList = std::vector;
+  using CheckerInfoListRange = llvm::iterator_range;
   using CheckerInfoSet = llvm::SetVector;
 
 private:
@@ -133,6 +149,7 @@ public:
::returnTrue, FullName, Desc, DocsUri);
   }
 
+  // FIXME: This *really* should be added to the frontend flag descriptions.
   /// Initializes a CheckerManager by calling the initialization functions for
   /// all checkers specified by the given CheckerOptInfo list. The order of 
this
   /// list is significant; later options can be used to reverse earlier ones.
@@ -150,8 +167,13 @@ public:
 private:
   CheckerInfoSet getEnabledCheckers() const;
 
-  mutable CheckerInfoList Checkers;
-  mutable llvm::StringMap Packages;
+  /// Return an iterator range of mutable CheckerInfos \p CmdLineArg applies 
to.
+  /// For example, it'll return the checkers for the core package, if
+  /// \p CmdLineArg is "core".
+  CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
+
+  CheckerInfoList Checkers;
+  llvm::StringMap Packages;
 
   DiagnosticsEngine 
   AnalyzerOptions 

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp?rev=352282=352281=352282=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp Sat Jan 26 
08:35:33 2019
@@ -38,12 +38,66 @@ static bool isCompatibleAPIVersion(const
   return strcmp(versionString, CLANG_ANALYZER_API_VERSION_STRING) == 0;
 }
 
+static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
+  const CheckerRegistry::CheckerInfo ) {
+  return a.FullName < b.FullName;
+}
+
+static constexpr char PackageSeparator = '.';
+
+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))
+return false;
+
+  // Is the package actually just the name of a specific checker?
+  if (checker.FullName.size() == packageName.size())
+return true;
+
+  // Is the checker in the package (or a subpackage)?
+  if (checker.FullName[packageName.size()] == PackageSeparator)
+return true;
+
+  return false;
+}
+
+CheckerRegistry::CheckerInfoListRange
+CheckerRegistry::getMutableCheckersForCmdLineArg(StringRef CmdLineArg) {
+
+  assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
+ "In 

[PATCH] D56989: [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

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

Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:143
+  // Would it be better to name it '~experimental' or something else
+  // that's ASCIIbetically last?
+  llvm::sort(Checkers, checkerNameLT);

NoQ wrote:
> I learned a new word today :) :)
Wish it came from me, but I just moved it around... put a smile on my face 
every time I came across it too though :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D56989



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

[PATCH] D57104: [AST] Pack GenericSelectionExpr

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

I was just working on this this afternoon to show you the difference when it is 
split up. Looks like you committed this meanwhile.

While building I noticed that you introduced a `-Wreorder` bug in D57238 
, which isn't surprising given how many 
different things you do in that one commit :). It looks like you fixed that bug 
before commit though.

Thanks for the refactor!

Stephen.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57104



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

[PATCH] D56988: [analyzer][NFC] Supply CheckerRegistry with AnalyzerOptions

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352279: [analyzer][NFC] Supply CheckerRegistry with 
AnalyzerOptions (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56988?vs=182722=183704#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56988

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -33,35 +33,36 @@
 DiagnosticsEngine ) {
   auto checkerMgr = llvm::make_unique(context, opts);
 
-  CheckerRegistry allCheckers(plugins, diags, context.getLangOpts());
+  CheckerRegistry allCheckers(plugins, diags, opts, context.getLangOpts());
 
   for (const auto  : checkerRegistrationFns)
 Fn(allCheckers);
 
-  allCheckers.initializeManager(*checkerMgr, opts);
-  allCheckers.validateCheckerOptions(opts);
+  allCheckers.initializeManager(*checkerMgr);
+  allCheckers.validateCheckerOptions();
   checkerMgr->finishedCheckerRegistration();
 
   return checkerMgr;
 }
 
 void ento::printCheckerHelp(raw_ostream , ArrayRef plugins,
+AnalyzerOptions ,
 DiagnosticsEngine ,
 const LangOptions ) {
   out << "OVERVIEW: Clang Static Analyzer Checkers List\n\n";
   out << "USAGE: -analyzer-checker \n\n";
 
-  CheckerRegistry(plugins, diags, langOpts).printHelp(out);
+  CheckerRegistry(plugins, diags, anopts, langOpts).printHelp(out);
 }
 
 void ento::printEnabledCheckerList(raw_ostream ,
ArrayRef plugins,
-   const AnalyzerOptions ,
+   AnalyzerOptions ,
DiagnosticsEngine ,
const LangOptions ) {
   out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
 
-  CheckerRegistry(plugins, diags, langOpts).printList(out, opts);
+  CheckerRegistry(plugins, diags, anopts, langOpts).printList(out);
 }
 
 void ento::printAnalyzerConfigList(raw_ostream ) {
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -40,8 +40,9 @@
 
 CheckerRegistry::CheckerRegistry(ArrayRef plugins,
  DiagnosticsEngine ,
+ AnalyzerOptions ,
  const LangOptions )
-  : Diags(diags), LangOpts(LangOpts) {
+  : Diags(diags), AnOpts(AnOpts), LangOpts(LangOpts) {
 
 #define GET_CHECKERS
 #define CHECKER(FULLNAME, CLASS, HELPTEXT, DOC_URI)\
@@ -106,8 +107,7 @@
   return false;
 }
 
-CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers(
-const AnalyzerOptions ) const {
+CheckerRegistry::CheckerInfoSet CheckerRegistry::getEnabledCheckers() const {
 
   assert(std::is_sorted(Checkers.begin(), Checkers.end(), checkerNameLT) &&
  "In order to efficiently gather checkers, this function expects them "
@@ -116,7 +116,7 @@
   CheckerInfoSet enabledCheckers;
   const auto end = Checkers.cend();
 
-  for (const std::pair  : Opts.CheckersControlList) {
+  for (const std::pair  : AnOpts.CheckersControlList) {
 // Use a binary search to find the possible start of the package.
 CheckerRegistry::CheckerInfo
 packageInfo(nullptr, nullptr, opt.first, "", "");
@@ -167,13 +167,12 @@
   }
 }
 
-void CheckerRegistry::initializeManager(CheckerManager ,
-const AnalyzerOptions ) const {
+void CheckerRegistry::initializeManager(CheckerManager ) const {
   // Sort checkers for efficient collection.
   llvm::sort(Checkers, checkerNameLT);
 
   // Collect checkers enabled by the options.
-  CheckerInfoSet enabledCheckers = getEnabledCheckers(Opts);
+  CheckerInfoSet enabledCheckers = getEnabledCheckers();
 
   // Initialize the CheckerManager with all enabled checkers.
   for (const auto *i : enabledCheckers) {
@@ -182,9 +181,8 @@
   }
 }
 
-void CheckerRegistry::validateCheckerOptions(
-const AnalyzerOptions ) const {
-  for (const auto  : opts.Config) {
+void CheckerRegistry::validateCheckerOptions() 

r352279 - [analyzer][NFC] Supply CheckerRegistry with AnalyzerOptions

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 07:59:21 2019
New Revision: 352279

URL: http://llvm.org/viewvc/llvm-project?rev=352279=rev
Log:
[analyzer][NFC] Supply CheckerRegistry with AnalyzerOptions

Since pretty much all methods of CheckerRegistry has AnalyzerOptions as an
argument, it makes sense to just simply require it in it's constructor.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h?rev=352279=352278=352279=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h Sat Jan 
26 07:59:21 2019
@@ -82,7 +82,7 @@ namespace ento {
 class CheckerRegistry {
 public:
   CheckerRegistry(ArrayRef plugins, DiagnosticsEngine ,
-  const LangOptions );
+  AnalyzerOptions , const LangOptions );
 
   /// Initialization functions perform any necessary setup for a checker.
   /// They should include a call to CheckerManager::registerChecker.
@@ -137,24 +137,24 @@ public:
   /// all checkers specified by the given CheckerOptInfo list. The order of 
this
   /// list is significant; later options can be used to reverse earlier ones.
   /// This can be used to exclude certain checkers in an included package.
-  void initializeManager(CheckerManager ,
- const AnalyzerOptions ) const;
+  void initializeManager(CheckerManager ) const;
 
   /// Check if every option corresponds to a specific checker or package.
-  void validateCheckerOptions(const AnalyzerOptions ) const;
+  void validateCheckerOptions() const;
 
   /// Prints the name and description of all checkers in this registry.
   /// This output is not intended to be machine-parseable.
   void printHelp(raw_ostream , size_t maxNameChars = 30) const;
-  void printList(raw_ostream , const AnalyzerOptions ) const;
+  void printList(raw_ostream ) const;
 
 private:
-  CheckerInfoSet getEnabledCheckers(const AnalyzerOptions ) const;
+  CheckerInfoSet getEnabledCheckers() const;
 
   mutable CheckerInfoList Checkers;
   mutable llvm::StringMap Packages;
 
   DiagnosticsEngine 
+  AnalyzerOptions 
   const LangOptions 
 };
 

Modified: cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h?rev=352279=352278=352279=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h Sat Jan 
26 07:59:21 2019
@@ -51,10 +51,13 @@ private:
   llvm::StringMap 
 };
 
-void printCheckerHelp(raw_ostream , ArrayRef plugins,
-  DiagnosticsEngine , const LangOptions );
+void printCheckerHelp(raw_ostream ,
+  ArrayRef plugins,
+  AnalyzerOptions ,
+  DiagnosticsEngine ,
+  const LangOptions );
 void printEnabledCheckerList(raw_ostream , ArrayRef plugins,
- const AnalyzerOptions ,
+ AnalyzerOptions ,
  DiagnosticsEngine ,
  const LangOptions );
 void printAnalyzerConfigList(raw_ostream );

Modified: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp?rev=352279=352278=352279=diff
==
--- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp (original)
+++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp Sat Jan 26 
07:59:21 2019
@@ -237,8 +237,11 @@ bool ExecuteCompilerInvocation(CompilerI
   // Honor -analyzer-checker-help.
   // This should happen AFTER plugins have been loaded!
   if (Clang->getAnalyzerOpts()->ShowCheckerHelp) {
-ento::printCheckerHelp(llvm::outs(), Clang->getFrontendOpts().Plugins,
-   Clang->getDiagnostics(), Clang->getLangOpts());
+ento::printCheckerHelp(llvm::outs(),
+   Clang->getFrontendOpts().Plugins,
+   *Clang->getAnalyzerOpts(),
+   Clang->getDiagnostics(),
+   Clang->getLangOpts());
 return true;
   }
 

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352278: [analyzer] Split unix.API up to UnixAPIMisuseChecker 
and… (authored by Szelethus, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55425?vs=177168=183703#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55425

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
  cfe/trunk/test/Analysis/Inputs/expected-plists/unix-fns.c.plist

Index: cfe/trunk/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
===
--- cfe/trunk/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
+++ cfe/trunk/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
@@ -3,7 +3,7 @@
 
 
  clang_version
-clang version 8.0.0 
+clang version 8.0.0
  diagnostics
  
   
@@ -744,9 +744,9 @@
descriptionCall to malloc has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_context0e841458f0cb7cf161d35f9db5862dcf
+   issue_hash_content_of_line_in_context4ddbefeb3fa802a0636dc63d679bdc89
   issue_context_kindfunction
   issue_contextpr2899
   issue_hash_function_offset1
@@ -835,9 +835,9 @@
descriptionCall to calloc has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_contexta267ff573c7e8b959a3f886677893eb0
+   issue_hash_content_of_line_in_context9f12ad2f0a645cb7e4485fed526f536e
   issue_context_kindfunction
   issue_contexttest_calloc
   issue_hash_function_offset1
@@ -926,9 +926,9 @@
descriptionCall to calloc has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_context14eb72957baab3c63bac610a10e6f48b
+   issue_hash_content_of_line_in_context835b2375daee5b05ac48f24ac578de4c
   issue_context_kindfunction
   issue_contexttest_calloc2
   issue_hash_function_offset1
@@ -1017,9 +1017,9 @@
descriptionCall to realloc has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_context7f6f67ebe3d481aed7750005bea7e371
+   issue_hash_content_of_line_in_contextbbdabcb6c5a3783012ae34bfea2a10fb
   issue_context_kindfunction
   issue_contexttest_realloc
   issue_hash_function_offset1
@@ -1108,9 +1108,9 @@
descriptionCall to reallocf has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_context4941698efbd81601653dff10ef9c645b
+   issue_hash_content_of_line_in_context5d222055bbf58b08ec345f0ebfd7b9d1
   issue_context_kindfunction
   issue_contexttest_reallocf
   issue_hash_function_offset1
@@ -1199,9 +1199,9 @@
descriptionCall to alloca has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_contextb7ca3488e81d9d9d4b8dc545258ce97c
+   issue_hash_content_of_line_in_contextf7bdefde93c0a58ec236918fb0c3a54e
   issue_context_kindfunction
   issue_contexttest_alloca
   issue_hash_function_offset1
@@ -1290,9 +1290,9 @@
descriptionCall to alloca has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_context1ec52551362b070237f47f6bb6c3847d
+   issue_hash_content_of_line_in_context4247526f8da82479508f2d364c2992d5
   issue_context_kindfunction
   issue_contexttest_builtin_alloca
   issue_hash_function_offset1
@@ -1381,9 +1381,9 @@
descriptionCall to valloc has an allocation size of 0 bytes
categoryUnix API
typeUndefined allocation of 0 bytes (CERT MEM04-C; CWE-131)
-   check_nameunix.API
+   check_nameoptin.portability.UnixAPI

-   issue_hash_content_of_line_in_context675741e04c8d0071d280324e23f41d35
+   issue_hash_content_of_line_in_contexte16dfa9598fd2fafe6dc5563990c1dd3
   issue_context_kindfunction
   issue_contexttest_valloc
   issue_hash_function_offset1
@@ -3015,7 +3015,7 @@
  
  files
  
-  /clang/test/Analysis/unix-fns.c
+  /home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c
  
 
 
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

r352278 - [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 07:56:40 2019
New Revision: 352278

URL: http://llvm.org/viewvc/llvm-project?rev=352278=rev
Log:
[analyzer] Split unix.API up to UnixAPIMisuseChecker and 
UnixAPIPortabilityChecker

The actual implementation of unix.API features a dual-checker: two checkers in
one, even though they don't even interact at all. Split them up, as this is a
problem for establishing dependencies.

I added no new code at all, just merely moved it around.

Since the plist files change (and that's a benefit!) this patch isn't NFC.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
cfe/trunk/test/Analysis/Inputs/expected-plists/unix-fns.c.plist

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp?rev=352278=352277=352278=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp Sat Jan 26 
07:56:40 2019
@@ -20,10 +20,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/raw_ostream.h"
-#include 
 
 using namespace clang;
 using namespace ento;
@@ -39,8 +36,9 @@ enum class OpenVariant {
 };
 
 namespace {
-class UnixAPIChecker : public Checker< check::PreStmt > {
-  mutable std::unique_ptr BT_open, BT_pthreadOnce, BT_mallocZero;
+
+class UnixAPIMisuseChecker : public Checker< check::PreStmt > {
+  mutable std::unique_ptr BT_open, BT_pthreadOnce;
   mutable Optional Val_O_CREAT;
 
 public:
@@ -50,8 +48,25 @@ public:
 
   void CheckOpen(CheckerContext , const CallExpr *CE) const;
   void CheckOpenAt(CheckerContext , const CallExpr *CE) const;
-
   void CheckPthreadOnce(CheckerContext , const CallExpr *CE) const;
+
+  void CheckOpenVariant(CheckerContext ,
+const CallExpr *CE, OpenVariant Variant) const;
+
+  void ReportOpenBug(CheckerContext ,
+ ProgramStateRef State,
+ const char *Msg,
+ SourceRange SR) const;
+
+};
+
+class UnixAPIPortabilityChecker : public Checker< check::PreStmt > {
+public:
+  void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
+
+private:
+  mutable std::unique_ptr BT_mallocZero;
+
   void CheckCallocZero(CheckerContext , const CallExpr *CE) const;
   void CheckMallocZero(CheckerContext , const CallExpr *CE) const;
   void CheckReallocZero(CheckerContext , const CallExpr *CE) const;
@@ -60,13 +75,6 @@ public:
   void CheckAllocaWithAlignZero(CheckerContext , const CallExpr *CE) const;
   void CheckVallocZero(CheckerContext , const CallExpr *CE) const;
 
-  typedef void (UnixAPIChecker::*SubChecker)(CheckerContext &,
- const CallExpr *) const;
-private:
-
-  void CheckOpenVariant(CheckerContext ,
-const CallExpr *CE, OpenVariant Variant) const;
-
   bool ReportZeroByteAllocation(CheckerContext ,
 ProgramStateRef falseState,
 const Expr *arg,
@@ -76,48 +84,75 @@ private:
 const unsigned numArgs,
 const unsigned sizeArg,
 const char *fn) const;
-  void LazyInitialize(std::unique_ptr , const char *name) const {
-if (BT)
-  return;
-BT.reset(new BugType(this, name, categories::UnixAPI));
-  }
-  void ReportOpenBug(CheckerContext ,
- ProgramStateRef State,
- const char *Msg,
- SourceRange SR) const;
 };
+
 } //end anonymous namespace
 
+static void LazyInitialize(const CheckerBase *Checker,
+   std::unique_ptr ,
+   const char *name) {
+  if (BT)
+return;
+  BT.reset(new BugType(Checker, name, categories::UnixAPI));
+}
+
 
//===--===//
 // "open" (man 2 open)
-//===--===//
+//===--===/
+
+void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+CheckerContext ) const {
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (!FD || FD->getKind() != Decl::Function)
+return;
+
+  // Don't treat functions in namespaces with the same name a Unix function
+  // as a call to the Unix function.
+  const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext();
+  if (NamespaceCtx && isa(NamespaceCtx))
+return;
 
-void UnixAPIChecker::ReportOpenBug(CheckerContext ,
-   ProgramStateRef 

[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 183702.
xazax.hun added a comment.

- Added some tests


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

https://reviews.llvm.org/D57230

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/call-invalidation.cpp
  test/Analysis/cxx-uninitialized-object.cpp
  test/Analysis/malloc.c
  test/Analysis/taint-generic.c
  test/Analysis/taint-tester.c

Index: test/Analysis/taint-tester.c
===
--- test/Analysis/taint-tester.c
+++ test/Analysis/taint-tester.c
@@ -51,7 +51,7 @@
   scanf("%d", );
   scanf("%d", );
   int tx = xy.x; // expected-warning + {{tainted}}
-  int ty = xy.y; // FIXME: This should be tainted as well.
+  int ty = xy.y; // expected-warning + {{tainted}}
   char ntz = xy.z;// no warning
   // Now, scanf scans both.
   scanf("%d %d", , );
Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -231,6 +231,7 @@
 
   int sock = socket(AF_INET, SOCK_STREAM, 0);
   read(sock, , sizeof(tainted.y));
+  tainted.x = 0;
   // FIXME: overlapping regions aren't detected by isTainted yet
   __builtin_memcpy(buffer, tainted.y, tainted.x);
 }
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1758,8 +1758,8 @@
 void testConstEscapeThroughAnotherField() {
   struct IntAndPtr s;
   s.p = malloc(sizeof(int));
-  constEscape(&(s.x)); // could free s->p!
-} // no-warning
+  constEscape(&(s.x));
+} // expected-warning {{Potential leak of memory pointed to by 's.p'}}
 
 // PR15623
 int testNoCheckerDataPropogationFromLogicalOpOperandToOpResult(void) {
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -358,7 +358,7 @@
 void wontInitialize(const T &);
 
 class PassingToUnknownFunctionTest1 {
-  int a, b;
+  int a, b; // expected-note{{uninitialized field 'this->b'}}
 
 public:
   PassingToUnknownFunctionTest1() {
@@ -368,8 +368,7 @@
   }
 
   PassingToUnknownFunctionTest1(int) {
-mayInitialize(a);
-// All good!
+mayInitialize(a); // expected-warning{{1 uninitialized field at the end of the constructor call}}
   }
 
   PassingToUnknownFunctionTest1(int, int) {
Index: test/Analysis/call-invalidation.cpp
===
--- test/Analysis/call-invalidation.cpp
+++ test/Analysis/call-invalidation.cpp
@@ -132,18 +132,21 @@
   PlainStruct s1;
   s1.x = 1;
   s1.z = 1;
+  s1.y = 1;
   clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
   clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
   // Not only passing a structure pointer through const pointer parameter,
   // but also passing a field pointer through const pointer parameter
   // should preserve the contents of the structure.
   useAnythingConst(&(s1.y));
+  clang_analyzer_eval(s1.y == 1); // expected-warning{{TRUE}}
   clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
   // FIXME: Should say "UNKNOWN", because it is not uncommon to
   // modify a mutable member variable through const pointer.
   clang_analyzer_eval(s1.z == 1); // expected-warning{{TRUE}}
   useAnything(&(s1.y));
-  clang_analyzer_eval(s1.x == 1); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(s1.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(s1.y == 1); // expected-warning{{UNKNOWN}}
 }
 
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -303,11 +303,23 @@
   for (unsigned Idx = 0, Count = getNumArgs(); Idx != Count; ++Idx) {
 // Mark this region for invalidation.  We batch invalidate regions
 // below for efficiency.
-if (PreserveArgs.count(Idx))
-  if (const MemRegion *MR = getArgSVal(Idx).getAsRegion())
-ETraits.setTrait(MR->getBaseRegion(),
-RegionAndSymbolInvalidationTraits::TK_PreserveContents);
-// TODO: Factor this out + handle the lower level const pointers.
+if (const MemRegion *MR = getArgSVal(Idx).getAsRegion()) {
+  bool UseBaseRegion = true;
+  if (const auto *FR = MR->getAs()) {
+if (const auto *TVR = FR->getSuperRegion()->getAs()) {
+  if (!TVR->getValueType()->isUnionType()) {
+ETraits.setTrait(MR, RegionAndSymbolInvalidationTraits::
+ TK_DoNotInvalidateSuperRegion);
+UseBaseRegion = false;
+  }
+}
+  }
+  // todo: factor this out + handle the lower level const pointers.
+  if (PreserveArgs.count(Idx))
+ETraits.setTrait(
+

[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 2 inline comments as done.
xazax.hun added a comment.

In D57230#1372275 , @NoQ wrote:

> Could you share reproducible examples for these, probably in the form of 
> FIXME tests? Given that they are "regressions", they are easy to creduce down 
> to a small repro by using the test "there is still a change in behavior on 
> this file".


I think the most common cause of false positives is infeasible paths. Do you 
have success reducing false positives using creduce? My problem usually is that 
we cannot tell if a reduction rendered a false positive into a true positive.




Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:320-321
+ETraits.setTrait(
+UseBaseRegion ? MR->getBaseRegion() : MR,
+RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+}

NoQ wrote:
> I suspect that the trait for non-base `MR` would never be read. The only 
> place where this trait is accessed is in RegionStore.cpp where it asks 
> whether the trait is applied to a cluster base, which is always a base region.
I see some test failures when I always used the base region. I suspect the 
reason is that `InvalidateRegionsWorker::AddToWorkList` will add the region 
itself instead of the base region when `TK_DoNotInvalidateSuperRegion` is set. 
So if we only set the `TK_PreserveContents` trait for the base region 
`InvalidateRegionsWorker::VisitCluster` will not see the `TK_PreserveContents` 
trait. 

In fact, the naming of regions in the those functions are very confusing. Even 
though the formal paramter is called `baseR`, my suspicion is that, we might 
visit non-base regions (due to the  `TK_DoNotInvalidateSuperRegion`  trait). 


Repository:
  rC Clang

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

https://reviews.llvm.org/D57230



___
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 module and a check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 183701.
lebedev.ri added a comment.
Herald added a subscriber: arphaman.

Mention new module in `docs/clang-tidy/index.rst`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.h
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/openmp-use-default-none.rst
  docs/clang-tidy/index.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' 

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

New module is still not mentioned in docs/clang-tidy/index.rst.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113



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

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC352277: [analyzer] Supply all checkers with a shouldRegister 
function (authored by Szelethus, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D55424

Files:
  include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h
  include/clang/StaticAnalyzer/Core/Checker.h
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
  lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
  lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
  lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
  lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp
  lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
  lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
  lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
  lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
  lib/StaticAnalyzer/Checkers/GTestChecker.cpp
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
  lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
  lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
  lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
  lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
  lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
  lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
  lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp
  lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp
  lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
  lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp
  lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

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

Szelethus added a comment.

In D55424#1364696 , @Szelethus wrote:

> In D55424#1326397 , @NoQ wrote:
>
> > Should we also pass `CheckerManager` into `shouldRegister...`? Or is it 
> > entirely useless?
>
>
> I wouldn't say useless, but I'm struggling to come up with an example where 
> registering would depend on non-language dependent reasons. I'm stuck on 
> rebasing my patches, so I'll give it a thought whether it's worth the chore 
> to change everything (but where I'd like to leave a system for the long term, 
> doing extra work shouldn't be a major concern).


For now, I decided against it just to make progress with the project, but if 
the need arises, I'll be happy to change it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55424



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

r352277 - [analyzer] Supply all checkers with a shouldRegister function

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Sat Jan 26 06:23:08 2019
New Revision: 352277

URL: http://llvm.org/viewvc/llvm-project?rev=352277=rev
Log:
[analyzer] Supply all checkers with a shouldRegister function

Introduce the boolean ento::shouldRegister##CHECKERNAME(const LangOptions )
function very similarly to ento::register##CHECKERNAME. This will force every
checker to implement this function, but maybe it isn't that bad: I saw a lot of
ObjC or C++ specific checkers that should probably not register themselves based
on some LangOptions (mine too), but they do anyways.

A big benefit of this is that all registry functions now register their checker,
once it is called, registration is guaranteed.

This patch is a part of a greater effort to reinvent checker registration, more
info here: D54438#1315953

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h
cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
cfe/trunk/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
cfe/trunk/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 183699.
lebedev.ri added a comment.

Adjust a few comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.h
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  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' kind, all 

[PATCH] D57104: [AST] Pack GenericSelectionExpr

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352276: [AST] Pack GenericSelectionExpr (authored by 
brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57104?vs=183349=183698#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57104

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprObjC.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/Sema/SemaPseudoObject.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
  cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Index: cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
+++ cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
@@ -968,18 +968,24 @@
 
 void ASTStmtWriter::VisitGenericSelectionExpr(GenericSelectionExpr *E) {
   VisitExpr(E);
+
   Record.push_back(E->getNumAssocs());
   Record.push_back(E->ResultIndex);
-
-  Record.AddStmt(E->getControllingExpr());
-  for (unsigned I = 0, N = E->getNumAssocs(); I != N; ++I) {
-Record.AddTypeSourceInfo(E->getAssocTypeSourceInfo(I));
-Record.AddStmt(E->getAssocExpr(I));
-  }
-
   Record.AddSourceLocation(E->getGenericLoc());
   Record.AddSourceLocation(E->getDefaultLoc());
   Record.AddSourceLocation(E->getRParenLoc());
+
+  Stmt **Stmts = E->getTrailingObjects();
+  // Add 1 to account for the controlling expression which is the first
+  // expression in the trailing array of Stmt *. This is not needed for
+  // the trailing array of TypeSourceInfo *.
+  for (unsigned I = 0, N = E->getNumAssocs() + 1; I < N; ++I)
+Record.AddStmt(Stmts[I]);
+
+  TypeSourceInfo **TSIs = E->getTrailingObjects();
+  for (unsigned I = 0, N = E->getNumAssocs(); I < N; ++I)
+Record.AddTypeSourceInfo(TSIs[I]);
+
   Code = serialization::EXPR_GENERIC_SELECTION;
 }
 
Index: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
@@ -1022,23 +1022,24 @@
 
 void ASTStmtReader::VisitGenericSelectionExpr(GenericSelectionExpr *E) {
   VisitExpr(E);
-  E->NumAssocs = Record.readInt();
-  E->ResultIndex = Record.readInt();
-
-  E->AssocTypes = new (Record.getContext()) TypeSourceInfo *[E->NumAssocs];
-  E->SubExprs = new (Record.getContext())
-  Stmt *[GenericSelectionExpr::AssocExprStartIndex + E->NumAssocs];
-
-  E->SubExprs[GenericSelectionExpr::ControllingIndex] = Record.readSubExpr();
-  for (unsigned I = 0, N = E->getNumAssocs(); I != N; ++I) {
-E->AssocTypes[I] = GetTypeSourceInfo();
-E->SubExprs[GenericSelectionExpr::AssocExprStartIndex + I] =
-Record.readSubExpr();
-  }
 
-  E->GenericLoc = ReadSourceLocation();
+  unsigned NumAssocs = Record.readInt();
+  assert(NumAssocs == E->getNumAssocs() && "Wrong NumAssocs!");
+  E->ResultIndex = Record.readInt();
+  E->GenericSelectionExprBits.GenericLoc = ReadSourceLocation();
   E->DefaultLoc = ReadSourceLocation();
   E->RParenLoc = ReadSourceLocation();
+
+  Stmt **Stmts = E->getTrailingObjects();
+  // Add 1 to account for the controlling expression which is the first
+  // expression in the trailing array of Stmt *. This is not needed for
+  // the trailing array of TypeSourceInfo *.
+  for (unsigned I = 0, N = NumAssocs + 1; I < N; ++I)
+Stmts[I] = Record.readSubExpr();
+
+  TypeSourceInfo **TSIs = E->getTrailingObjects();
+  for (unsigned I = 0, N = NumAssocs; I < N; ++I)
+TSIs[I] = GetTypeSourceInfo();
 }
 
 void ASTStmtReader::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
@@ -2677,7 +2678,9 @@
   break;
 
 case EXPR_GENERIC_SELECTION:
-  S = new (Context) GenericSelectionExpr(Empty);
+  S = GenericSelectionExpr::CreateEmpty(
+  Context,
+  /*NumAssocs=*/Record[ASTStmtReader::NumExprFields]);
   break;
 
 case EXPR_OBJC_STRING_LITERAL:
Index: cfe/trunk/lib/AST/Expr.cpp
===
--- cfe/trunk/lib/AST/Expr.cpp
+++ cfe/trunk/lib/AST/Expr.cpp
@@ -3775,7 +3775,7 @@
 }
 
 GenericSelectionExpr::GenericSelectionExpr(
-const ASTContext , SourceLocation GenericLoc, Expr *ControllingExpr,
+const ASTContext &, SourceLocation GenericLoc, Expr *ControllingExpr,
 ArrayRef AssocTypes, ArrayRef AssocExprs,
 SourceLocation DefaultLoc, SourceLocation RParenLoc,
 bool ContainsUnexpandedParameterPack, unsigned ResultIndex)
@@ -3787,18 +3787,18 @@
AssocExprs[ResultIndex]->isInstantiationDependent(),
ContainsUnexpandedParameterPack),
   NumAssocs(AssocExprs.size()), ResultIndex(ResultIndex),
-  AssocTypes(new (Context) TypeSourceInfo 

r352276 - [AST] Pack GenericSelectionExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Sat Jan 26 06:15:10 2019
New Revision: 352276

URL: http://llvm.org/viewvc/llvm-project?rev=352276=rev
Log:
[AST] Pack GenericSelectionExpr

Store the controlling expression, the association expressions and the
corresponding TypeSourceInfos as trailing objects.

Additionally use the bit-fields of Stmt to store one SourceLocation,
saving one additional pointer. This saves 3 pointers in total per
GenericSelectionExpr.

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

Reviewed By: aaron.ballman

Reviewers: aaron.ballman, steveire


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=352276=352275=352276=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Sat Jan 26 06:15:10 2019
@@ -5013,9 +5013,13 @@ public:
 /// which names a dependent type in its association list is result-dependent,
 /// which means that the choice of result expression is dependent.
 /// Result-dependent generic associations are both type- and value-dependent.
-class GenericSelectionExpr : public Expr {
+class GenericSelectionExpr final
+: public Expr,
+  private llvm::TrailingObjects {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
+  friend TrailingObjects;
 
   /// The number of association expressions and the index of the result
   /// expression in the case where the generic selection expression is not
@@ -5028,13 +5032,27 @@ class GenericSelectionExpr : public Expr
 AssocExprStartIndex = 1
   };
 
-  /// The location of the "_Generic", "default" and of the right parenthesis.
-  SourceLocation GenericLoc, DefaultLoc, RParenLoc;
+  /// The location of the "default" and of the right parenthesis.
+  SourceLocation DefaultLoc, RParenLoc;
 
-  TypeSourceInfo **AssocTypes;
-  Stmt **SubExprs;
+  // GenericSelectionExpr is followed by several trailing objects.
+  // They are (in order):
+  //
+  // * A single Stmt * for the controlling expression.
+  // * An array of getNumAssocs() Stmt * for the association expressions.
+  // * An array of getNumAssocs() TypeSourceInfo *, one for each of the
+  //   association expressions.
+  unsigned numTrailingObjects(OverloadToken) const {
+// Add one to account for the controlling expression; the remainder
+// are the associated expressions.
+return 1 + getNumAssocs();
+  }
 
-public:
+  unsigned numTrailingObjects(OverloadToken) const {
+return getNumAssocs();
+  }
+
+  /// Build a non-result-dependent generic selection expression.
   GenericSelectionExpr(const ASTContext , SourceLocation GenericLoc,
Expr *ControllingExpr,
ArrayRef AssocTypes,
@@ -5043,7 +5061,7 @@ public:
bool ContainsUnexpandedParameterPack,
unsigned ResultIndex);
 
-  /// This constructor is used in the result-dependent case.
+  /// Build a result-dependent generic selection expression.
   GenericSelectionExpr(const ASTContext , SourceLocation GenericLoc,
Expr *ControllingExpr,
ArrayRef AssocTypes,
@@ -5051,8 +5069,28 @@ public:
SourceLocation RParenLoc,
bool ContainsUnexpandedParameterPack);
 
-  explicit GenericSelectionExpr(EmptyShell Empty)
-  : Expr(GenericSelectionExprClass, Empty) {}
+  /// Build an empty generic selection expression for deserialization.
+  explicit GenericSelectionExpr(EmptyShell Empty, unsigned NumAssocs);
+
+public:
+  /// Create a non-result-dependent generic selection expression.
+  static GenericSelectionExpr *
+  Create(const ASTContext , SourceLocation GenericLoc,
+ Expr *ControllingExpr, ArrayRef AssocTypes,
+ ArrayRef AssocExprs, SourceLocation DefaultLoc,
+ SourceLocation RParenLoc, bool ContainsUnexpandedParameterPack,
+ unsigned ResultIndex);
+
+  /// Create a result-dependent generic selection expression.
+  static GenericSelectionExpr *
+  Create(const ASTContext , SourceLocation GenericLoc,
+ Expr *ControllingExpr, ArrayRef AssocTypes,
+ ArrayRef AssocExprs, SourceLocation DefaultLoc,
+ SourceLocation RParenLoc, bool ContainsUnexpandedParameterPack);
+
+  /// Create an empty generic selection expression for deserialization.
+  static GenericSelectionExpr *CreateEmpty(const ASTContext ,
+   unsigned NumAssocs);
 
   /// The number of association expressions.
   

[PATCH] D57238: [AST][NFC] Various cleanups to GenericSelectionExpr

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352275: [AST][NFC] Various cleanups to GenericSelectionExpr 
(authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57238?vs=183530=183697#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57238

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
  cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -5014,98 +5014,118 @@
 /// which means that the choice of result expression is dependent.
 /// Result-dependent generic associations are both type- and value-dependent.
 class GenericSelectionExpr : public Expr {
-  enum { CONTROLLING, END_EXPR };
-  TypeSourceInfo **AssocTypes;
-  Stmt **SubExprs;
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
+
+  /// The number of association expressions and the index of the result
+  /// expression in the case where the generic selection expression is not
+  /// result-dependent. The result index is equal to ResultDependentIndex
+  /// if and only if the generic selection expression is result-dependent.
   unsigned NumAssocs, ResultIndex;
+  enum : unsigned {
+ResultDependentIndex = std::numeric_limits::max(),
+ControllingIndex = 0,
+AssocExprStartIndex = 1
+  };
+
+  /// The location of the "_Generic", "default" and of the right parenthesis.
   SourceLocation GenericLoc, DefaultLoc, RParenLoc;
 
+  TypeSourceInfo **AssocTypes;
+  Stmt **SubExprs;
+
 public:
-  GenericSelectionExpr(const ASTContext ,
-   SourceLocation GenericLoc, Expr *ControllingExpr,
-   ArrayRef AssocTypes,
-   ArrayRef AssocExprs,
-   SourceLocation DefaultLoc, SourceLocation RParenLoc,
+  GenericSelectionExpr(const ASTContext , SourceLocation GenericLoc,
+   Expr *ControllingExpr,
+   ArrayRef AssocTypes,
+   ArrayRef AssocExprs, SourceLocation DefaultLoc,
+   SourceLocation RParenLoc,
bool ContainsUnexpandedParameterPack,
unsigned ResultIndex);
 
   /// This constructor is used in the result-dependent case.
-  GenericSelectionExpr(const ASTContext ,
-   SourceLocation GenericLoc, Expr *ControllingExpr,
-   ArrayRef AssocTypes,
-   ArrayRef AssocExprs,
-   SourceLocation DefaultLoc, SourceLocation RParenLoc,
+  GenericSelectionExpr(const ASTContext , SourceLocation GenericLoc,
+   Expr *ControllingExpr,
+   ArrayRef AssocTypes,
+   ArrayRef AssocExprs, SourceLocation DefaultLoc,
+   SourceLocation RParenLoc,
bool ContainsUnexpandedParameterPack);
 
   explicit GenericSelectionExpr(EmptyShell Empty)
-: Expr(GenericSelectionExprClass, Empty) { }
+  : Expr(GenericSelectionExprClass, Empty) {}
 
+  /// The number of association expressions.
   unsigned getNumAssocs() const { return NumAssocs; }
 
-  SourceLocation getGenericLoc() const { return GenericLoc; }
-  SourceLocation getDefaultLoc() const { return DefaultLoc; }
-  SourceLocation getRParenLoc() const { return RParenLoc; }
+  /// The zero-based index of the result expression's generic association in
+  /// the generic selection's association list.  Defined only if the
+  /// generic selection is not result-dependent.
+  unsigned getResultIndex() const {
+assert(!isResultDependent() &&
+   "Generic selection is result-dependent but getResultIndex called!");
+return ResultIndex;
+  }
 
-  const Expr *getAssocExpr(unsigned i) const {
-return cast(SubExprs[END_EXPR+i]);
+  /// Whether this generic selection is result-dependent.
+  bool isResultDependent() const { return ResultIndex == ResultDependentIndex; }
+
+  /// Return the controlling expression of this generic selection expression.
+  Expr *getControllingExpr() { return cast(SubExprs[ControllingIndex]); }
+  const Expr *getControllingExpr() const {
+return cast(SubExprs[ControllingIndex]);
   }
-  Expr *getAssocExpr(unsigned i) { return cast(SubExprs[END_EXPR+i]); }
-  ArrayRef getAssocExprs() const {
-return NumAssocs
-   ? llvm::makeArrayRef(
- _cast(SubExprs)[END_EXPR], NumAssocs)
-   : None;
+
+  /// Return the result expression of this controlling expression. Defined if
+  /// and only if the generic selection expression is not result-dependent.
+  Expr *getResultExpr() {
+return 

r352275 - [AST][NFC] Various cleanups to GenericSelectionExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Sat Jan 26 05:58:15 2019
New Revision: 352275

URL: http://llvm.org/viewvc/llvm-project?rev=352275=rev
Log:
[AST][NFC] Various cleanups to GenericSelectionExpr

Various cleanups to GenericSelectionExpr factored out of D57104. In particular:

1. Move the friend declaration to the top.
2. Introduce a constant ResultDependentIndex instead of the magic "-1".
3. clang-format
4. Group the member function together so that they can be removed as one block
   by D57106.

NFC.

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

Reviewed By: aaron.ballman


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=352275=352274=352275=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Sat Jan 26 05:58:15 2019
@@ -5014,98 +5014,118 @@ public:
 /// which means that the choice of result expression is dependent.
 /// Result-dependent generic associations are both type- and value-dependent.
 class GenericSelectionExpr : public Expr {
-  enum { CONTROLLING, END_EXPR };
-  TypeSourceInfo **AssocTypes;
-  Stmt **SubExprs;
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
+
+  /// The number of association expressions and the index of the result
+  /// expression in the case where the generic selection expression is not
+  /// result-dependent. The result index is equal to ResultDependentIndex
+  /// if and only if the generic selection expression is result-dependent.
   unsigned NumAssocs, ResultIndex;
+  enum : unsigned {
+ResultDependentIndex = std::numeric_limits::max(),
+ControllingIndex = 0,
+AssocExprStartIndex = 1
+  };
+
+  /// The location of the "_Generic", "default" and of the right parenthesis.
   SourceLocation GenericLoc, DefaultLoc, RParenLoc;
 
+  TypeSourceInfo **AssocTypes;
+  Stmt **SubExprs;
+
 public:
-  GenericSelectionExpr(const ASTContext ,
-   SourceLocation GenericLoc, Expr *ControllingExpr,
-   ArrayRef AssocTypes,
-   ArrayRef AssocExprs,
-   SourceLocation DefaultLoc, SourceLocation RParenLoc,
+  GenericSelectionExpr(const ASTContext , SourceLocation GenericLoc,
+   Expr *ControllingExpr,
+   ArrayRef AssocTypes,
+   ArrayRef AssocExprs, SourceLocation DefaultLoc,
+   SourceLocation RParenLoc,
bool ContainsUnexpandedParameterPack,
unsigned ResultIndex);
 
   /// This constructor is used in the result-dependent case.
-  GenericSelectionExpr(const ASTContext ,
-   SourceLocation GenericLoc, Expr *ControllingExpr,
-   ArrayRef AssocTypes,
-   ArrayRef AssocExprs,
-   SourceLocation DefaultLoc, SourceLocation RParenLoc,
+  GenericSelectionExpr(const ASTContext , SourceLocation GenericLoc,
+   Expr *ControllingExpr,
+   ArrayRef AssocTypes,
+   ArrayRef AssocExprs, SourceLocation DefaultLoc,
+   SourceLocation RParenLoc,
bool ContainsUnexpandedParameterPack);
 
   explicit GenericSelectionExpr(EmptyShell Empty)
-: Expr(GenericSelectionExprClass, Empty) { }
+  : Expr(GenericSelectionExprClass, Empty) {}
 
+  /// The number of association expressions.
   unsigned getNumAssocs() const { return NumAssocs; }
 
-  SourceLocation getGenericLoc() const { return GenericLoc; }
-  SourceLocation getDefaultLoc() const { return DefaultLoc; }
-  SourceLocation getRParenLoc() const { return RParenLoc; }
+  /// The zero-based index of the result expression's generic association in
+  /// the generic selection's association list.  Defined only if the
+  /// generic selection is not result-dependent.
+  unsigned getResultIndex() const {
+assert(!isResultDependent() &&
+   "Generic selection is result-dependent but getResultIndex called!");
+return ResultIndex;
+  }
 
-  const Expr *getAssocExpr(unsigned i) const {
-return cast(SubExprs[END_EXPR+i]);
+  /// Whether this generic selection is result-dependent.
+  bool isResultDependent() const { return ResultIndex == ResultDependentIndex; 
}
+
+  /// Return the controlling expression of this generic selection expression.
+  Expr *getControllingExpr() { return cast(SubExprs[ControllingIndex]); }
+  const Expr *getControllingExpr() const {
+return cast(SubExprs[ControllingIndex]);
   }
-  Expr *getAssocExpr(unsigned i) { return cast(SubExprs[END_EXPR+i]); }
-  ArrayRef getAssocExprs() const {
-return NumAssocs
-   ? 

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 183696.
riccibruno added a comment.

Update the comment in `IgnoreImpCastsExtraSingleStep` and return early from 
`IgnoreImpCastsExtraSingleStep` and `IgnoreImplicitSingleStep` when 
`IgnoreImpCastsSingleStep` skipped something.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267

Files:
  include/clang/AST/Expr.h
  lib/AST/Expr.cpp

Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2556,192 +2556,186 @@
   return QualType();
 }
 
-Expr *Expr::IgnoreImpCasts() {
-  Expr *E = this;
-  while (true)
-if (ImplicitCastExpr *ICE = dyn_cast(E))
-  E = ICE->getSubExpr();
-else if (FullExpr *FE = dyn_cast(E))
-  E = FE->getSubExpr();
-else
-  break;
+/* Logic for the various Expr::Ignore* /
+
+static Expr *IgnoreImpCastsSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast_or_null(E))
+return ICE->getSubExpr();
+
+  else if (auto *FE = dyn_cast_or_null(E))
+return FE->getSubExpr();
+
   return E;
 }
 
-Expr *Expr::IgnoreImplicit() {
-  Expr *E = this;
-  Expr *LastE = nullptr;
-  while (E != LastE) {
-LastE = E;
+static Expr *IgnoreImpCastsExtraSingleStep(Expr *E) {
+  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
+  // addition to what IgnoreImpCasts() skips to account for the current
+  // behaviour of IgnoreParenImpCasts().
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
 
-if (auto *ICE = dyn_cast(E))
-  E = ICE->getSubExpr();
+  if (auto *MTE = dyn_cast_or_null(E))
+return MTE->GetTemporaryExpr();
 
-if (auto *FE = dyn_cast(E))
-  E = FE->getSubExpr();
+  else if (auto *NTTP = dyn_cast_or_null(E))
+return NTTP->getReplacement();
 
-if (auto *MTE = dyn_cast(E))
-  E = MTE->GetTemporaryExpr();
+  return E;
+}
+
+static Expr *IgnoreCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast_or_null(E))
+return CE->getSubExpr();
+
+  else if (auto *FE = dyn_cast_or_null(E))
+return FE->getSubExpr();
+
+  else if (auto *MTE = dyn_cast_or_null(E))
+return MTE->GetTemporaryExpr();
+
+  else if (auto *NTTP = dyn_cast_or_null(E))
+return NTTP->getReplacement();
 
-if (auto *BTE = dyn_cast(E))
-  E = BTE->getSubExpr();
-  }
   return E;
 }
 
-Expr* Expr::IgnoreParens() {
-  Expr* E = this;
-  while (true) {
-if (ParenExpr* P = dyn_cast(E)) {
-  E = P->getSubExpr();
-  continue;
-}
-if (UnaryOperator* P = dyn_cast(E)) {
-  if (P->getOpcode() == UO_Extension) {
-E = P->getSubExpr();
-continue;
-  }
-}
-if (GenericSelectionExpr* P = dyn_cast(E)) {
-  if (!P->isResultDependent()) {
-E = P->getResultExpr();
-continue;
-  }
-}
-if (ChooseExpr* P = dyn_cast(E)) {
-  if (!P->isConditionDependent()) {
-E = P->getChosenSubExpr();
-continue;
-  }
-}
-if (ConstantExpr *CE = dyn_cast(E)) {
-  E = CE->getSubExpr();
-  continue;
-}
-return E;
+static Expr *IgnoreLValueCastsSingleStep(Expr *E) {
+  // Skip what IgnoreCastsSingleStep skips, except that only
+  // lvalue-to-rvalue casts are skipped.
+  if (auto *CE = dyn_cast_or_null(E))
+if (CE->getCastKind() != CK_LValueToRValue)
+  return E;
+
+  return IgnoreCastsSingleStep(E);
+}
+
+static Expr *IgnoreBaseCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast_or_null(E))
+if (CE->getCastKind() == CK_DerivedToBase ||
+CE->getCastKind() == CK_UncheckedDerivedToBase ||
+CE->getCastKind() == CK_NoOp)
+  return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitSingleStep(Expr *E) {
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+return SubE;
+
+  if (auto *MTE = dyn_cast_or_null(E))
+return MTE->GetTemporaryExpr();
+
+  else if (auto *BTE = dyn_cast_or_null(E))
+return BTE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreParensSingleStep(Expr *E) {
+  if (auto *PE = dyn_cast_or_null(E))
+return PE->getSubExpr();
+
+  else if (auto *UO = dyn_cast_or_null(E)) {
+if (UO->getOpcode() == UO_Extension)
+  return UO->getSubExpr();
+  }
+
+  else if (auto *GSE = dyn_cast_or_null(E)) {
+if (!GSE->isResultDependent())
+  return GSE->getResultExpr();
+  }
+
+  else if (auto *CE = dyn_cast_or_null(E)) {
+if (!CE->isConditionDependent())
+  return CE->getChosenSubExpr();
   }
+
+  else if (auto *CE = dyn_cast_or_null(E))
+return CE->getSubExpr();
+
+  return E;
 }
 
-/// IgnoreParenCasts - Ignore parentheses and casts.  Strip off any ParenExpr
-/// or CastExprs or ImplicitCastExprs, returning their operand.
-Expr *Expr::IgnoreParenCasts() {
-  Expr *E = this;
-  while (true) {
-E = E->IgnoreParens();
-if (CastExpr *P = dyn_cast(E)) {
-  E = P->getSubExpr();
-  

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 183694.
lebedev.ri marked 28 inline comments as done.
lebedev.ri added a comment.

Addressed review notes;
found a way to use the `OMPClause` matcher in `isAllowedToContainClause()`,
as opposed to passing the `OpenMPClauseKind`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.h
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  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' 

[PATCH] D57266: [AST] Update the comments of the various Expr::Ignore* + Related cleanups

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno updated this revision to Diff 183695.
riccibruno added a comment.

Removed superfluous braces added in some if statements.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57266

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  lib/ARCMigrate/TransRetainReleaseDealloc.cpp
  lib/ARCMigrate/TransformActions.cpp
  lib/ARCMigrate/Transforms.cpp
  lib/AST/Expr.cpp
  lib/AST/Stmt.cpp
  lib/Analysis/ReachableCode.cpp
  lib/Tooling/ASTDiff/ASTDiff.cpp

Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -237,8 +237,8 @@
 return true;
   }
   bool TraverseStmt(Stmt *S) {
-if (S)
-  S = S->IgnoreImplicit();
+if (auto *E = dyn_cast_or_null(S))
+  S = E->IgnoreImplicit();
 if (isNodeExcluded(Tree.AST.getSourceManager(), S))
   return true;
 auto SavedState = PreTraverse(S);
Index: lib/Analysis/ReachableCode.cpp
===
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -192,9 +192,10 @@
   if (!S)
 return false;
 
-  S = S->IgnoreImplicit();
+  if (const auto *Ex = dyn_cast(S))
+S = Ex->IgnoreImplicit();
 
-  if (const Expr *Ex = dyn_cast(S))
+  if (const auto *Ex = dyn_cast(S))
 S = Ex->IgnoreCasts();
 
   // Special case looking for the sigil '()' around an integer literal.
Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -117,30 +117,6 @@
   StatisticsEnabled = true;
 }
 
-Stmt *Stmt::IgnoreImplicit() {
-  Stmt *s = this;
-
-  Stmt *lasts = nullptr;
-
-  while (s != lasts) {
-lasts = s;
-
-if (auto *fe = dyn_cast(s))
-  s = fe->getSubExpr();
-
-if (auto *mte = dyn_cast(s))
-  s = mte->GetTemporaryExpr();
-
-if (auto *bte = dyn_cast(s))
-  s = bte->getSubExpr();
-
-if (auto *ice = dyn_cast(s))
-  s = ice->getSubExpr();
-  }
-
-  return s;
-}
-
 /// Skip no-op (attributed, compound) container stmts and skip captured
 /// stmt at the top, if \a IgnoreCaptured is true.
 Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured) {
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2556,6 +2556,39 @@
   return QualType();
 }
 
+Expr *Expr::IgnoreImpCasts() {
+  Expr *E = this;
+  while (true)
+if (ImplicitCastExpr *ICE = dyn_cast(E))
+  E = ICE->getSubExpr();
+else if (FullExpr *FE = dyn_cast(E))
+  E = FE->getSubExpr();
+else
+  break;
+  return E;
+}
+
+Expr *Expr::IgnoreImplicit() {
+  Expr *E = this;
+  Expr *LastE = nullptr;
+  while (E != LastE) {
+LastE = E;
+
+if (auto *ICE = dyn_cast(E))
+  E = ICE->getSubExpr();
+
+if (auto *FE = dyn_cast(E))
+  E = FE->getSubExpr();
+
+if (auto *MTE = dyn_cast(E))
+  E = MTE->GetTemporaryExpr();
+
+if (auto *BTE = dyn_cast(E))
+  E = BTE->getSubExpr();
+  }
+  return E;
+}
+
 Expr* Expr::IgnoreParens() {
   Expr* E = this;
   while (true) {
@@ -2722,7 +2755,7 @@
 /// IgnoreParenNoopCasts - Ignore parentheses and casts that do not change the
 /// value (including ptr->int casts of the same size).  Strip off any
 /// ParenExpr or CastExprs, returning their operand.
-Expr *Expr::IgnoreParenNoopCasts(ASTContext ) {
+Expr *Expr::IgnoreParenNoopCasts(const ASTContext ) {
   Expr *E = this;
   while (true) {
 E = E->IgnoreParens();
Index: lib/ARCMigrate/Transforms.cpp
===
--- lib/ARCMigrate/Transforms.cpp
+++ lib/ARCMigrate/Transforms.cpp
@@ -286,10 +286,11 @@
   void mark(Stmt *S) {
 if (!S) return;
 
-while (LabelStmt *Label = dyn_cast(S))
+while (auto *Label = dyn_cast(S))
   S = Label->getSubStmt();
-S = S->IgnoreImplicit();
-if (Expr *E = dyn_cast(S))
+if (auto *E = dyn_cast(S))
+  S = E->IgnoreImplicit();
+if (auto *E = dyn_cast(S))
   Removables.insert(E);
   }
 };
Index: lib/ARCMigrate/TransformActions.cpp
===
--- lib/ARCMigrate/TransformActions.cpp
+++ lib/ARCMigrate/TransformActions.cpp
@@ -313,7 +313,9 @@
   assert(IsInTransaction && "Actions only allowed during a transaction");
   ActionData data;
   data.Kind = Act_RemoveStmt;
-  data.S = S->IgnoreImplicit(); // important for uniquing
+  if (auto *E = dyn_cast(S))
+S = E->IgnoreImplicit(); // important for uniquing
+  data.S = S;
   CachedActions.push_back(data);
 }
 
Index: lib/ARCMigrate/TransRetainReleaseDealloc.cpp
===
--- lib/ARCMigrate/TransRetainReleaseDealloc.cpp
+++ lib/ARCMigrate/TransRetainReleaseDealloc.cpp
@@ -269,8 +269,8 @@
 
 if (prevChildS != childE) {
   

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:49-50
+///
+/// FIXME: would be better to pass the actual class name (e.g. 
OMPDefaultClause)
+///instead of the actual OpenMPClauseKind.
+AST_MATCHER_P(OMPExecutableDirective, isAllowedToContainClause,

Solved, this is so ugly xD



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:97
+///
+/// Given, `ompDefaultClause(hasKind(OMPC_DEFAULT_shared))`:
+///

lebedev.ri wrote:
> JonasToth wrote:
> > you could provide helper-matchers similiar to `isImplicit()`, `isInline()` 
> > to allow `ompDefaultClause(isDefaultShared())`.
> Could work, will take a look, thanks for a hint!
Not quite sure re `Kind` suffix, but i guess this is better.



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:113
+void UseDefaultNoneCheck::registerMatchers(MatchFinder *Finder) {
+  // If OpenMP is not enabled, don't register the check, it won't find 
anything.
+  if (!getLangOpts().OpenMP)

ABataev wrote:
> JonasToth wrote:
> > Is that the case? Will the pragmas be ignored if non-omp?
> > 
> > You could reorder that sentence `Don't register the check if OpenMP is not 
> > enabled as the directives are not considered in the AST.` or so.
> If OpenMP is disabled, the pragmas are just completely ignored.
Reworded a little, hopefully better.



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.h:19
+/// Finds OpenMP directives that are allowed to contain ``default`` clause,
+/// but either don``t specify it, or the clause is specified but with the kind
+/// other than ``none``, and suggests to use ``default(none)`` clause.

JonasToth wrote:
> double back-tick in dont, please use the normal straight tick instead
Hah, that is mass-replace for you.



Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:12
+being implicitly determined, and thus forces developer to be explicit about the
+desired data scoping for each variable.
+

lebedev.ri wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > JonasToth wrote:
> > > > JonasToth wrote:
> > > > > If I understand correctly the issue is more about implicitly shared 
> > > > > variables that lead to data-races and bad access patterns, is that 
> > > > > correct?
> > > > > If so it might clarify the reason for this check, if added directly 
> > > > > in the first motivational sentence.
> > > > is it scoping or rather sharing? The scope is C++-terrain or at least 
> > > > used in C++-Speak. Maybe there is a unambiguous word instead?
> > > I'm not quite sure how to formulate the reasoning to be honest.
> > > Let me try to show some reasonably-complete example:
> > > https://godbolt.org/z/mMQhbr
> > > 
> > > We have 4 compilers: clang trunk + openmp 3.1, clang trunk + openmp 4, 
> > > gcc 8, gcc trunk
> > > 
> > > We have 5 code samples, all operating with a `const` variable: (the same 
> > > code, just different omp clauses)
> > > * If no `default` clause is specified, every compiler is fine with the 
> > > code.
> > > * If `default(shared)` clause is specified, every compiler is still fine 
> > > with the code.
> > >   On this example, no clause and `default(shared)` clause are equivalent.
> > >   I can't/won't claim whether or not that is always the case, as i'm 
> > > mostly arguing re `default(none)`.
> > > * If `default(none) shared(num)` is specified, all is also fine.
> > >   That is equivalent to just the `default(none)` for OpenMP 3.1
> > > * If `default(none) firstprivate(num)` is specified, all still fine fine.
> > > * If only ``default(none)` is specified, things start to get wonky.
> > >   The general idea is that before OpenMP 4.0 such `const` variables were 
> > > auto-determined as `shared`,
> > >   and now they won't. So one will need to add either `shared(num)` or 
> > > `firstprivate(num)`.
> > >   Except the older gcc will diagnose `shared(num)` :)
> > > 
> > > Roughly the same is true when the variable is not `const`: 
> > > https://godbolt.org/z/wuouI_
> > > 
> > > Thus, `default(none)` forced one to be explicit about what shall be done 
> > > with the variable,
> > > should it be `shared`, or `firstprivate`.
> > > 
> > > 
> > > ^ Not whether this rambling makes sense?
> > Yes, so its about being explicit if state is shared or not. Given that its 
> > hard to reason about parallel programs being explicit helps reading the 
> > code. I think that could be condensed in a short motivation section.
> > Yes, so its about being explicit if state is shared or not. Given that its 
> > hard to reason about parallel programs being explicit helps reading the 
> > code. I think that could be condensed in a short motivation section.
> 
> Yep. Will try to reword.
> 
> > is it scoping or rather sharing? The scope is C++-terrain or at least used 
> > in C++-Speak. Maybe there is a unambiguous word instead?
> 
> E.g. 

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 183692.
lebedev.ri added a comment.

Store OpenMP clause spelling in the `ASTNodeKind::KindInfo 
ASTNodeKind::AllKindInfo[]`, not the stringified clang AST class name.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112

Files:
  include/clang/AST/ASTTypeTraits.h
  lib/AST/ASTTypeTraits.cpp

Index: lib/AST/ASTTypeTraits.cpp
===
--- lib/AST/ASTTypeTraits.cpp
+++ lib/AST/ASTTypeTraits.cpp
@@ -37,6 +37,10 @@
   { NKI_None, "Type" },
 #define TYPE(DERIVED, BASE) { NKI_##BASE, #DERIVED "Type" },
 #include "clang/AST/TypeNodes.def"
+  { NKI_None, "OMPClause" },
+#define OPENMP_CLAUSE(TextualSpelling, DERIVED)\
+  {NKI_OMPClause, #TextualSpelling},
+#include "clang/Basic/OpenMPKinds.def"
 };
 
 bool ASTNodeKind::isBaseOf(ASTNodeKind Other, unsigned *Distance) const {
@@ -103,6 +107,20 @@
 #include "clang/AST/TypeNodes.def"
   }
   llvm_unreachable("invalid type kind");
+ }
+
+ASTNodeKind ASTNodeKind::getFromNode(const OMPClause ) {
+  switch (C.getClauseKind()) {
+#define OPENMP_CLAUSE(Name, Class) \
+case OMPC_##Name: return ASTNodeKind(NKI_##Class);
+#include "clang/Basic/OpenMPKinds.def"
+case OMPC_flush:
+case OMPC_threadprivate:
+case OMPC_uniform:
+case OMPC_unknown:
+  llvm_unreachable("unexpected OpenMP clause kind");
+  }
+  llvm_unreachable("invalid stmt kind");
 }
 
 void DynTypedNode::print(llvm::raw_ostream ,
@@ -151,6 +169,8 @@
 return D->getSourceRange();
   if (const Stmt *S = get())
 return S->getSourceRange();
+  if (const auto *C = get())
+return SourceRange(C->getBeginLoc(), C->getEndLoc());
   return SourceRange();
 }
 
Index: include/clang/AST/ASTTypeTraits.h
===
--- include/clang/AST/ASTTypeTraits.h
+++ include/clang/AST/ASTTypeTraits.h
@@ -18,6 +18,7 @@
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TypeLoc.h"
@@ -58,6 +59,7 @@
   static ASTNodeKind getFromNode(const Decl );
   static ASTNodeKind getFromNode(const Stmt );
   static ASTNodeKind getFromNode(const Type );
+  static ASTNodeKind getFromNode(const OMPClause );
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
@@ -136,6 +138,9 @@
 NKI_Type,
 #define TYPE(DERIVED, BASE) NKI_##DERIVED##Type,
 #include "clang/AST/TypeNodes.def"
+NKI_OMPClause,
+#define OPENMP_CLAUSE(TextualSpelling, DERIVED) NKI_##DERIVED,
+#include "clang/Basic/OpenMPKinds.def"
 NKI_NumberOfKinds
   };
 
@@ -183,12 +188,15 @@
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
 KIND_TO_KIND_ID(Type)
+KIND_TO_KIND_ID(OMPClause)
 #define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl)
 #include "clang/AST/DeclNodes.inc"
 #define STMT(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED)
 #include "clang/AST/StmtNodes.inc"
 #define TYPE(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Type)
 #include "clang/AST/TypeNodes.def"
+#define OPENMP_CLAUSE(TextualSpelling, DERIVED) KIND_TO_KIND_ID(DERIVED)
+#include "clang/Basic/OpenMPKinds.def"
 #undef KIND_TO_KIND_ID
 
 inline raw_ostream <<(raw_ostream , ASTNodeKind K) {
@@ -459,6 +467,11 @@
 T, typename std::enable_if::value>::type>
 : public DynCastPtrConverter {};
 
+template 
+struct DynTypedNode::BaseConverter<
+T, typename std::enable_if::value>::type>
+: public DynCastPtrConverter {};
+
 template <>
 struct DynTypedNode::BaseConverter<
 NestedNameSpecifier, void> : public PtrConverter {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 183690.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Rebased ontop of D57280 , use `const auto*`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57112

Files:
  include/clang/AST/ASTTypeTraits.h
  lib/AST/ASTTypeTraits.cpp

Index: lib/AST/ASTTypeTraits.cpp
===
--- lib/AST/ASTTypeTraits.cpp
+++ lib/AST/ASTTypeTraits.cpp
@@ -37,6 +37,9 @@
   { NKI_None, "Type" },
 #define TYPE(DERIVED, BASE) { NKI_##BASE, #DERIVED "Type" },
 #include "clang/AST/TypeNodes.def"
+  { NKI_None, "OMPClause" },
+#define OPENMP_CLAUSE(TextualSpelling, DERIVED) { NKI_OMPClause, #DERIVED },
+#include "clang/Basic/OpenMPKinds.def"
 };
 
 bool ASTNodeKind::isBaseOf(ASTNodeKind Other, unsigned *Distance) const {
@@ -103,6 +106,20 @@
 #include "clang/AST/TypeNodes.def"
   }
   llvm_unreachable("invalid type kind");
+ }
+
+ASTNodeKind ASTNodeKind::getFromNode(const OMPClause ) {
+  switch (C.getClauseKind()) {
+#define OPENMP_CLAUSE(Name, Class) \
+case OMPC_##Name: return ASTNodeKind(NKI_##Class);
+#include "clang/Basic/OpenMPKinds.def"
+case OMPC_flush:
+case OMPC_threadprivate:
+case OMPC_uniform:
+case OMPC_unknown:
+  llvm_unreachable("unexpected OpenMP clause kind");
+  }
+  llvm_unreachable("invalid stmt kind");
 }
 
 void DynTypedNode::print(llvm::raw_ostream ,
@@ -151,6 +168,8 @@
 return D->getSourceRange();
   if (const Stmt *S = get())
 return S->getSourceRange();
+  if (const auto *C = get())
+return SourceRange(C->getBeginLoc(), C->getEndLoc());
   return SourceRange();
 }
 
Index: include/clang/AST/ASTTypeTraits.h
===
--- include/clang/AST/ASTTypeTraits.h
+++ include/clang/AST/ASTTypeTraits.h
@@ -18,6 +18,7 @@
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TypeLoc.h"
@@ -58,6 +59,7 @@
   static ASTNodeKind getFromNode(const Decl );
   static ASTNodeKind getFromNode(const Stmt );
   static ASTNodeKind getFromNode(const Type );
+  static ASTNodeKind getFromNode(const OMPClause );
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
@@ -136,6 +138,9 @@
 NKI_Type,
 #define TYPE(DERIVED, BASE) NKI_##DERIVED##Type,
 #include "clang/AST/TypeNodes.def"
+NKI_OMPClause,
+#define OPENMP_CLAUSE(TextualSpelling, DERIVED) NKI_##DERIVED,
+#include "clang/Basic/OpenMPKinds.def"
 NKI_NumberOfKinds
   };
 
@@ -183,12 +188,15 @@
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
 KIND_TO_KIND_ID(Type)
+KIND_TO_KIND_ID(OMPClause)
 #define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl)
 #include "clang/AST/DeclNodes.inc"
 #define STMT(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED)
 #include "clang/AST/StmtNodes.inc"
 #define TYPE(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Type)
 #include "clang/AST/TypeNodes.def"
+#define OPENMP_CLAUSE(TextualSpelling, DERIVED) KIND_TO_KIND_ID(DERIVED)
+#include "clang/Basic/OpenMPKinds.def"
 #undef KIND_TO_KIND_ID
 
 inline raw_ostream <<(raw_ostream , ASTNodeKind K) {
@@ -459,6 +467,11 @@
 T, typename std::enable_if::value>::type>
 : public DynCastPtrConverter {};
 
+template 
+struct DynTypedNode::BaseConverter<
+T, typename std::enable_if::value>::type>
+: public DynCastPtrConverter {};
+
 template <>
 struct DynTypedNode::BaseConverter<
 NestedNameSpecifier, void> : public PtrConverter {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam marked an inline comment as done.
zahiraam added a comment.

Aaron,

Please advise. 
Thanks.




Comment at: test/Sema/dllexport.c:70
+// const variables
+__declspec(dllexport) int const x = 3;
 

aaron.ballman wrote:
> Can you think of any redeclaration scenarios we should also test? e.g., weird 
> cases like this:
> ```
> extern int const x;
> __declspec(dllexport) int const x = 3;
> ```
> which brings up an interesting question -- does MSVC truly treat it as though 
> it were extern, or just only-kinda-sorta treat it like it was extern? e.g., 
> can you do this:
> ```
> __declspec(dllexport) const int j;
> ```
> Regardless, having some codegen tests demonstrating that the variable has the 
> correct semantics that we're emulating would be nice.
With MSVC:

ksh-3.2$ cat t2.cpp
__declspec(dllexport) const int j;
ksh-3.2$

ksh-3.2$ cl -c t2.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

t2.cpp
t2.cpp(1): error C2734: 'j': 'const' object must be initialized if not 'extern'

ksh-3.2$ cat t3.cpp
__declspec(dllexport) int const x = 3;
int main ()
{
  int y = x + 10;

  return y;
}

ksh-3.2$ cl -c t3.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

t3.cpp
ksh-3.2$ dumpbin /symbols t3.obj | grep External
**008  SECT3  notype ()External | main**
ksh-3.2$
ksh-3.2$ cat t4.cpp
extern int const x = 3;
int main ()
{
  int y = x + 10;

  return y;
}

ksh-3.2$ cl -c t4.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

t4.cpp
ksh-3.2$ dumpbin /symbols t4.obj | grep External
**008  SECT3  notype   External | ?x@@3HB (int const x)
00B  SECT4  notype ()External | main**
ksh-3.2$

So we see that with dllexport, the ony symbol marked with "External" is main. 
With "extern" both main and x are marked as External.

With clang without the patch:
ksh-3.2$ clang -c t2.cpp
t2.cpp:1:33: error: default initialization of an object of const type 'const 
int'
__declspec(dllexport) const int j;
^
  = 0
t2.cpp:1:33: error: 'j' must have external linkage when declared 'dllexport'
2 errors generated.
ksh-3.2$
ksh-3.2$ clang -c t3.cpp
t3.cpp:1:33: error: 'x' must have external linkage when declared 'dllexport'
__declspec(dllexport) int const x = 3;
^
1 error generated.
ksh-3.2$ clang -c t4.cpp
ksh-3.2$ dumpbin /symbols t4.o | grep External
**00F  SECT1  notype ()External | main
010  SECT5  notype   External | ?x@@3HB (int const x)**
ksh-3.2$

Clang with patch above at the right place (I am thinking in  
Sema::AddInitializerToDecl):

ksh-3.2$ clang -c t3.cpp
ksh-3.2$ dumpbin /symbols t3.o | grep External
**00E  SECT1  notype ()External | main
00F  SECT5  notype   External | ?x@@3HB (int const x)**
ksh-3.2$
ksh-3.2$ clang -c t4.cpp
ksh-3.2$ dumpbin /symbols t4.o | grep External
00C  SECT1  notype ()External | main
00D  SECT5  notype   External | ?x@@3HB (int const x)
ksh-3.2$

Both MSVC and clang have the same behavior with t2.cpp.
With the patch clang will have the same beahvior than MSVC when extern and 
dllexport are used. That's not  quite what MSVC's behavior is!
What are your thoughts?
Thanks.




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

https://reviews.llvm.org/D45978



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

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 5 inline comments as done.
lebedev.ri added inline comments.



Comment at: lib/AST/ASTTypeTraits.cpp:114
+#define OPENMP_CLAUSE(Name, Class) 
\
+case OMPC_##Name: return ASTNodeKind(NKI_##Class);
+#include "clang/Basic/OpenMPKinds.def"

lebedev.ri wrote:
> ABataev wrote:
> > ABataev wrote:
> > > lebedev.ri wrote:
> > > > ABataev wrote:
> > > > > Well, I think it would be good to filter out `OMPC_flush` somehow 
> > > > > because there is no such clause actually, it is a pseudo clause for 
> > > > > better handling of the `flush` directive.
> > > > > 
> > > > Are `OMPC_threadprivate` and `OMPC_uniform` also in the same boat?
> > > > I don't see those clauses in spec.
> > > > 
> > > > Perhaps `OMPC_flush` should be made more like those other two?
> > > > (i.e. handled outside of `OPENMP_CLAUSE` macro)
> > > `OMPC_threadprivate` is also a special kind of pseudo-clause.
> > > `OMPC_flush` is in the enum, because it has the corresponding class. You 
> > > can try to exclude it from the enum, but it may require some additional 
> > > work.
> > > `OMPC_uniform` is a normal clause, but it has the corresponding class. 
> > > This clause can be used on `declare simd` directive, which is represented 
> > > as an attribute.
> > I mean, `OOMPC_uniform` has no(!) corresponding class. Mistyped
> As one would expect, simply adding 
> ```
>   case OMPC_flush: // Pseudo clause, does not exist (keep before including 
> .def)
> llvm_unreachable("unexpected OpenMP clause kind");
> ```
> results in a
> ```
> [58/1118 5.6/sec] Building CXX object 
> tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o
> FAILED: tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o 
> /usr/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -Itools/clang/lib/AST -I/build/clang/lib/AST -I/build/clang/include 
> -Itools/clang/include -I/usr/include/libxml2 -Iinclude -I/build/llvm/include 
> -pipe -O2 -g0 -UNDEBUG -fPIC -fvisibility-inlines-hidden -Werror=date-time 
> -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wno-missing-field-initializers -pedantic -Wno-long-long 
> -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess 
> -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color 
> -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
> -fno-strict-aliasing -pipe -O2 -g0 -UNDEBUG -fPIC   -UNDEBUG  -fno-exceptions 
> -fno-rtti -MD -MT 
> tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -MF 
> tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o.d -o 
> tools/clang/lib/AST/CMakeFiles/clangAST.dir/ASTTypeTraits.cpp.o -c 
> /build/clang/lib/AST/ASTTypeTraits.cpp
> /build/clang/include/clang/Basic/OpenMPKinds.def: In static member function 
> ‘static clang::ast_type_traits::ASTNodeKind 
> clang::ast_type_traits::ASTNodeKind::getFromNode(const clang::OMPClause&)’:
> /build/clang/lib/AST/ASTTypeTraits.cpp:116:5: error: duplicate case value
>  case OMPC_##Name: return ASTNodeKind(NKI_##Class);
>  ^~~~
> /build/clang/include/clang/Basic/OpenMPKinds.def:261:1: note: in expansion of 
> macro ‘OPENMP_CLAUSE’
>  OPENMP_CLAUSE(flush, OMPFlushClause)
>  ^
> /build/clang/lib/AST/ASTTypeTraits.cpp:113:3: note: previously used here
>case OMPC_flush: // Pseudo clause, does not exist (keep before including 
> .def)
>^~~~
> ```
> So one will need to pull `OMPC_flush` out of `clang/Basic/OpenMPKinds.def`.
D57280, will rebase this.


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] D57280: [clang][OpenMP] OMPFlushClause is synthetic, no such clause exists

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added a reviewer: ABataev.
lebedev.ri added a project: OpenMP.
Herald added subscribers: arphaman, guansong.

As discussed in https://reviews.llvm.org/D57112#inline-506781, 
'flush' clause does not exist in the OpenMP spec, it can not be
specified, and `OMPFlushClause` class is just a helper class.

Therefore `OPENMP_CLAUSE()` in `clang/Basic/OpenMPKinds.def`
should not contain 'flush' "clause".

I have simply removed the `OPENMP_CLAUSE(flush, OMPFlushClause)`
from `clang/Basic/OpenMPKinds.def`, grepped for `OPENMP_CLAUSE`
and added `OPENMP_CLAUSE(flush, OMPFlushClause)` back to the **every**
place where `OPENMP_CLAUSE` is defined and `clang/Basic/OpenMPKinds.def`
is then included.

So as-is, this patch is a NFC. Possibly, some of these
`OPENMP_CLAUSE(flush, OMPFlushClause)` should be dropped,
i don't really know.

Test plan: `ninja check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D57280

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ASTWriter.h
  lib/AST/OpenMPClause.cpp
  lib/AST/StmtProfile.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Sema/TreeTransform.h
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -2130,6 +2130,7 @@
   OMPClauseEnqueue(EnqueueVisitor *Visitor) : Visitor(Visitor) { }
 #define OPENMP_CLAUSE(Name, Class) \
   void Visit##Class(const Class *C);
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   void VisitOMPClauseWithPreInit(const OMPClauseWithPreInit *C);
   void VisitOMPClauseWithPostUpdate(const OMPClauseWithPostUpdate *C);
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -681,6 +681,7 @@
 #define OPENMP_CLAUSE(Name, Class)\
   LLVM_ATTRIBUTE_NOINLINE \
   OMPClause *Transform ## Class(Class *S);
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
 
   /// Build a new qualified type given its unqualified type and type location.
@@ -3311,6 +3312,7 @@
 #define OPENMP_CLAUSE(Name, Class) \
   case OMPC_ ## Name : \
 return getDerived().Transform ## Class(cast(S));
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   }
 
Index: lib/Basic/OpenMPKinds.cpp
===
--- lib/Basic/OpenMPKinds.cpp
+++ lib/Basic/OpenMPKinds.cpp
@@ -53,6 +53,7 @@
 return OMPC_unknown;
   return llvm::StringSwitch(Str)
 #define OPENMP_CLAUSE(Name, Class) .Case(#Name, OMPC_##Name)
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   .Case("uniform", OMPC_uniform)
   .Default(OMPC_unknown);
@@ -67,6 +68,8 @@
   case OMPC_##Name:\
 return #Name;
 #include "clang/Basic/OpenMPKinds.def"
+  case OMPC_flush:
+return "flush";
   case OMPC_uniform:
 return "uniform";
   case OMPC_threadprivate:
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -412,6 +412,7 @@
   OMPClauseProfiler(StmtProfiler *P) : Profiler(P) { }
 #define OPENMP_CLAUSE(Name, Class) \
   void Visit##Class(const Class *C);
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   void VistOMPClauseWithPreInit(const OMPClauseWithPreInit *C);
   void VistOMPClauseWithPostUpdate(const OMPClauseWithPostUpdate *C);
Index: lib/AST/OpenMPClause.cpp
===
--- lib/AST/OpenMPClause.cpp
+++ lib/AST/OpenMPClause.cpp
@@ -30,6 +30,7 @@
 #define OPENMP_CLAUSE(Name, Class) \
   case OMPC_##Name:\
 return static_cast(this)->children();
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   }
   llvm_unreachable("unknown OMPClause");
Index: include/clang/Serialization/ASTWriter.h
===
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -999,6 +999,7 @@
 public:
   OMPClauseWriter(ASTRecordWriter ) : Record(Record) {}
 #define OPENMP_CLAUSE(Name, Class) void Visit##Class(Class *S);
+  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   void writeClause(OMPClause