[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

2019-09-06 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

In D64671#1661585 , @jpakkane wrote:

> > It'll be reasonable to get IncludeStyle default from .clang-format.
>
> I looked at existing checks and they all do the same thing as this one. In 
> fact I got the code for this by directly copypasting an existing check.


SInse this option may be also set in .clang-format, it's reasonable to re-use 
it instead of forcing user to duplicate it.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:71
+
+- New :doc:`cppcoreguidelines-init-variables
+  ` check.

jpakkane wrote:
> Eugene.Zelenko wrote:
> > Please sort new checks list alphabetically.
> The list is already incorrectly alphabetized. Should I reorganize all the 
> entries or just put mine in a "correct" spot (between "bugprone" and 
> "linuxkernel")?
It'll not hurt to correct other mistakes too.


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

https://reviews.llvm.org/D64671



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


[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

2019-09-06 Thread Jussi Pakkanen via Phabricator via cfe-commits
jpakkane updated this revision to Diff 219179.
jpakkane added a comment.

Ordered doc list alphabetically.


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

https://reviews.llvm.org/D64671

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp

Index: clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t
+
+// Ensure that function declarations are not changed.
+void some_func(int x, double d, bool b, const char *p);
+
+// Ensure that function arguments are not changed
+int identity_function(int x) {
+  return x;
+}
+
+int do_not_modify_me;
+
+static int should_not_be_initialized;
+extern int should_not_be_initialized2;
+
+typedef struct {
+  int unaltered1;
+  int unaltered2;
+} UnusedStruct;
+
+typedef int my_int_type;
+#define MACRO_INT int
+#define FULL_DECLARATION() int macrodecl;
+
+template 
+void template_test_function() {
+  T t;
+  int uninitialized;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'uninitialized' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int uninitialized = 0;{{$}}
+}
+
+void init_unit_tests() {
+  int x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'x' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int x = 0;{{$}}
+  my_int_type myint;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'myint' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  my_int_type myint = 0;{{$}}
+
+  MACRO_INT macroint;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'macroint' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  MACRO_INT macroint = 0;{{$}}
+  FULL_DECLARATION();
+
+  int x0 = 1, x1, x2 = 2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'x1' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int x0 = 1, x1 = 0, x2 = 2;{{$}}
+  int y0, y1 = 1, y2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'y0' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-MESSAGES: :[[@LINE-2]]:19: warning: variable 'y2' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int y0 = 0, y1 = 1, y2 = 0;{{$}}
+  int hasval = 42;
+
+  float f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'f' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  float f = NAN;{{$}}
+  float fval = 85.0;
+  double d;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: variable 'd' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  double d = NAN;{{$}}
+  double dval = 99.0;
+
+  bool b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable 'b' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  bool b = 0;{{$}}
+  bool bval = true;
+
+  const char *ptr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'ptr' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  const char *ptr = nullptr;{{$}}
+  const char *ptrval = "a string";
+
+  UnusedStruct u;
+
+  static int does_not_need_an_initializer;
+  extern int does_not_need_an_initializer2;
+  int parens(42);
+  int braces{42};
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -193,6 +193,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-explicit-virtual-functions (redirects to modernize-use-override) 
+   cppcoreguidelines-init-variables
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
@@ -0,0 +1,46 @@
+.. title:: clang-tidy - cppcoreguidelines-init-variables
+
+cppcoreguidelines-init-variables
+

[PATCH] D67135: [clang-tidy] performance-inefficient-vector-operation: Support proto repeated field

2019-09-06 Thread Cong Liu via Phabricator via cfe-commits
congliu updated this revision to Diff 219180.
congliu marked 7 inline comments as done.
congliu added a comment.

Addressed Haojian's comments.

- Do not warn when there is reference to the proto variable between its 
declaration and the loop body.
- Exclude const methods.
- Update tests.


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

https://reviews.llvm.org/D67135

Files:
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp

Index: clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
===
--- clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp
@@ -1,4 +1,7 @@
-// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- -format-style=llvm
+// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
+// RUN: -format-style=llvm \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
 
 namespace std {
 
@@ -62,6 +65,28 @@
 
 int Op(int);
 
+namespace proto2 {
+class MessageLite {};
+class Message : public MessageLite {};
+} // namespace proto2
+
+class FooProto : public proto2::Message {
+ public:
+  int *add_x();  // repeated int x;
+  void add_x(int x);
+  void mutable_x();
+  void mutable_y();
+  int add_z() const; // optional int add_z;
+};
+
+class BarProto : public proto2::Message {
+ public:
+  int *add_x();
+  void add_x(int x);
+  void mutable_x();
+  void mutable_y();
+};
+
 void f(std::vector& t) {
   {
 std::vector v0;
@@ -162,6 +187,15 @@
 }
   }
 
+  {
+FooProto foo;
+// CHECK-FIXES: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'add_x' is called inside a loop; consider pre-allocating the repeated field capacity before the loop
+}
+  }
+
   //  Non-fixed Cases 
   {
 std::vector z0;
@@ -274,4 +308,54 @@
   z12.push_back(e);
 }
   }
+
+  {
+FooProto foo;
+foo.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+foo.add_x(-1);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+BarProto bar;
+bar.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x();
+  bar.add_x();
+}
+  }
+  {
+FooProto foo;
+foo.mutable_y();
+// CHECK-FIXES-NOT: foo.mutable_x()->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_x(i);
+}
+  }
+  {
+FooProto foo;
+// CHECK-FIXES-NOT: foo.mutable_z->Reserve(5);
+for (int i = 0; i < 5; i++) {
+  foo.add_z();
+}
+  }
 }
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
@@ -6,6 +6,10 @@
 Finds possible inefficient ``std::vector`` operations (e.g. ``push_back``,
 ``emplace_back``) that may cause unnecessary memory reallocations.
 
+It can also find calls that add element to protobuf repeated field in a loop
+without calling Reserve() before the loop. Calling Reserve() first can avoid
+unnecessary memory reallocations.
+
 Currently, the check only detects following kinds of loops with a single
 statement body:
 
@@ -21,6 +25,13 @@
 // statement before the for statement.
   }
 
+  SomeProto p;
+  for (int i = 0; i < n; ++i) {
+p.add_xxx(n);
+// This will trigger the warning since the add_xxx may cause multiple memory
+// relloacations. This can be avoid by inserting a
+// 'p.mutable_xxx().Reserve(n)' statement before the for statement.
+  }
 
 * For-range loops like ``for (range-declaration : range_expression)``, the type
   of ``range_expression`` can be ``std::vector``, ``std::array``,
@@ -47,3 +58,8 @@
 
Semicolon-separated list of names of vector-like classes. By default only
``::std::vector`` is considered.
+
+.. option:: EnableProto
+   When non-zero, the check will also warn on inefficient operations for proto
+   repeated fields. Otherwise, the check only warns 

[PATCH] D67135: [clang-tidy] performance-inefficient-vector-operation: Support proto repeated field

2019-09-06 Thread Cong Liu via Phabricator via cfe-commits
congliu added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp:227
+std::string MutableFieldName =
+("mutable_" + ProtoAddFieldCall->getMethodDecl()->getName().substr(4))
+.str();

hokein wrote:
> nit: getName().drop_front(sizeof("add_")).
Used 'sizeof("add_")-1' since "add_" is const char [5].



Comment at: 
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp:233
+  if (Matches.empty()) {
+// There is no method with name "mutable_xxx".
+return;

hokein wrote:
> for repeated fields, there should be `add_foo`, `mutable_foo` methods in the 
> proto generated code 
> (https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#repeatednumeric).
> 
> So I think we can remove this check here.
I intended to rule out the corner cases when a proto field name is "add_xxx". 
But now I think checking whether there is a "mutable_xxx" method is not a 
effective way to rule out the corner case. A simpler way is just checking 
whether add_xxx is const... I have updated the matcher to exclude const methods.



Comment at: 
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp:249
+match.getNodeAs("maybe_reallocation");
+// Skip cases where "mutable_xxx" or "add_xxx" is called before the
+// loop.

hokein wrote:
> the heuristic is limited, and will fail the cases like below:
> 
> ```
> MyProto proto;
> set_proto_xxx_size(&proto);
> for (int i = 0; i < n; ++i) {
>proto.add_xxx(i);
> }
> ```
> 
> In the vector case, we do a more strict check, maybe we do the same way as 
> well (but it will make the check fail to spot some cases)...
Good point. Let's avoid those false positives.


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

https://reviews.llvm.org/D67135



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


[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

2019-09-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.

I'm fine with moving in-class function into anonymous namespace later. LGTM, 
feel free to commit!


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

https://reviews.llvm.org/D59637



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


[PATCH] D67200: Add -static-openmp driver option

2019-09-06 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama updated this revision to Diff 219182.
pirama added a comment.
Herald added a subscriber: ychen.

Mention NDK issue https://github.com/android-ndk/ndk/issues/1028.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67200

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Driver/fopenmp.c

Index: clang/test/Driver/fopenmp.c
===
--- clang/test/Driver/fopenmp.c
+++ clang/test/Driver/fopenmp.c
@@ -31,6 +31,10 @@
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-RT
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-OMP
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-GOMP --check-prefix=CHECK-LD-STATIC-GOMP-RT
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5
+//
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -47,6 +51,10 @@
 // RUN: %clang -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
 // RUN: %clang -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
+// RUN: %clang -target x86_64-freebsd -fopenmp=libomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-OMP
+// RUN: %clang -target x86_64-freebsd -fopenmp=libgomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-GOMP --check-prefix=CHECK-LD-STATIC-GOMP-NO-RT
+// RUN: %clang -target x86_64-freebsd -fopenmp=libiomp5 -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5
+//
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -55,6 +63,10 @@
 // RUN: %clang -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
 // RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
+// RUN: %clang -target x86_64-netbsd -fopenmp=libomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-OMP
+// RUN: %clang -target x86_64-netbsd -fopenmp=libgomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-GOMP --check-prefix=CHECK-LD-STATIC-GOMP-NO-RT
+// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5
+//
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -93,6 +105,17 @@
 // CHECK-NO-IOMP5MD: "{{.*}}ld{{(.exe)?}}"
 // CHECK-NO-IOMP5MD-NOT: "-liomp5md"
 //
+// CHECK-LD-STATIC-OMP: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD-STATIC-OMP: "-Bstatic" "-lomp" "-Bdynamic"
+//
+// CHECK-LD-STATIC-GOMP: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD-STATIC-GOMP: "-Bstatic" "-lgomp" "-Bdynamic"
+// CHECK-LD-STATIC-GOMP-RT: "-lrt"
+// CHECK-LD-STATIC-NO-GOMP-RT-NOT: "-lrt"
+//
+// CHECK-LD-STATIC-IOMP5: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD-STATIC-IOMP5: "-Bstatic" "-liomp5" "-Bdynamic"
+//
 // We'd like to check that the default is sane, but until we have the ability
 // to *always* semantically analyze OpenMP without always generating runtime
 // calls (in the event of an unsupported runtime), we don't have a good way to
Index: clang/lib/Dr

[PATCH] D67200: Add -static-openmp driver option

2019-09-06 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama added a comment.

In D67200#1660147 , @srhines wrote:

> Looks really nice. I am sure the NDK developers will be happy to see support 
> for static OpenMP. Do you want to add the public NDK github issue link in the 
> commit message?


Done.

In D67200#1660192 , @MaskRay wrote:

> Edit: Added `-static=` in D53238 . If that 
> is accepted, you may consider `-static=openmp`


Thanks Fangrui!  That change would be very helpful and I'll move to 
`-static=openmp` once it lands.  As for this change, I'll wait over the weekend 
and submit it on Monday to allow everyone a chance to look at it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67200



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


[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

2019-09-06 Thread Thomas Lively via Phabricator via cfe-commits
tlively added a comment.

Thanks for the clarification. It makes sense to me that `-mexception-handling` 
only enables the architectural feature and a separate flag enables the behavior 
change. This is indeed consistent with how `-pthread` works.

What happens when users have exceptions in their code but don't pass 
`-fwasm-exceptions`? Do they get the old SJLJ emulated exception handling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67208



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


[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

2019-09-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added reviewers: hans, rsmith, STL_MSFT.
Herald added a project: clang.

These three modes need to range check enumerator values differently than
C++ does. Before this change, we had two codepaths doing the same thing
in two cases:

1. enum complete (ObjC fixed & Microsoft unfixed)
2. enum incomplete (C99 unfixed)

Now we share the code, but have separate diagnostic paths.

The main functional change is that -fno-ms-compatibility now no longer
sends us down the hard error diagnostic code path for ObjC fixed enums.
Instead, complete-but-not-fixed MS enums go down the C99 codepath which
issues a -Wmicrosoft-enum-value warning about the enum value not being
representable as an 'int'. This was highlighted as the single largest
blocker to compiling windows.h with -fno-ms-compatibility, so this
should make that feasible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67304

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/MicrosoftCompatibility.c
  clang/test/SemaCXX/MicrosoftCompatibility.cpp

Index: clang/test/SemaCXX/MicrosoftCompatibility.cpp
===
--- clang/test/SemaCXX/MicrosoftCompatibility.cpp
+++ clang/test/SemaCXX/MicrosoftCompatibility.cpp
@@ -235,8 +235,8 @@
 
 enum ENUM2 {
 	ENUM2_a = (enum ENUM2) 4,
-	ENUM2_b = 0x9FFF, // expected-warning {{enumerator value is not representable in the underlying type 'int'}}
-	ENUM2_c = 0x1 // expected-warning {{enumerator value is not representable in the underlying type 'int'}}
+	ENUM2_b = 0x9FFF, // expected-warning {{the Microsoft ABI restricts unfixed enumerator values to the range of 'int' (2684354559 is too large)}}
+	ENUM2_c = 0x1 // expected-warning {{the Microsoft ABI restricts unfixed enumerator values to the range of 'int' (4294967296 is too large)}}
 };
 
 namespace NsEnumForwardDecl {
Index: clang/test/Sema/MicrosoftCompatibility.c
===
--- clang/test/Sema/MicrosoftCompatibility.c
+++ clang/test/Sema/MicrosoftCompatibility.c
@@ -7,8 +7,8 @@
 
 enum ENUM2 {
   ENUM2_a = (enum ENUM2) 4,
-  ENUM2_b = 0x9FFF, // expected-warning {{enumerator value is not representable in the underlying type 'int'}}
-  ENUM2_c = 0x1 // expected-warning {{enumerator value is not representable in the underlying type 'int'}}
+  ENUM2_b = 0x9FFF, // expected-warning {{the Microsoft ABI restricts unfixed enumerator values to the range of 'int' (2684354559 is too large)}}
+  ENUM2_c = 0x1 // expected-warning {{the Microsoft ABI restricts unfixed enumerator values to the range of 'int' (4294967296 is too large)}}
 };
 
 __declspec(noreturn) void f6( void ) {
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14423,7 +14423,7 @@
   UPPC_FixedUnderlyingType))
 EnumUnderlying = Context.IntTy.getTypePtr();
 
-} else if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+} else if (Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) {
   // For MSVC ABI compatibility, unfixed enums must use an underlying type
   // of 'int'. However, if this is an unfixed forward declaration, don't set
   // the underlying type unless the user enables -fms-compatibility. This
@@ -16552,8 +16552,7 @@
 if (Enum->isDependentType() || Val->isTypeDependent())
   EltTy = Context.DependentTy;
 else {
-  if (getLangOpts().CPlusPlus11 && Enum->isFixed() &&
-  !getLangOpts().MSVCCompat) {
+  if (getLangOpts().CPlusPlus11 && Enum->isFixed()) {
 // C++11 [dcl.enum]p5: If the underlying type is fixed, [...] the
 // constant-expression in the enumerator-definition shall be a converted
 // constant expression of the underlying type.
@@ -16570,25 +16569,7 @@
  &EnumVal).get())) {
 // C99 6.7.2.2p2: Make sure we have an integer constant expression.
   } else {
-if (Enum->isComplete()) {
-  EltTy = Enum->getIntegerType();
-
-  // In Obj-C and Microsoft mode, require the enumeration value to be
-  // representable in the underlying type of the enumeration. In C++11,
-  // we perform a non-narrowing conversion as part of converted constant
-  // expression checking.
-  if (!isRepresentableIntegerValue(Context, EnumVal, EltTy)) {
-if (getLangOpts().MSVCCompat) {
-  Diag(IdLoc, diag::ext_enumerator_too_large) << EltTy;
-  Val = ImpCastExprToType(Val, EltTy, CK_IntegralCast).get();
-} else
-  Diag(IdLoc, diag::err_enumerator_too_large) << EltTy;
-  } else
-Val = ImpCastExprToType(Val, EltTy,
- 

[PATCH] D67028: Use musttail for variadic method thunks when possible

2019-09-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done.
rnk added inline comments.



Comment at: clang/test/CodeGenCXX/ms-thunks-variadic-return.cpp:9
+struct B : virtual A {
+  // expected-error@+1 2 {{cannot compile this return-adjusting thunk with 
variadic arguments yet}}
+  B *clone(const char *f, ...) override;

I do love the optimism of the CGM.ErrorUnsupported diagnostic: "cannot compile 
this ${unimplementable_feature} yet".

I suppose it could be done if we standardized a new ABI for variadic virtual 
methods with pointer-like return values, so that they emit the main 
implementation under a new symbol name that takes a va_list and then the 
variadic one thunks to it. =P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67028



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


[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

2019-09-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 219191.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Rename function and fix comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65256

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/SemaObjC/Inputs/non-trivial-c-union.h
  test/SemaObjC/non-trivial-c-union.m

Index: test/SemaObjC/non-trivial-c-union.m
===
--- test/SemaObjC/non-trivial-c-union.m
+++ test/SemaObjC/non-trivial-c-union.m
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -I %S/Inputs -verify %s
+
+#include "non-trivial-c-union.h"
 
 typedef union { // expected-note 12 {{'U0' has subobjects that are non-trivial to default-initialize}} expected-note 36 {{'U0' has subobjects that are non-trivial to destruct}} expected-note 28 {{'U0' has subobjects that are non-trivial to copy}}
   id f0; // expected-note 12 {{f0 has type '__strong id' that is non-trivial to default-initialize}} expected-note 36 {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note 28 {{f0 has type '__strong id' that is non-trivial to copy}}
@@ -80,3 +82,7 @@
 void testVolatileLValueToRValue(volatile U0 *a) {
   (void)*a; // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to destruct}} // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to copy}}
 }
+
+void unionInSystemHeader0(U0_SystemHeader);
+
+void unionInSystemHeader1(U1_SystemHeader); // expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to copy}}
Index: test/SemaObjC/Inputs/non-trivial-c-union.h
===
--- /dev/null
+++ test/SemaObjC/Inputs/non-trivial-c-union.h
@@ -0,0 +1,19 @@
+// For backward compatibility, fields of C unions declared in system headers
+// that have non-trivial ObjC ownership qualifications are marked as unavailable
+// unless the qualifier is explicit and __strong.
+
+#pragma clang system_header
+
+typedef __strong id StrongID;
+
+typedef union {
+  id f0;
+  _Nonnull id f1;
+  __weak id f2;
+  StrongID f3;
+} U0_SystemHeader;
+
+typedef union { // expected-note {{'U1_SystemHeader' has subobjects that are non-trivial to destruct}} expected-note {{'U1_SystemHeader' has subobjects that are non-trivial to copy}}
+  __strong id f0; // expected-note {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note {{f0 has type '__strong id' that is non-trivial to copy}}
+  _Nonnull id f1;
+} U1_SystemHeader;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -6027,36 +6027,6 @@
   }
 }
 
-/// Does this type have a "direct" ownership qualifier?  That is,
-/// is it written like "__strong id", as opposed to something like
-/// "typeof(foo)", where that happens to be strong?
-static bool hasDirectOwnershipQualifier(QualType type) {
-  // Fast path: no qualifier at all.
-  assert(type.getQualifiers().hasObjCLifetime());
-
-  while (true) {
-// __strong id
-if (const AttributedType *attr = dyn_cast(type)) {
-  if (attr->getAttrKind() == attr::ObjCOwnership)
-return true;
-
-  type = attr->getModifiedType();
-
-// X *__strong (...)
-} else if (const ParenType *paren = dyn_cast(type)) {
-  type = paren->getInnerType();
-
-// That's it for things we want to complain about.  In particular,
-// we do not want to look through typedefs, typeof(expr),
-// typeof(type), or any other way that the type is somehow
-// abstracted.
-} else {
-
-  return false;
-}
-  }
-}
-
 /// handleObjCOwnershipTypeAttr - Process an objc_ownership
 /// attribute on the specified type.
 ///
@@ -6132,7 +6102,7 @@
   if (Qualifiers::ObjCLifetime previousLifetime
 = type.getQualifiers().getObjCLifetime()) {
 // If it's written directly, that's an error.
-if (hasDirectOwnershipQualifier(type)) {
+if (S.Context.hasDirectOwnershipQualifier(type)) {
   S.Diag(AttrLoc, diag::err_attr_objc_ownership_redundant)
 << type;
   return true;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -15700,27 +15700,11 @@
 
   // Warn about implicitly autorelea

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

2019-09-06 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11144
+  // Ignore unavailable fields since they don't affect the triviality of the
+  // containing struct/union.
+  return FD->hasAttr();

rjmccall wrote:
> The "since" clause here is circular: this function *defines* what affects the 
> triviality.  The comment should talk about the motivations (e.g. fields that 
> aren't available in particular language modes) and why this might be okay or 
> is the best-available option (despite e.g. the potential for ABI 
> incompatibility across language modes),
I commented on the ABI incompatibility issue in `ActOnFields` where the 
unavailable attribute is added to the field.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65256



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


[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-06 Thread Juergen Ributzka via Phabricator via cfe-commits
ributzka added inline comments.



Comment at: clang/lib/Serialization/ASTWriter.cpp:1767
   bool IsTopLevelModuleMap;
+  uint32_t ContentHash[2];
 };

bruno wrote:
> aprantl wrote:
> > Why is this not a uint64_t?
> Serializing a `uint64_t` directly instead of two `uint32_t` gives me a 
> slightly bigger final cache. One could argue that the value is negligible 
> (+800 bytes for a 40MB cache), but here's the rationale :)
Creating an abbrev might fix this, because this should be a fixed size field. 
The generic emission code for a record without an abbrev is not very space 
efficient for single fields.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67249



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


[PATCH] D67028: Use musttail for variadic method thunks when possible

2019-09-06 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67028



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


[PATCH] D67200: Add -static-openmp driver option

2019-09-06 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama added a comment.

I'll update this review addressing @Joerg's reply to cfe-commits:

> Needs testing for the -static interaction?

Thanks @srhines for pointing me to it - I'd only subscribed to cfe-dev and not 
cfe-commits so I'd missed it..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67200



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


r371269 - Use musttail for variadic method thunks when possible

2019-09-06 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Sep  6 15:55:26 2019
New Revision: 371269

URL: http://llvm.org/viewvc/llvm-project?rev=371269&view=rev
Log:
Use musttail for variadic method thunks when possible

This avoids cloning variadic virtual methods when the target supports
musttail and the return type is not covariant. I think we never
implemented this previously because it doesn't handle the covariant
case. But, in the MS ABI, there are some cases where vtable thunks must
be emitted even when the variadic method defintion is not available, so
it looks like we need to implement this. Do it for both ABIs, since it's
a nice size improvement and simplification for Itanium.

Emit an error when emitting thunks for variadic methods with a covariant
return type. This case is essentially not implementable unless the ABI
provides a way to perfectly forward variadic arguments without a tail
call.

Fixes PR43173.

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

Added:
cfe/trunk/test/CodeGenCXX/ms-thunks-variadic-return.cpp
Modified:
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/test/CodeGenCXX/linetable-virtual-variadic.cpp
cfe/trunk/test/CodeGenCXX/thunks.cpp

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=371269&r1=371268&r2=371269&view=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Fri Sep  6 15:55:26 2019
@@ -166,6 +166,15 @@ CodeGenFunction::GenerateVarArgsThunk(ll
   llvm::Value *Callee = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
   llvm::Function *BaseFn = cast(Callee);
 
+  // Cloning can't work if we don't have a definition. The Microsoft ABI may
+  // require thunks when a definition is not available. Emit an error in these
+  // cases.
+  if (!MD->isDefined()) {
+CGM.ErrorUnsupported(MD, "return-adjusting thunk with variadic arguments");
+return Fn;
+  }
+  assert(!BaseFn->isDeclaration() && "cannot clone undefined variadic method");
+
   // Clone to thunk.
   llvm::ValueToValueMapTy VMap;
 
@@ -201,6 +210,8 @@ CodeGenFunction::GenerateVarArgsThunk(ll
   Builder.SetInsertPoint(&*ThisStore);
   llvm::Value *AdjustedThisPtr =
   CGM.getCXXABI().performThisAdjustment(*this, ThisPtr, Thunk.This);
+  AdjustedThisPtr = Builder.CreateBitCast(AdjustedThisPtr,
+  ThisStore->getOperand(0)->getType());
   ThisStore->setOperand(0, AdjustedThisPtr);
 
   if (!Thunk.Return.isEmpty()) {
@@ -291,14 +302,17 @@ void CodeGenFunction::EmitCallAndReturnF
   *this, LoadCXXThisAddress(), Thunk->This)
   : LoadCXXThis();
 
-  if (CurFnInfo->usesInAlloca() || IsUnprototyped) {
-// We don't handle return adjusting thunks, because they require us to call
-// the copy constructor.  For now, fall through and pretend the return
-// adjustment was empty so we don't crash.
+  // If perfect forwarding is required a variadic method, a method using
+  // inalloca, or an unprototyped thunk, use musttail. Emit an error if this
+  // thunk requires a return adjustment, since that is impossible with 
musttail.
+  if (CurFnInfo->usesInAlloca() || CurFnInfo->isVariadic() || IsUnprototyped) {
 if (Thunk && !Thunk->Return.isEmpty()) {
   if (IsUnprototyped)
 CGM.ErrorUnsupported(
 MD, "return-adjusting thunk with incomplete parameter type");
+  else if (CurFnInfo->isVariadic())
+llvm_unreachable("shouldn't try to emit musttail return-adjusting "
+ "thunks for variadic functions");
   else
 CGM.ErrorUnsupported(
 MD, "non-trivial argument copy for return-adjusting thunk");
@@ -549,16 +563,32 @@ llvm::Constant *CodeGenVTables::maybeEmi
 
   CGM.SetLLVMFunctionAttributesForDefinition(GD.getDecl(), ThunkFn);
 
+  // Thunks for variadic methods are special because in general variadic
+  // arguments cannot be perferctly forwarded. In the general case, clang
+  // implements such thunks by cloning the original function body. However, for
+  // thunks with no return adjustment on targets that support musttail, we can
+  // use musttail to perfectly forward the variadic arguments.
+  bool ShouldCloneVarArgs = false;
   if (!IsUnprototyped && ThunkFn->isVarArg()) {
-// Varargs thunks are special; we can't just generate a call because
-// we can't copy the varargs.  Our implementation is rather
-// expensive/sucky at the moment, so don't generate the thunk unless
-// we have to.
-// FIXME: Do something better here; GenerateVarArgsThunk is extremely ugly.
+ShouldCloneVarArgs = true;
+if (TI.Return.isEmpty()) {
+  switch (CGM.getTriple().getArch()) {
+  case llvm::Triple::x86_64:
+  case llvm::Triple::x86:
+  case llvm::Triple::aarch64:
+ShouldCloneVarArgs = false;
+break;
+  default:
+bre

[PATCH] D67028: Use musttail for variadic method thunks when possible

2019-09-06 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371269: Use musttail for variadic method thunks when 
possible (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67028?vs=218995&id=219193#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67028

Files:
  cfe/trunk/lib/CodeGen/CGVTables.cpp
  cfe/trunk/test/CodeGenCXX/linetable-virtual-variadic.cpp
  cfe/trunk/test/CodeGenCXX/ms-thunks-variadic-return.cpp
  cfe/trunk/test/CodeGenCXX/thunks.cpp

Index: cfe/trunk/test/CodeGenCXX/linetable-virtual-variadic.cpp
===
--- cfe/trunk/test/CodeGenCXX/linetable-virtual-variadic.cpp
+++ cfe/trunk/test/CodeGenCXX/linetable-virtual-variadic.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -debug-info-kind=line-tables-only %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -debug-info-kind=line-directives-only %s -o - | FileCheck %s
+// Sparc64 is used because AArch64 and X86_64 would both use musttail.
+// RUN: %clang_cc1 -triple sparc64-linux-gnu -emit-llvm -debug-info-kind=line-tables-only %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple sparc64-linux-gnu -emit-llvm -debug-info-kind=line-directives-only %s -o - | FileCheck %s
 // Crasher for PR22929.
 class Base {
   virtual void VariadicFunction(...);
Index: cfe/trunk/test/CodeGenCXX/thunks.cpp
===
--- cfe/trunk/test/CodeGenCXX/thunks.cpp
+++ cfe/trunk/test/CodeGenCXX/thunks.cpp
@@ -1,6 +1,20 @@
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT %s
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT --check-prefix=CHECK-DBG %s
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=CHECK --check-prefix=CHECK-OPT %s
+// Sparc64 doesn't support musttail (yet), so it uses method cloning for
+// variadic thunks. Use it for testing.
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - \
+// RUN: | FileCheck --check-prefixes=CHECK,CHECK-CLONE,CHECK-NONOPT %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - \
+// RUN: | FileCheck --check-prefixes=CHECK,CHECK-CLONE,CHECK-NONOPT,CHECK-DBG %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes \
+// RUN: | FileCheck --check-prefixes=CHECK,CHECK-CLONE,CHECK-OPT %s
+
+// Test x86_64, which uses musttail for variadic thunks.
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes \
+// RUN: | FileCheck --check-prefixes=CHECK,CHECK-TAIL,CHECK-OPT %s
+
+// Finally, reuse these tests for the MS ABI.
+// RUN: %clang_cc1 %s -triple=x86_64-windows-msvc -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes \
+// RUN: | FileCheck --check-prefixes=WIN64 %s
+
 
 namespace Test1 {
 
@@ -23,6 +37,11 @@
 // CHECK-LABEL: define void @_ZThn8_N5Test11C1fEv(
 // CHECK-DBG-NOT: dbg.declare
 // CHECK: ret void
+//
+// WIN64-LABEL: define dso_local void @"?f@C@Test1@@UEAAXXZ"(
+// WIN64-LABEL: define linkonce_odr dso_local void @"?f@C@Test1@@W7EAAXXZ"(
+// WIN64: getelementptr i8, i8* {{.*}}, i32 -8
+// WIN64: ret void
 void C::f() { }
 
 }
@@ -45,6 +64,10 @@
 // CHECK: ret void
 void B::f() { }
 
+// No thunk is used for this case in the MS ABI.
+// WIN64-LABEL: define dso_local void @"?f@B@Test2@@UEAAXXZ"(
+// WIN64-NOT: define {{.*}} void @"?f@B@Test2
+
 }
 
 namespace Test3 {
@@ -65,6 +88,7 @@
 };
 
 // CHECK: define %{{.*}}* @_ZTch0_v0_n24_N5Test31B1fEv(
+// WIN64: define weak_odr dso_local %{{.*}} @"?f@B@Test3@@QEAAPEAUV1@2@XZ"(
 V2 *B::f() { return 0; }
 
 }
@@ -92,6 +116,10 @@
 // CHECK: ret void
 void C::f() { }
 
+// Visibility doesn't matter on COFF, but whatever. We could add an ELF test
+// mode later.
+// WIN64-LABEL: define protected void @"?f@C@Test4@@UEAAXXZ"(
+// WIN64-LABEL: define linkonce_odr protected void @"?f@C@Test4@@W7EAAXXZ"(
 }
 
 // Check that the thunk gets internal linkage.
@@ -119,6 +147,8 @@
 c.f();
   }
 }
+// Not sure why this isn't delayed like in Itanium.
+// WIN64-LABEL: define internal void @"?f@C@?A0xAEF74CE7@Test4B@@UEAAXXZ"(
 
 namespace Test5 {
 
@@ -134,6 +164,7 @@
 void f(B b) {
   b.f();
 }
+// No thunk in MS ABI in this case.
 }
 
 namespace Test6 {
@@ -178,6 +209,10 @@
   // CHECK: {{call void @_ZN5Test66Thunks1fEv.*sret}}
   // CHECK: ret void
   X Thu

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

2019-09-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 219194.
vsapsai added a comment.

- Add a test for unused absolute path in a header map; simplify code.


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

https://reviews.llvm.org/D58094

Files:
  clang/include/clang/Lex/DirectoryLookup.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
  clang/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h
  clang/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h
  clang/test/Preprocessor/nonportable-include-with-hmap.c

Index: clang/test/Preprocessor/nonportable-include-with-hmap.c
===
--- clang/test/Preprocessor/nonportable-include-with-hmap.c
+++ clang/test/Preprocessor/nonportable-include-with-hmap.c
@@ -1,5 +1,9 @@
+// REQUIRES: shell
+
 // RUN: rm -f %t.hmap
-// RUN: %hmaptool write %S/Inputs/nonportable-hmaps/foo.hmap.json %t.hmap
+// RUN: sed -e "s:INPUTS_DIR:%S/Inputs:g" \
+// RUN:   %S/Inputs/nonportable-hmaps/foo.hmap.json > %t.hmap.json
+// RUN: %hmaptool write %t.hmap.json %t.hmap
 // RUN: %clang_cc1 -Eonly\
 // RUN:   -I%t.hmap \
 // RUN:   -I%S/Inputs/nonportable-hmaps  \
@@ -15,4 +19,16 @@
 //  5. Return.
 //
 // There is nothing nonportable; -Wnonportable-include-path should not fire.
-#include "Foo/Foo.h" // expected-no-diagnostics
+#include "Foo/Foo.h" // no warning
+
+// Verify files with absolute paths in the header map are handled too.
+// "Bar.h" is included twice to make sure that when we see potentially
+// nonportable path, the file has been already discovered through a relative
+// path which triggers the file to be opened and `FileEntry::RealPathName`
+// to be set.
+#include "Bar.h"
+#include "Foo/Bar.h" // no warning
+
+// But the presence of the absolute path in the header map is not enough. If we
+// didn't use it to discover a file, shouldn't suppress the warning.
+#include "headers/Foo/Baz.h" // expected-warning {{non-portable path}}
Index: clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
===
--- clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
+++ clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
@@ -1,6 +1,9 @@
 {
   "mappings" :
 {
- "Foo/Foo.h" : "headers/foo/Foo.h"
+ "Foo/Foo.h" : "headers/foo/Foo.h",
+ "Bar.h" : "headers/foo/Bar.h",
+ "Foo/Bar.h" : "INPUTS_DIR/nonportable-hmaps/headers/foo/Bar.h",
+ "headers/Foo/Baz.h" : "/not/existing/absolute/path/Baz.h"
 }
 }
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -341,9 +341,10 @@
 SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath,
 Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
 bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
-bool &HasBeenMapped, SmallVectorImpl &MappedName) const {
+bool &IsInHeaderMap, SmallVectorImpl &MappedName) const {
   InUserSpecifiedSystemFramework = false;
-  HasBeenMapped = false;
+  IsInHeaderMap = false;
+  MappedName.clear();
 
   SmallString<1024> TmpDir;
   if (isNormalDir()) {
@@ -377,6 +378,8 @@
   if (Dest.empty())
 return None;
 
+  IsInHeaderMap = true;
+
   auto FixupSearchPath = [&]() {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
@@ -393,10 +396,8 @@
   // ("Foo.h" -> "Foo/Foo.h"), in which case continue header lookup using the
   // framework include.
   if (llvm::sys::path::is_relative(Dest)) {
-MappedName.clear();
 MappedName.append(Dest.begin(), Dest.end());
 Filename = StringRef(MappedName.begin(), MappedName.size());
-HasBeenMapped = true;
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
   FixupSearchPath();
@@ -883,18 +884,22 @@
   // Check each directory in sequence to see if it contains this file.
   for (; i != SearchDirs.size(); ++i) {
 bool InUserSpecifiedSystemFramework = false;
-bool HasBeenMapped = false;
+bool IsInHeaderMap = false;
 bool IsFrameworkFoundInDir = false;
 Optional File = SearchDirs[i].LookupFile(
 Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
 SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
-HasBeenMapped, MappedName);
-if (HasBeenMapped) {
+IsInHeaderMap, MappedName);
+if (!MappedName.empty()) {
+  assert(IsInHeaderMap && "MappedName should come from a header map");
   CacheLookup.MappedName =
-  copyString(Filename, LookupFileCache.getAllocator());
-  if (IsMapped)
-*IsMapped = true;
+  copyString(MappedName, LookupFileCache.getAllocator());
 }
+if (IsMapped)
+  // A filename is mapped when a header map remapped it to a relative path

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

2019-09-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai marked 4 inline comments as done.
vsapsai added a comment.

Updated the code. Hope it is easier to understand now.




Comment at: clang/lib/Lex/HeaderSearch.cpp:908-909
 
+if (IsMapped)
+  *IsMapped = CurrentInHeaderMap || HasBeenMapped;
+

dexonsmith wrote:
> Are you intentionally delaying this until after the `if (!File)` check?  If 
> so, why?
Agree it is non-obvious and error-prone when changing the code. Changed it.


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

https://reviews.llvm.org/D58094



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


[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

2019-09-06 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay, thanks.  LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65256



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


[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

2019-09-06 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin added a comment.

In D67208#1661694 , @tlively wrote:

> What happens when users have exceptions in their code but don't pass  Do they 
> get the old SJLJ emulated exception handling?


The current emscripten EH is enabled by `-mllvm 
-enable-emscripten-cxx-exceptions`, which is not a clang flag (it's passed by 
`-mllvm`). The reason it is not a clang flag is emscripten EH does not need any 
action in clang. If  users use exceptions but don't pass `-fwasm-exceptions`, 
if they pass `-mllvm -enable-emscripten-cxx-exceptions` instead, emscripten EH 
will be used. If they pass neither, neither EH will be used and all invokes 
will be lowered to calls.

Having said that, `-fwasm-exceptions` and `-mllvm 
-enable-emscripten-cxx-exceptions` are not compatible. If we can check this 
that'd be better. I'll check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67208



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


[PATCH] D67200: Add -static-openmp driver option

2019-09-06 Thread Pirama Arumuga Nainar via Phabricator via cfe-commits
pirama updated this revision to Diff 219200.
pirama edited the summary of this revision.
pirama added a comment.

Test -static, -static-openmp interaction.  Added these only for iomp5 to avoid 
test-case explosion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67200

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Driver/fopenmp.c

Index: clang/test/Driver/fopenmp.c
===
--- clang/test/Driver/fopenmp.c
+++ clang/test/Driver/fopenmp.c
@@ -31,6 +31,11 @@
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-RT
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-OMP
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-GOMP --check-prefix=CHECK-LD-STATIC-GOMP-RT
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5
+// RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 -static -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC
+//
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -47,6 +52,11 @@
 // RUN: %clang -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
 // RUN: %clang -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
+// RUN: %clang -target x86_64-freebsd -fopenmp=libomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-OMP
+// RUN: %clang -target x86_64-freebsd -fopenmp=libgomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-GOMP --check-prefix=CHECK-LD-STATIC-GOMP-NO-RT
+// RUN: %clang -target x86_64-freebsd -fopenmp=libiomp5 -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5
+// RUN: %clang -target x86_64-freebsd -fopenmp=libiomp5 -static -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC
+//
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -55,6 +65,11 @@
 // RUN: %clang -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
 // RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
+// RUN: %clang -target x86_64-netbsd -fopenmp=libomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-OMP
+// RUN: %clang -target x86_64-netbsd -fopenmp=libgomp -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-GOMP --check-prefix=CHECK-LD-STATIC-GOMP-NO-RT
+// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5
+// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 -static -static-openmp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC
+//
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -93,6 +108,22 @@
 // CHECK-NO-IOMP5MD: "{{.*}}ld{{(.exe)?}}"
 // CHECK-NO-IOMP5MD-NOT: "-liomp5md"
 //
+// CHECK-LD-STATIC-OMP: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD-STATIC-OMP: "-Bstatic" "-lomp" "-Bdynamic"
+//
+// CHECK-LD-STATIC-GOM

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

2019-09-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

This patch set the goal of splitting `BugReport` into two different report 
kinds, and I think it did that well. Not only that, we drastically improved the 
documentation, formalized many things that weren't in the core before, so I 
wouldn't like you to bear the burdern of never ending rebases (it doesn't make 
reviewing easier either). Let's work on the rest of the code in a followup 
patch.

I agree with @gribozavr, we could do better, and should do better. Many core 
classes in the analyzer feel like everyone just added 1-2 functions, 1-2 
branches to get something done quickly, and while those functions in the 
context of the patch they were added in may have been obvious, it lead us to a 
cluster on non-descriptive, undocumented, confusing interface and 
implementation code (n+1 location related functions in this instance). While 
adding new checkers, better support for C++17 and whatnot is what will 
ultimately make the end user experience better, I like how we're investing a 
lot of effort into the health of the codebase nowadays, and I think it'll pay 
off in the long term.


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

https://reviews.llvm.org/D66572



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


[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

2019-09-06 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment.

> But TLDR, either the fix in https://github.com/google/filament/pull/1566
>  is incorrect and the actually-bad code is elsewhere,
>  or you have some other unsanitized UB elsewhere. Could be both :S

My money is on "both" :p

I tested a random sample of a couple thousand tests internally and ~1% of them 
failed, but when I looked at them, they were all coming from two separate 
widely used libraries. I fixed those, and I'll do a broader test now to see how 
bad the long tail of issues are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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


[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

2019-09-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

But, of course, let's wait for @gribozavr to give his blessings as well, I'm 
only accepting because removing/changing other parts of the API seems to 
deserve a separate revision :)


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

https://reviews.llvm.org/D66572



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


[PATCH] D67208: [WebAssembly] Add -fwasm-exceptions for wasm EH

2019-09-06 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 219203.
aheejin added a comment.

- Error out when -fwasm-exceptions is specified with 
-enable-emscripten-cxx-exceptions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67208

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/wasm-eh.cpp
  clang/test/Driver/wasm-toolchain.c

Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -73,6 +73,25 @@
 // RUN:   | FileCheck -check-prefix=PTHREAD_NO_MUT_GLOBALS %s
 // PTHREAD_NO_MUT_GLOBALS: invalid argument '-pthread' not allowed with '-mno-mutable-globals'
 
+// '-fwasm-exceptions' sets +exception-handling
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN:--sysroot=/foo %s -fwasm-exceptions 2>&1 \
+// RUN:  | FileCheck -check-prefix=WASM_EXCEPTIONS %s
+// WASM_EXCEPTIONS: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+exception-handling"
+
+// '-fwasm-exceptions' not allowed with '-mno-exception-handling'
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -fwasm-exceptions -mno-exception-handling 2>&1 \
+// RUN:   | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_EH %s
+// WASM_EXCEPTIONS_NO_EH: invalid argument '-fwasm-exceptions' not allowed with '-mno-exception-handling'
+
+// '-fwasm-exceptions' not allowed with
+// '-mllvm -enable-emscripten-cxx-exceptions'
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -fwasm-exceptions -mllvm -enable-emscripten-cxx-exceptions 2>&1 \
+// RUN:   | FileCheck -check-prefix=WASM_EXCEPTIONS_EMSCRIPTEN_EH %s
+// WASM_EXCEPTIONS_EMSCRIPTEN_EH: invalid argument '-fwasm-exceptions' not allowed with '-mllvm -enable-emscripten-cxx-exceptions'
+
 // RUN: %clang %s -### -fsanitize=address -target wasm32-unknown-emscripten 2>&1 | FileCheck -check-prefix=CHECK-ASAN-EMSCRIPTEN %s
 // CHECK-ASAN-EMSCRIPTEN: "-fsanitize=address"
 // CHECK-ASAN-EMSCRIPTEN: "-fsanitize-address-globals-dead-stripping"
Index: clang/test/CodeGenCXX/wasm-eh.cpp
===
--- clang/test/CodeGenCXX/wasm-eh.cpp
+++ clang/test/CodeGenCXX/wasm-eh.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -target-feature +exception-handling -emit-llvm -o - -std=c++11 | FileCheck %s
-// RUN: %clang_cc1 %s -triple wasm64-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -target-feature +exception-handling -emit-llvm -o - -std=c++11 | FileCheck %s
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -fwasm-exceptions -target-feature +exception-handling -emit-llvm -o - -std=c++11 | FileCheck %s
+// RUN: %clang_cc1 %s -triple wasm64-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -fwasm-exceptions -target-feature +exception-handling -emit-llvm -o - -std=c++11 | FileCheck %s
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -fwasm-exceptions -target-feature +exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
 
 void may_throw();
 void dont_throw() noexcept;
@@ -382,3 +383,11 @@
 // CHECK:   cleanupret from %[[CLEANUPPAD1]] unwind to caller
 
 // CHECK:   unreachable
+
+// Here we only check if the command enables wasm exception handling in the
+// backend so that exception handling instructions can be generated in .s file.
+
+// ASSEMBLY: try
+// ASSEMBLY: catch
+// ASSEMBLY: rethrow
+// ASSEMBLY: end_try
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2686,9 +2686,9 @@
   Opts.FixedPoint;
 
   // Handle exception personalities
-  Arg *A = Args.getLastArg(options::OPT_fsjlj_exceptions,
-   options::OPT_fseh_exceptions,
-   options::OPT_fdwarf_exceptions);
+  Arg *A = Args.getLastArg(
+  options::OPT_fsjlj_exceptions, options::OPT_fseh_exceptions,
+  options::OPT_fdwarf_exceptions, options::OPT_fwasm_exceptions);
   if (A) {
 const Option &Opt = A->getOption();
 llvm::Triple T(TargetOpts.Triple);
@@ -2699,6 +2699,7 @@
 Opts.SjLjExceptions = Opt.matches(options::OPT_fsjlj_exceptions);
 Opts.SEHExceptions = Opt.matches(options::OPT_fseh_exceptions);
 Opts.DWARFExceptions = Opt.matches(options::OPT_fdwarf_exce

[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

2019-09-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Nice! Thank you so much for sorting this out! I think this will make 
reviewing far more accessible for newcomers to the IteratorChecker family. I 
have a couple nits to comment on, but I won't clutter the code just yet. @NoQ, 
do you have any high level objections to this?




Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1689
+  if (CE->getNumArgs() == 0) {
+reportDebugMsg("Missing container argument", C);
+return;

Ah, right, so this would fire for `clang_analyzer_iterator_position()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67156



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


[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

2019-09-06 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Yup, thanks, this is really nice!




Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:243-244
+  Getter get) const;
+  void analyzerContainerBegin(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerContainerEnd(const CallExpr *CE, CheckerContext &C) const;
+  template 

We usually define such getters for stuff that the programmer cannot obtain 
otherwise during normal program execution. These two functions look like 
they're probably equivalent to normal `.begin()` and `.end()` calls. I don't 
really object but do we really ever need them other than for testing the 
trivial implementations of `.begin()` and `.end()`?



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1659-1660
+Handler = llvm::StringSwitch(C.getCalleeName(CE))
+  .Case("clang_analyzer_container_begin",
+&IteratorChecker::analyzerContainerBegin)
+  .Case("clang_analyzer_container_end",

`CallDescriptionMap` please? ^.^



Comment at: test/Analysis/iterator-inspection.cpp:41-42
+
+  clang_analyzer_dump(&v0); //expected-warning{{&v0}}
+  clang_analyzer_dump(clang_analyzer_iterator_container(b0)); 
//expected-warning{{&v0}}
+}

Slightly more robust: 
`clang_analyzer_eval(clang_analyzer_iterator_container(b0) == &v0); // 
expected-warning{{TRUE}}`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67156



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


[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

2019-09-06 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:1328-1337
+def ContainerInspectionChecker : Checker<"ContainerInspection">,
+  HelpText<"Check the analyzer's understanding of C++ containers">,
+  Dependencies<[IteratorModeling]>,
+  Documentation;
+
+def IteratorInspectionChecker : Checker<"IteratorInspection">,
+  HelpText<"Check the analyzer's understanding of C++ iterators">,

Dunno, i would keep it all in one checker, just to save a few `// RUN:` lines :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67156



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


[PATCH] D63960: [C++20] Add consteval-specifique semantic for functions

2019-09-06 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 219205.
Tyker retitled this revision from "[C++20] Add consteval-specifique semantic" 
to "[C++20] Add consteval-specifique semantic for functions".
Tyker added a comment.
Herald added a subscriber: mgrang.

I narrowed the patch because it was getting quite big. this patch only handle 
consteval function, not constructors.

Changes:

- keep track of DeclRefExpr on consteval decaltions.
- keep track of candidates for immediate invocation.
- detect and remove nested immediate invocation candidates
- detect and remove DeclRefExpr on consteval declarations inside immediate 
invocations.
- diagnose non removed DeclRefExpr on consteval declarations and candidates for 
immediate invocation.
- fixe existing bug where defaulted consteval special members where constexpr 
instead of consteval.
- improve tests.


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

https://reviews.llvm.org/D63960

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -12,6 +12,7 @@
 }
 
 constexpr auto l_eval = [](int i) consteval {
+// expected-note@-1+ {{declared here}}
 
   return i;
 };
@@ -23,6 +24,7 @@
 
 struct A {
   consteval int f1(int i) const {
+// expected-note@-1 {{declared here}}
 return i;
   }
   consteval A(int i);
@@ -56,3 +58,183 @@
 consteval int main() { // expected-error {{'main' is not allowed to be declared consteval}}
   return 0;
 }
+
+consteval int f_eval(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+namespace taking_address {
+
+using func_type = int(int);
+
+func_type* p1 = (&f_eval);
+// expected-error@-1 {{take address}}
+func_type* p7 = __builtin_addressof(f_eval);
+// expected-error@-1 {{take address}}
+
+auto p = f_eval;
+// expected-error@-1 {{take address}}
+
+auto m1 = &basic_sema::A::f1;
+// expected-error@-1 {{take address}}
+auto l1 = &decltype(basic_sema::l_eval)::operator();
+// expected-error@-1 {{take address}}
+
+consteval int f(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+auto ptr = &f;
+// expected-error@-1 {{take address}}
+
+auto f1() {
+  return &f;
+// expected-error@-1 {{take address}}
+}
+
+}
+
+namespace invalid_function {
+using size_t = unsigned long;
+struct A {
+  consteval void *operator new(size_t count);
+  // expected-error@-1 {{operator new cannot be declared consteval}}
+  consteval void *operator new[](size_t count);
+  // expected-error@-1 {{operator new[] cannot be declared consteval}}
+  consteval void operator delete(void* ptr);
+  // expected-error@-1 {{operator delete cannot be declared consteval}}
+  consteval void operator delete[](void* ptr);
+  // expected-error@-1 {{operator delete[] cannot be declared consteval}}
+  consteval ~A();
+  // expected-error@-1 {{destructor cannot be marked consteval}}
+};
+
+}
+
+namespace nested {
+consteval int f() {
+  return 0;
+}
+
+consteval int f1(...) {
+  return 1;
+}
+
+enum E {};
+
+using T = int(&)();
+
+consteval auto operator+ (E, int(*a)()) {
+  return 0;
+}
+
+void d() {
+  auto i = f1(E() + &f);
+}
+
+auto l0 = [](auto) consteval {
+  return 0;
+};
+
+int i0 = l0(&f1);
+
+int i1 = f1(l0(4));
+
+}
+
+namespace user_defined_literal {
+
+consteval int operator"" _test(unsigned long long i) {
+// expected-note@-1+ {{declared here}}
+  return 0;
+}
+
+int i = 0_test;
+
+auto ptr = &operator"" _test;
+// expected-error@-1 {{take address}}
+
+}
+
+namespace return_address {
+
+consteval int f() {
+  return 0;
+}
+
+consteval int(*ret1(int i))() {
+  return &f;
+}
+
+auto ptr = ret1(0);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{pointer on a consteval}}
+
+struct A {
+  consteval int f(int) {
+return 0;
+  }
+};
+
+using mem_ptr_type = int (A::*)(int);
+
+template
+struct C {};
+
+C<&A::f> c;
+// expected-error@-1 {{is not a constant expression}}
+// expected-note@-2 {{pointer on a consteval}}
+
+consteval mem_ptr_type ret2() {
+  return &A::f;
+}
+
+C c1;
+// expected-error@-1 {{is not a constant expression}}
+// expected-note@-2 {{pointer on a consteval}}
+
+}
+
+namespace context {
+
+int g_i;
+// expected-note@-1 {{declared here}}
+
+consteval int f(int) {
+  return 0;
+}
+
+constexpr int c_i = 0;
+
+int t1 = f(g_i);
+// expected-error@-1 {{could not be evaluated}}
+

r371276 - [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership

2019-09-06 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Fri Sep  6 17:34:47 2019
New Revision: 371276

URL: http://llvm.org/viewvc/llvm-project?rev=371276&view=rev
Log:
[Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership
qualifications as unavailable if the union is declared in a system
header

r365985 stopped marking those fields as unavailable, which caused the
union's NonTrivialToPrimitive* bits to be set to true. This patch
restores the behavior prior to r365985, except that users can explicitly
specify the ownership qualification of the field to instruct the
compiler not to mark it as unavailable.

rdar://problem/53420753

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

Added:
cfe/trunk/test/SemaObjC/Inputs/non-trivial-c-union.h
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaObjC/non-trivial-c-union.m

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=371276&r1=371275&r2=371276&view=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Fri Sep  6 17:34:47 2019
@@ -2045,6 +2045,11 @@ public:
   /// types.
   bool areCompatibleVectorTypes(QualType FirstVec, QualType SecondVec);
 
+  /// Return true if the type has been explicitly qualified with ObjC 
ownership.
+  /// A type may be implicitly qualified with ownership under ObjC ARC, and in
+  /// some cases the compiler treats these differently.
+  bool hasDirectOwnershipQualifier(QualType Ty) const;
+
   /// Return true if this is an \c NSObject object with its \c NSObject
   /// attribute set.
   static bool isObjCNSObjectType(QualType Ty) {

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=371276&r1=371275&r2=371276&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Sep  6 17:34:47 2019
@@ -7949,6 +7949,28 @@ bool ASTContext::areCompatibleVectorType
   return false;
 }
 
+bool ASTContext::hasDirectOwnershipQualifier(QualType Ty) const {
+  while (true) {
+// __strong id
+if (const AttributedType *Attr = dyn_cast(Ty)) {
+  if (Attr->getAttrKind() == attr::ObjCOwnership)
+return true;
+
+  Ty = Attr->getModifiedType();
+
+// X *__strong (...)
+} else if (const ParenType *Paren = dyn_cast(Ty)) {
+  Ty = Paren->getInnerType();
+
+// We do not want to look through typedefs, typeof(expr),
+// typeof(type), or any other way that the type is somehow
+// abstracted.
+} else {
+  return false;
+}
+  }
+}
+
 
//===--===//
 // ObjCQualifiedIdTypesAreCompatible - Compatibility testing for qualified 
id's.
 
//===--===//

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=371276&r1=371275&r2=371276&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Sep  6 17:34:47 2019
@@ -11255,6 +11255,15 @@ void Sema::checkNonTrivialCUnionInInitia
 
 namespace {
 
+bool shouldIgnoreForRecordTriviality(const FieldDecl *FD) {
+  // Ignore unavailable fields. A field can be marked as unavailable explicitly
+  // in the source code or implicitly by the compiler if it is in a union
+  // defined in a system header and has non-trivial ObjC ownership
+  // qualifications. We don't want those fields to participate in determining
+  // whether the containing union is non-trivial.
+  return FD->hasAttr();
+}
+
 struct DiagNonTrivalCUnionDefaultInitializeVisitor
 : 
DefaultInitializedTypeVisitor {
@@ -11308,7 +11317,8 @@ struct DiagNonTrivalCUnionDefaultInitial
   << 0 << 0 << QT.getUnqualifiedType() << "";
 
 for (const FieldDecl *FD : RD->fields())
-  asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+  if (!shouldIgnoreForRecordTriviality(FD))
+asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
   }
 
   void visitTrivial(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) 
{}
@@ -11372,7 +11382,8 @@ struct DiagNonTrivalCUnionDestructedType
   << 0 << 1 << QT.getUnqualifiedType() << "";
 
 for (const FieldDecl *FD : RD->fields())
-  asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+  if (!shouldIgnoreForRecordTriviality(FD))
+asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
   }
 
   void visitTrivial(QualType QT, const FieldDecl

r371275 - [Sema] Diagnose default-initialization, destruction, and copying of

2019-09-06 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Fri Sep  6 17:34:43 2019
New Revision: 371275

URL: http://llvm.org/viewvc/llvm-project?rev=371275&view=rev
Log:
[Sema] Diagnose default-initialization, destruction, and copying of
non-trivial C union types

This recommits r365985, which was reverted because it broke a few
projects using unions containing non-trivial ObjC pointer fields in
system headers. We now have a patch to fix the problem (see
https://reviews.llvm.org/D65256).

Original commit message:

This patch diagnoses uses of non-trivial C unions and structs/unions
containing non-trivial C unions in the following contexts, which require
default-initialization, destruction, or copying of the union objects,
instead of disallowing fields of non-trivial types in C unions, which is
what we currently do:

- function parameters.
- function returns.
- assignments.
- compound literals.
- block captures except capturing of `__block` variables by non-escaping blocks.
- local and global variable definitions.
- lvalue-to-rvalue conversions of volatile types.

See the discussion in https://reviews.llvm.org/D62988 for more background.

rdar://problem/50679094

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

Added:
cfe/trunk/test/PCH/non-trivial-c-union.m
cfe/trunk/test/SemaObjC/non-trivial-c-union.m
Removed:
cfe/trunk/test/CodeGenObjC/Inputs/strong_in_union.h
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m
cfe/trunk/test/SemaObjC/arc-decls.m

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=371275&r1=371274&r2=371275&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Fri Sep  6 17:34:43 2019
@@ -3754,6 +3754,30 @@ public:
 RecordDeclBits.NonTrivialToPrimitiveDestroy = V;
   }
 
+  bool hasNonTrivialToPrimitiveDefaultInitializeCUnion() const {
+return RecordDeclBits.HasNonTrivialToPrimitiveDefaultInitializeCUnion;
+  }
+
+  void setHasNonTrivialToPrimitiveDefaultInitializeCUnion(bool V) {
+RecordDeclBits.HasNonTrivialToPrimitiveDefaultInitializeCUnion = V;
+  }
+
+  bool hasNonTrivialToPrimitiveDestructCUnion() const {
+return RecordDeclBits.HasNonTrivialToPrimitiveDestructCUnion;
+  }
+
+  void setHasNonTrivialToPrimitiveDestructCUnion(bool V) {
+RecordDeclBits.HasNonTrivialToPrimitiveDestructCUnion = V;
+  }
+
+  bool hasNonTrivialToPrimitiveCopyCUnion() const {
+return RecordDeclBits.HasNonTrivialToPrimitiveCopyCUnion;
+  }
+
+  void setHasNonTrivialToPrimitiveCopyCUnion(bool V) {
+RecordDeclBits.HasNonTrivialToPrimitiveCopyCUnion = V;
+  }
+
   /// Determine whether this class can be passed in registers. In C++ mode,
   /// it must have at least one trivial, non-deleted copy or move constructor.
   /// FIXME: This should be set as part of completeDefinition.

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=371275&r1=371274&r2=371275&view=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Fri Sep  6 17:34:43 2019
@@ -1440,6 +1440,13 @@ class DeclContext {
 uint64_t NonTrivialToPrimitiveCopy : 1;
 uint64_t NonTrivialToPrimitiveDestroy : 1;
 
+/// The following bits indicate whether this is or contains a C union that
+/// is non-trivial to default-initialize, destruct, or copy. These bits
+/// imply the associated basic non-triviality predicates declared above.
+uint64_t HasNonTrivialToPrimitiveDefaultInitializeCUnion : 1;
+uint64_t HasNonTrivialToPrimitiveDestructCUnion : 1;
+uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;
+
 /// Indicates whether this struct is destroyed in the callee.
 uint64_t ParamDestroyedInCallee : 1;
 
@@ -1448,7 +1455,7 @@ class DeclContext {
   };
 
   /// Number of non-inherited bits in RecordDeclBitfields.
-  enum { NumRecordDeclBits = 11 };
+  enum { NumRecordDeclBits = 14 };
 
   /// Stores the bits used by OMPDeclareReductionDecl.
   /// If modified NumOMPDeclareReductionDeclBits and the accessor

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=371275&r1=371274&r2=371275&view=diff
==

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

2019-09-06 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371276: [Sema][ObjC] Mark C union fields that have 
non-trivial ObjC ownership (authored by ahatanak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65256?vs=219191&id=219206#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65256

Files:
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/SemaObjC/Inputs/non-trivial-c-union.h
  cfe/trunk/test/SemaObjC/non-trivial-c-union.m

Index: cfe/trunk/test/SemaObjC/non-trivial-c-union.m
===
--- cfe/trunk/test/SemaObjC/non-trivial-c-union.m
+++ cfe/trunk/test/SemaObjC/non-trivial-c-union.m
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -I %S/Inputs -verify %s
+
+#include "non-trivial-c-union.h"
 
 typedef union { // expected-note 12 {{'U0' has subobjects that are non-trivial to default-initialize}} expected-note 36 {{'U0' has subobjects that are non-trivial to destruct}} expected-note 28 {{'U0' has subobjects that are non-trivial to copy}}
   id f0; // expected-note 12 {{f0 has type '__strong id' that is non-trivial to default-initialize}} expected-note 36 {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note 28 {{f0 has type '__strong id' that is non-trivial to copy}}
@@ -80,3 +82,7 @@
 void testVolatileLValueToRValue(volatile U0 *a) {
   (void)*a; // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to destruct}} // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to copy}}
 }
+
+void unionInSystemHeader0(U0_SystemHeader);
+
+void unionInSystemHeader1(U1_SystemHeader); // expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to copy}}
Index: cfe/trunk/test/SemaObjC/Inputs/non-trivial-c-union.h
===
--- cfe/trunk/test/SemaObjC/Inputs/non-trivial-c-union.h
+++ cfe/trunk/test/SemaObjC/Inputs/non-trivial-c-union.h
@@ -0,0 +1,19 @@
+// For backward compatibility, fields of C unions declared in system headers
+// that have non-trivial ObjC ownership qualifications are marked as unavailable
+// unless the qualifier is explicit and __strong.
+
+#pragma clang system_header
+
+typedef __strong id StrongID;
+
+typedef union {
+  id f0;
+  _Nonnull id f1;
+  __weak id f2;
+  StrongID f3;
+} U0_SystemHeader;
+
+typedef union { // expected-note {{'U1_SystemHeader' has subobjects that are non-trivial to destruct}} expected-note {{'U1_SystemHeader' has subobjects that are non-trivial to copy}}
+  __strong id f0; // expected-note {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note {{f0 has type '__strong id' that is non-trivial to copy}}
+  _Nonnull id f1;
+} U1_SystemHeader;
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -7949,6 +7949,28 @@
   return false;
 }
 
+bool ASTContext::hasDirectOwnershipQualifier(QualType Ty) const {
+  while (true) {
+// __strong id
+if (const AttributedType *Attr = dyn_cast(Ty)) {
+  if (Attr->getAttrKind() == attr::ObjCOwnership)
+return true;
+
+  Ty = Attr->getModifiedType();
+
+// X *__strong (...)
+} else if (const ParenType *Paren = dyn_cast(Ty)) {
+  Ty = Paren->getInnerType();
+
+// We do not want to look through typedefs, typeof(expr),
+// typeof(type), or any other way that the type is somehow
+// abstracted.
+} else {
+  return false;
+}
+  }
+}
+
 //===--===//
 // ObjCQualifiedIdTypesAreCompatible - Compatibility testing for qualified id's.
 //===--===//
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -11255,6 +11255,15 @@
 
 namespace {
 
+bool shouldIgnoreForRecordTriviality(const FieldDecl *FD) {
+  // Ignore unavailable fields. A field can be marked as unavailable explicitly
+  // in the source

r371277 - Fix thunks.cpp test, don't FileCheck for anon namespace id

2019-09-06 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Sep  6 17:41:08 2019
New Revision: 371277

URL: http://llvm.org/viewvc/llvm-project?rev=371277&view=rev
Log:
Fix thunks.cpp test, don't FileCheck for anon namespace id

The anon namespace id is a hash of the main input path to the compiler,
which varies in the test suite because the input path is absolute.

Modified:
cfe/trunk/test/CodeGenCXX/thunks.cpp

Modified: cfe/trunk/test/CodeGenCXX/thunks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/thunks.cpp?rev=371277&r1=371276&r2=371277&view=diff
==
--- cfe/trunk/test/CodeGenCXX/thunks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/thunks.cpp Fri Sep  6 17:41:08 2019
@@ -148,7 +148,7 @@ namespace Test4B {
   }
 }
 // Not sure why this isn't delayed like in Itanium.
-// WIN64-LABEL: define internal void @"?f@C@?A0xAEF74CE7@Test4B@@UEAAXXZ"(
+// WIN64-LABEL: define internal void @"?f@C@?A{{.*}}@Test4B@@UEAAXXZ"(
 
 namespace Test5 {
 


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


[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

2019-09-06 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.  I have one idea for you to consider inline.




Comment at: clang/lib/Lex/HeaderSearch.cpp:892-902
+IsInHeaderMap, MappedName);
+if (!MappedName.empty()) {
+  assert(IsInHeaderMap && "MappedName should come from a header map");
   CacheLookup.MappedName =
-  copyString(Filename, LookupFileCache.getAllocator());
-  if (IsMapped)
-*IsMapped = true;
+  copyString(MappedName, LookupFileCache.getAllocator());
 }
+if (IsMapped)

I wonder if this would be easier to follow if you refactored like this:

```
if (!MappedName.empty()) {
  // other logic.
  if (IsMapped)
*IsMapped = true;
} else if (IsInHeaderMap && File) {
  if (IsMapped)
*IsMapped = true;
}
```

but maybe my aesthetics are being thrown off by all the intervening comments in 
Phab.  I'll leave it up to you.


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

https://reviews.llvm.org/D58094



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


Re: [PATCH] D66361: Improve behavior in the case of stack exhaustion.

2019-09-06 Thread Richard Smith via cfe-commits
On Tue, 3 Sep 2019 at 17:59, Kamil Rytarowski via Phabricator via
cfe-commits  wrote:

> krytarowski added a comment.
>
> In D66361#1656037 , @rsmith
> wrote:
>
> > In D66361#1655903 ,
> @krytarowski wrote:
> >
> > > This change broke on NetBSD.
> > >
> > >
> http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22003/steps/run%20unit%20tests/logs/FAIL%3A%20Clang%3A%3Astack-exhaustion.cpp
> >
> >
> > Test disabled for NetBSD in r370801. If you're interested in
> investigating why this isn't working there, feel free, but this is only a
> best-effort mitigation for the case where things have already gone wrong,
> so there are limits to how much effort it makes sense to resolve this.
> >
> > Does NetBSD set a hard stack rlimit of less than 8MB by any chance?
>
>
> The stack rlimit is restricted by default to 4MB. The maximum at least on
> amd64 is 128MB... but if someone really needs it, it could by bypassed with
> more stacks.
>
>   $ ulimit -a
>   time(cpu-seconds)unlimited
>   file(blocks) unlimited
>   coredump(blocks) unlimited
>   data(kbytes) 262144
>   stack(kbytes)4096
>   lockedmem(kbytes)10847213
>   memory(kbytes)   32541640
>   nofiles(descriptors) 1024
>   processes1024
>   threads  1024
>   vmemory(kbytes)  unlimited
>   sbsize(bytes)unlimited
>
> Should the limit by raised by default in the system?
>

Clang expects 8MB stacks. When possible (if CLANG_HAVE_RLIMITS is enabled
at configure-time) it will increase the soft stack rlimit to 8MB if it can.

It could be that that's failing here, either because CLANG_HAVE_RLIMITS is
configured as 0, or because the hard stack limit is set below 8MB, or maybe
because this kernel doesn't apply new stack limits that are set after a
thread begins running. In the former case, there's not much we can do;
without a working getrlimit, we can't guess how much stack is even
available. In the latter cases, we could ask the system what the actual
limit is, and apply our stack overflow mitigation when we reach that
instead of when we reach (nearly) 8MB.

Can you find out which of these (if any) is the case?


> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D66361/new/
>
> https://reviews.llvm.org/D66361
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r371279 - [clang][DependencyFileGenerator] Fix missing -MT option handling

2019-09-06 Thread Jan Korous via cfe-commits
Author: jkorous
Date: Fri Sep  6 17:59:13 2019
New Revision: 371279

URL: http://llvm.org/viewvc/llvm-project?rev=371279&view=rev
Log:
[clang][DependencyFileGenerator] Fix missing -MT option handling

Targets in DependencyFileGenerator don't necessarily come from -MT option.

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

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=371279&r1=371278&r2=371279&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Sep  6 17:59:13 2019
@@ -3406,6 +3406,11 @@ bool CompilerInvocation::CreateFromArgs(
   Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
   Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
   ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
+  if (!Res.getDependencyOutputOpts().OutputFile.empty() &&
+  Res.getDependencyOutputOpts().Targets.empty()) {
+Diags.Report(diag::err_fe_dependency_file_requires_MT);
+Success = false;
+  }
   Success &=
   ParseDiagnosticArgs(Res.getDiagnosticOpts(), Args, &Diags,
   false /*DefaultDiagColor*/, false 
/*DefaultShowOpt*/);

Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=371279&r1=371278&r2=371279&view=diff
==
--- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
+++ cfe/trunk/lib/Frontend/DependencyFile.cpp Fri Sep  6 17:59:13 2019
@@ -192,11 +192,6 @@ DependencyFileGenerator::DependencyFileG
 }
 
 void DependencyFileGenerator::attachToPreprocessor(Preprocessor &PP) {
-  if (Targets.empty()) {
-PP.getDiagnostics().Report(diag::err_fe_dependency_file_requires_MT);
-return;
-  }
-
   // Disable the "file not found" diagnostic if the -MG option was given.
   if (AddMissingHeaderDeps)
 PP.SetSuppressIncludeNotFoundError(true);


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


[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

2019-09-06 Thread Alex Cameron via Phabricator via cfe-commits
tetsuo-cpp added a comment.

Friendly ping. Could I please get this looked at?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037



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


[PATCH] D65433: [clangd] DefineInline action availability checks

2019-09-06 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

When fully implemented, will define inline tweak work with C++ methods in 
classes as well?
E.g.

  HEADER:
  class Foo { void foo(); }
  CPP:
  #include "Header.h"
  void Foo::foo() {}

becomes:

  HEADER:
  class Foo { void foo() { } }
  CPP:
  #include "Header.h"






Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:192
+  Intent intent() const override { return Intent::Refactor; }
+  std::string title() const override { return "Inline function definition"; }
+

Sorry, I'm not really familiar with design of tweaks, so this might be a bad 
question: Is it possible to change the title of the tweak on a per-TU basis. 
More specifically, some tweaks might want to have different titles for 
Objective-C actions compared to their C++ siblings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65433



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


[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

2019-09-06 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 219211.
plotfi marked 2 inline comments as done.
plotfi added a reviewer: cishida.
plotfi added a comment.

Adding better wiring up to llvm-ifs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63978

Files:
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Phases.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Phases.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.h
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1738,7 +1738,7 @@
   llvm::Optional ProgramAction =
   llvm::StringSwitch>(ArgStr)
   .Case("experimental-ifs-v1", frontend::GenerateInterfaceIfsExpV1)
-  .Default(llvm::None);
+  .Default(frontend::GenerateInterfaceIfsExpV1);
   if (!ProgramAction) {
 std::string ErrorMessage =
 "Invalid interface stub format: " + ArgStr.str() +
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -319,6 +319,17 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
+  // Treat Interface Stubs like its own compilation mode.
+  else if (DAL.getLastArg(options::OPT_emit_iterface_stubs)) {
+llvm::SmallVector IfsModePhaseList;
+types::getCompilationPhases(types::TY_IFS, IfsModePhaseList);
+llvm::copy_if(
+IfsModePhaseList, std::back_inserter(P), [&DAL](phases::ID Phase) {
+  return Phase <= ((DAL.getLastArg(options::OPT_c)) ? phases::Compile
+: phases::IfsMerge);
+});
+  }
+
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
@@ -327,7 +338,6 @@
DAL.getLastArg(options::OPT_rewrite_objc) ||
DAL.getLastArg(options::OPT_rewrite_legacy_objc) ||
DAL.getLastArg(options::OPT__migrate) ||
-   DAL.getLastArg(options::OPT_emit_iterface_stubs) ||
DAL.getLastArg(options::OPT__analyze, options::OPT__analyze_auto) ||
DAL.getLastArg(options::OPT_emit_ast))
 llvm::copy_if(PhaseList, std::back_inserter(P),
Index: clang/lib/Driver/ToolChains/InterfaceStubs.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/InterfaceStubs.h
@@ -0,0 +1,40 @@
+//===---  InterfaceStubs.cpp - Base InterfaceStubs Implementations C++  ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_IFS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_IFS_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+using namespace clang;
+using namespace llvm::opt;
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace ifstool {
+class LLVM_LIBRARY_VISIBILITY Merger : public Tool {
+public:
+  Merger(const ToolChain &TC) : Tool("IFS::Merger", "llvm-ifs", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return false; }
+
+  void ConstructJob(Compilation &C, const JobAction &JA,
+const InputInfo &Output, const InputInfoList &Inputs,
+const llvm::opt::ArgList &TCArgs,
+const char *LinkingOutput) const override;
+};
+} // end namespace ifstool
+} // end namespace tools
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_IFS_H
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -0,0 +1,43 @@
+//===---  InterfaceStubs.cpp - Base InterfaceStubs Implementations C++  ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===

[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

2019-09-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I'm sadly not knowledgeable enough with `CallDescriptionMap`, so let's have 
another round of review on this, otherwise, its perfect.

We talked about dividing this checker into multiple files, which would also 
make reviewing a bit easier. With that done, combined with this patch, I am 
very confident that we could enable parts of this checker by default by, well, 
you know, soon enough :^)




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:1328-1337
+def ContainerInspectionChecker : Checker<"ContainerInspection">,
+  HelpText<"Check the analyzer's understanding of C++ containers">,
+  Dependencies<[IteratorModeling]>,
+  Documentation;
+
+def IteratorInspectionChecker : Checker<"IteratorInspection">,
+  HelpText<"Check the analyzer's understanding of C++ iterators">,

NoQ wrote:
> Dunno, i would keep it all in one checker, just to save a few `// RUN:` lines 
> :)
I agree. Let's combine these into `DebugIteratorModeling`, because, as I 
understand it, that is what we're doing!



Comment at: test/Analysis/iterator-inspection.cpp:1-2
+// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,cplusplus,debug.ContainerInspection,debug.IteratorInspection,debug.ExprInspection
 -analyzer-config aggressive-binary-operation-simplification=true 
-analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,cplusplus,debug.ContainerInspection,debug.IteratorInspection,debug.ExprInspection
 -analyzer-config aggressive-binary-operation-simplification=true 
-analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+

Could you please format these? c:



Comment at: test/Analysis/iterator-inspection.cpp:38-43
+void iterator_container(const std::vector v0) {
+  auto b0 = v0.begin();
+
+  clang_analyzer_dump(&v0); //expected-warning{{&v0}}
+  clang_analyzer_dump(clang_analyzer_iterator_container(b0)); 
//expected-warning{{&v0}}
+}

Yea, I agree, let's add a testcase with a little more substance.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67156



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


[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

2019-09-06 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D67156#1661881 , @Szelethus wrote:

> I'm sadly not knowledgeable enough with `CallDescriptionMap`


D62557  contains a basic example.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67156



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


[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-09-06 Thread Richard Trieu via Phabricator via cfe-commits
rtrieu updated this revision to Diff 219213.
rtrieu added a comment.

Add more test cases and split new warnings into a separate warning group, but 
still under -Wparentheses


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

https://reviews.llvm.org/D66043

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/parentheses.c


Index: test/Sema/parentheses.c
===
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -144,6 +144,28 @@
 
   (void)(x + y > 0 ? 1 : 2); // no warning
   (void)(x + (y > 0) ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '+'}} expected-note 2{{place parentheses}}
+
+  (void)(b ? 0xf0 : 0x10 | b ? 0x5 : 0x2); // expected-warning {{operator '?:' 
has lower precedence than '|'}} expected-note 2{{place parentheses}}
+
+  (void)((b ? 0xf0 : 0x10) | (b ? 0x5 : 0x2));  // no warning, has parentheses
+  (void)(b ? 0xf0 : (0x10 | b) ? 0x5 : 0x2);  // no warning, has parentheses
+
+  (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '|'}} expected-note 2{{place parentheses}}
+  (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '&'}} expected-note 2{{place parentheses}}
+
+  (void)((x | b) ? 1 : 2);  // no warning, has parentheses
+  (void)(x | (b ? 1 : 2));  // no warning, has parentheses
+  (void)((x & b) ? 1 : 2);  // no warning, has parentheses
+  (void)(x & (b ? 1 : 2));  // no warning, has parentheses
+
+  // Only warn on uses of the bitwise operators, and not the logical operators.
+  // The bitwise operators are more likely to be bugs while the logical
+  // operators are more likely to be used correctly.  Since there is no
+  // explicit logical-xor operator, the bitwise-xor is commonly used instead.
+  // For this warning, treat the bitwise-xor as if it were a logical operator.
+  (void)(x ^ b ? 1 : 2);  // no warning, ^ is often used as logical xor
+  (void)(x || b ? 1 : 2);  // no warning, logical operator
+  (void)(x && b ? 1 : 2);  // no warning, logical operator
 }
 
 // RUN: not %clang_cc1 -fsyntax-only -Wparentheses -Werror 
-fdiagnostics-show-option %s 2>&1 | FileCheck %s -check-prefix=CHECK-FLAG
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7485,7 +7485,12 @@
 static bool IsArithmeticOp(BinaryOperatorKind Opc) {
   return BinaryOperator::isAdditiveOp(Opc) ||
  BinaryOperator::isMultiplicativeOp(Opc) ||
- BinaryOperator::isShiftOp(Opc);
+ BinaryOperator::isShiftOp(Opc) || Opc == BO_And || Opc == BO_Or;
+  // This only checks for bitwise-or and bitwise-and, but not bitwise-xor and
+  // not any of the logical operators.  Bitwise-xor is commonly used as a
+  // logical-xor because there is no logical-xor operator.  The logical
+  // operators, including uses of xor, have a high false positive rate for
+  // precedence warnings.
 }
 
 /// IsArithmeticBinaryExpr - Returns true if E is an arithmetic binary
@@ -7575,7 +7580,11 @@
   // The condition is an arithmetic binary expression, with a right-
   // hand side that looks boolean, so warn.
 
-  Self.Diag(OpLoc, diag::warn_precedence_conditional)
+  unsigned DiagID = BinaryOperator::isBitwiseOp(CondOpCode)
+? diag::warn_precedence_bitwise_conditional
+: diag::warn_precedence_conditional;
+
+  Self.Diag(OpLoc, DiagID)
   << Condition->getSourceRange()
   << BinaryOperator::getOpcodeStr(CondOpcode);
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5618,6 +5618,9 @@
 def warn_precedence_conditional : Warning<
   "operator '?:' has lower precedence than '%0'; '%0' will be evaluated 
first">,
   InGroup;
+def warn_precedence_bitwise_conditional : Warning<
+  "operator '?:' has lower precedence than '%0'; '%0' will be evaluated 
first">,
+  InGroup;
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -280,6 +280,7 @@
 def FlexibleArrayExtensions : DiagGroup<"flexible-array-extensions">;
 def FourByteMultiChar : DiagGroup<"four-char-constants">;
 def GlobalConstructors : DiagGroup<"global-constructors">;
+def BitwiseConditionalParentheses: 
DiagGroup<"bitwise-conditional-parentheses">;
 def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
 def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
 def LogicalNotParentheses: DiagGroup<"logical-not-

r371285 - Remove stale TLI Module level pass registration

2019-09-06 Thread Teresa Johnson via cfe-commits
Author: tejohnson
Date: Fri Sep  6 20:09:46 2019
New Revision: 371285

URL: http://llvm.org/viewvc/llvm-project?rev=371285&view=rev
Log:
Remove stale TLI Module level pass registration

Clang patch to adapt to LLVM changes in D66428 that make the TLI
require a Function. There is no longer a module-level
TargetLibraryAnalysis, so remove its registration

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=371285&r1=371284&r2=371285&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Fri Sep  6 20:09:46 2019
@@ -1093,7 +1093,6 @@ void EmitAssemblyHelper::EmitAssemblyWit
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
-  MAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
 
   // Register all the basic analyses with the managers.
   PB.registerModuleAnalyses(MAM);


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


[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

2019-09-06 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth updated this revision to Diff 219219.
paulkirth added a comment.

Improve diagnostic output with profile counts


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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use 

<    1   2