[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


https://reviews.llvm.org/D52271



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

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang marked 5 inline comments as done.
shuaiwang added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;

JonasToth wrote:
> That doesn't look like a super idea. It is super hidden that variable 'p*' 
> will be analyzed for pointee mutation and others aren't. Especially in 12 
> months and when someone else wants to change something, this will be the 
> source of headaches.
> 
> Don't you think there could be a better way of doing this switch?
Yeah... agree :)
But how about let's keep it this way just for now, and I'll pause the work on 
supporting pointee analysis and fix it properly but in a separate diff 
following this one?

I've been thinking about this and also did some prototyping, and here are my 
thoughts:
In order to properly fix this we need to change the interface of 
`ExprMutationAnalyzer` to not simply return a `Stmt` but instead return a 
`MutationTrace` with additional metadata. This wasn't needed before because we 
can reconstruct a mutation chain by continuously calling `findMutation`, and 
that works well for cases like "int x; int& r0 = x; int& r1 = r0; r1 = 123;". 
But now that pointers come into play, and we need to handle cases like "int *p; 
int *p2 = p; int& r = *p2; r = 123;", and in order to reconstruct the mutation 
chain, we need to do `findPointeeMutation(p)` -> `findPointeeMutation(p2)` -> 
`findMutation(r)`, notice that we need to switch from calling 
`findPointeeMutation` to calling `findMutation`. However we don't know where 
the switch should happen simply based on what's returned from 
`findPointeeMutation`, we need additional metadata for that.

That interface change will unavoidably affect how memoization is done so we 
need to change memoization as well.

Now since we need to change both the interface and memoization anyway, we might 
as well design them properly so that multiple-layers of pointers can be 
supported nicely. I think we can end up with something like this:
```
class ExprMutationAnalyzer {
public:
  struct MutationTrace {
const Stmt *Value;
const MutationTrace *Next;
// Other metadata
  };

  // Finds whether any mutative operations are performed on the given 
expression after dereferencing `DerefLayers` times.
  const MutationTrace *findMutation(const Expr *, int DerefLayers = 0);
  const MutationTrace *findMutation(const Decl *, int DerefLayers = 0);
};
```
This involves quite some refactoring, and doing this refactoring before this 
diff and later rebasing seems quite some trouble that I'd like to avoid.

Let me know what do you think :)


Repository:
  rC Clang

https://reviews.llvm.org/D52219



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

[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 166224.
erik.pilkington added a comment.

Sure, the new patch just preserves __restrict qualifiers. Thanks!


https://reviews.llvm.org/D52271

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/subst-restrict.cpp


Index: clang/test/SemaCXX/subst-restrict.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+  return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.


Index: clang/test/SemaCXX/subst-restrict.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+// The only qualifier that applies to a reference type is restrict.
+if (!Quals.hasRestrict())
+  return T;
+Quals = Qualifiers::fromCVRMask(Qualifiers::Restrict);
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52074: [PowerPC] [Clang] Add vector int128 pack/unpack builtins

[qshanz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342614: [PowerPC] [Clang] Add vector int128 pack/unpack 
builtins (authored by qshanz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52074?vs=165424=166222#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52074

Files:
  cfe/trunk/include/clang/Basic/BuiltinsPPC.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/CodeGen/builtins-ppc-error.c
  cfe/trunk/test/CodeGen/builtins-ppc-p7-disabled.c
  cfe/trunk/test/CodeGen/builtins-ppc-vsx.c

Index: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def
@@ -470,6 +470,10 @@
 BUILTIN(__builtin_divdeu, "ULLiULLiULLi", "")
 BUILTIN(__builtin_bpermd, "SLLiSLLiSLLi", "")
 
+// Vector int128 (un)pack
+BUILTIN(__builtin_unpack_vector_int128, "ULLiV1LLLii", "")
+BUILTIN(__builtin_pack_vector_int128, "V1LLLiULLiULLi", "")
+
 // FIXME: Obviously incomplete.
 
 #undef BUILTIN
Index: cfe/trunk/test/CodeGen/builtins-ppc-p7-disabled.c
===
--- cfe/trunk/test/CodeGen/builtins-ppc-p7-disabled.c
+++ cfe/trunk/test/CodeGen/builtins-ppc-p7-disabled.c
@@ -6,13 +6,17 @@
 // RUN: not %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - 2>&1 \
 // RUN: -target-cpu pwr7 | FileCheck %s -check-prefix=CHECK-32
 
+vector signed __int128 vslll = {33};
+
 void call_p7_builtins(void)
 {
   int a = __builtin_divwe(33, 11);
   unsigned int b = __builtin_divweu(33U, 11U);
   unsigned long long d = __builtin_divde(33ULL, 11ULL);
   unsigned long long e = __builtin_divdeu(33ULL, 11ULL);
   unsigned long long f = __builtin_bpermd(33ULL, 11ULL);
+  __builtin_pack_vector_int128(33ULL, 11ULL);
+  __builtin_unpack_vector_int128(vslll, 1);
 }
 
 // CHECK: error: this builtin is only valid on POWER7 or later CPUs
@@ -25,6 +29,10 @@
 // CHECK: __builtin_divdeu
 // CHECK: error: this builtin is only valid on POWER7 or later CPUs
 // CHECK: __builtin_bpermd
+// CHECK: error: this builtin is only valid on POWER7 or later CPUs
+// CHECK: __builtin_pack_vector_int128
+// CHECK: error: this builtin is only valid on POWER7 or later CPUs
+// CHECK: __builtin_unpack_vector_int128
 // CHECK-32: error: this builtin is only available on 64-bit targets
 // CHECK-32: __builtin_divde
 // CHECK-32: error: this builtin is only available on 64-bit targets
Index: cfe/trunk/test/CodeGen/builtins-ppc-vsx.c
===
--- cfe/trunk/test/CodeGen/builtins-ppc-vsx.c
+++ cfe/trunk/test/CodeGen/builtins-ppc-vsx.c
@@ -49,6 +49,7 @@
 vector bool long long res_vbll;
 vector signed long long res_vsll;
 vector unsigned long long res_vull;
+vector signed __int128 res_vslll;
 
 double res_d;
 float res_af[4];
@@ -1803,3 +1804,31 @@
 // CHECK-NEXT-LE:  shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> 
 // CHECK-NEXT-LE:  bitcast <4 x i32> %{{[0-9]+}} to <2 x double>
 }
+
+void testVectorInt128Pack(){
+// CHECK-LABEL: testVectorInt128Pack
+// CHECK-LABEL-LE: testVectorInt128Pack
+  res_vslll = __builtin_pack_vector_int128(aull[0], aull[1]);
+// CHECK: %[[V1:[0-9]+]] = insertelement <2 x i64> undef, i64 %{{[0-9]+}}, i64 0
+// CHECK-NEXT: %[[V2:[0-9]+]] = insertelement <2 x i64> %[[V1]], i64 %{{[0-9]+}}, i64 1
+// CHECK-NEXT:  bitcast <2 x i64> %[[V2]] to <1 x i128>
+
+// CHECK-LE: %[[V1:[0-9]+]] = insertelement <2 x i64> undef, i64 %{{[0-9]+}}, i64 1
+// CHECK-NEXT-LE: %[[V2:[0-9]+]] = insertelement <2 x i64> %[[V1]], i64 %{{[0-9]+}}, i64 0
+// CHECK-NEXT-LE:  bitcast <2 x i64> %[[V2]] to <1 x i128>
+
+  __builtin_unpack_vector_int128(res_vslll, 0);
+// CHECK:  %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64>
+// CHECK-NEXT: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 0
+
+// CHECK-LE:  %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64>
+// CHECK-NEXT-LE: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 1
+
+  __builtin_unpack_vector_int128(res_vslll, 1);
+// CHECK:  %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64>
+// CHECK-NEXT: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 1
+
+// CHECK-LE:  %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64>
+// CHECK-NEXT-LE: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 0
+
+}
Index: cfe/trunk/test/CodeGen/builtins-ppc-error.c
===
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c
@@ -14,6 +14,7 @@
 extern vector signed int vui;
 extern vector float vf;
 extern vector unsigned char vuc;
+extern vector signed __int128 vsllli;
 
 void testInsertWord(void) {
   int index = 5;
@@ -67,3 +68,8 @@
 void testVCTUXS(int index) {
   

[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:4268
+Quals.removeVolatile();
+  }
 

`restrict` is unique in being able to be applied to reference types.  You 
should just rebuild `Quals` from scratch, preserving only `restrict`.


Repository:
  rC Clang

https://reviews.llvm.org/D52271



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

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Conceptually, this change looks great.  And it should be fine to require extra 
alignment on `IdentifierInfo` on 32-bit hosts; I doubt that will have 
measurable impact.

I believe it's possible to eliminate the need for most, perhaps all, of these 
`static_asserts` by just defining the constants more sensibly in the first 
place.




Comment at: include/clang/AST/DeclarationName.h:46
 
-/// DeclarationName - The name of a declaration. In the common case,
-/// this just stores an IdentifierInfo pointer to a normal
-/// name. However, it also provides encodings for Objective-C
-/// selectors (optimizing zero- and one-argument selectors, which make
-/// up 78% percent of all selectors in Cocoa.h) and special C++ names
-/// for constructors, destructors, and conversion functions.
+namespace detail {
+

Should `DeclarationNameExtra` be moved here?  I'm not sure why it's in 
`IdentifierTable.h` in the first place, and your refactor seems to make that 
even more pointless.



Comment at: include/clang/AST/DeclarationName.h:54
+/// we use three different FoldingSet in DeclarationNameTable.
+class alignas(8) CXXSpecialName : public llvm::FoldingSetNode {
+  friend class clang::DeclarationName;

If this isn't a self-contained description of the name anymore, I think adding 
`Extra` to the class name would be appropriate.  And maybe the class name 
should directly express that is for the kinds of names that are basically 
uniqued by type.

Please introduce a symbolic constant for 8 here, since you use and assume it 
across multiple places.  Either `#define` or a `const` variable works.



Comment at: include/clang/AST/DeclarationName.h:98
+/// Contains extra information for the name of an overloaded
+/// operator in C++, such as "operator+.
+class alignas(8) CXXOperatorIdName {

This is probably pre-existing, but the comment should clarify that this doesn't 
include literal or conversion operators.



Comment at: include/clang/AST/DeclarationName.h:164
+CXXUsingDirective = 10,
+ObjCMultiArgSelector = 11
+  };

Is the criterion for inclusion in the first seven really just frequency of use, 
or is it a combination of that and relative benefit?

The only one I would really quibble about is that multi-argument selectors are 
probably more common and important to Objective-C code than conversion 
operators are to C++.  But it's quite possible that the relative benefit is 
much higher for C++, since selectors only appear on specific kinds of 
declarations that know they're declared with selectors — relatively little code 
actually needs to be polymorphic about them — and since they have to be defined 
out-of-line.



Comment at: include/clang/AST/DeclarationName.h:178
+  ///   This enable efficient conversion between the two enumerations
+  ///   in the usual case.
+  ///

You can express this directly by defining `StoredNameKind` first and then 
defining the corresponding enumerators in `NameKind` in terms of them.



Comment at: include/clang/AST/DeclarationName.h:184
+  ///   between DeclarationNameExtra::ExtraKind and NameKind possible with
+  ///   a single addition/substraction.
+  ///

Same deal.  Just define the enumerators of `NameKind` appropriately.



Comment at: include/clang/AST/DeclarationName.h:243
+  ///   C++ literal operator, or C++ using directive.
   uintptr_t Ptr = 0;
 

riccibruno wrote:
> erichkeane wrote:
> > There is an llvm type for storing something in the lower bits of a pointer. 
> >  I THINK it is llvm::PointerIntPair.
> > 
> > I'd MUCH prefer you do that instead of the reimplementation here.
> Well it was already like this but point taken.
Using `PointerIntPair` probably won't work here because the pointer type is 
basically a union, and you can't use `void*` because `PointerIntPair` wants to 
assert that the pointer type is sufficiently aligned.


Repository:
  rC Clang

https://reviews.llvm.org/D52267



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

[PATCH] D51464: clang: fix MIPS/N32 triple and paths

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa updated this revision to Diff 166220.
wzssyqa added a comment.

remove mips64(el)-linux-gnu from path search.


Repository:
  rC Clang

https://reviews.llvm.org/D51464

Files:
  lib/Basic/Targets/Mips.h
  lib/Driver/ToolChains/Arch/Mips.cpp
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/CodeGen/atomics-inlining.c
  test/CodeGen/mips-zero-sized-struct.c
  test/CodeGen/target-data.c
  test/CodeGen/xray-attributes-supported.cpp
  test/Driver/clang-translation.c

Index: test/Driver/clang-translation.c
===
--- test/Driver/clang-translation.c
+++ test/Driver/clang-translation.c
@@ -330,6 +330,38 @@
 // MIPS64EL: "-target-cpu" "mips64r2"
 // MIPS64EL: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mips64-linux-gnuabi64 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPS64-GNUABI64 %s
+// MIPS64-GNUABI64: clang
+// MIPS64-GNUABI64: "-cc1"
+// MIPS64-GNUABI64: "-target-cpu" "mips64r2"
+// MIPS64-GNUABI64: "-target-abi" "n64"
+// MIPS64-GNUABI64: "-mfloat-abi" "hard"
+
+// RUN: %clang -target mips64el-linux-gnuabi64 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPS64EL-GNUABI64 %s
+// MIPS64EL-GNUABI64: clang
+// MIPS64EL-GNUABI64: "-cc1"
+// MIPS64EL-GNUABI64: "-target-cpu" "mips64r2"
+// MIPS64EL-GNUABI64: "-target-abi" "n64"
+// MIPS64EL-GNUABI64: "-mfloat-abi" "hard"
+
+// RUN: %clang -target mips64-linux-gnuabin32 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPSN32 %s
+// MIPSN32: clang
+// MIPSN32: "-cc1"
+// MIPSN32: "-target-cpu" "mips64r2"
+// MIPSN32: "-target-abi" "n32"
+// MIPSN32: "-mfloat-abi" "hard"
+
+// RUN: %clang -target mips64el-linux-gnuabin32 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPSN32EL %s
+// MIPSN32EL: clang
+// MIPSN32EL: "-cc1"
+// MIPSN32EL: "-target-cpu" "mips64r2"
+// MIPSN32EL: "-target-abi" "n32"
+// MIPSN32EL: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64el-linux-android -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPS64EL-ANDROID %s
 // MIPS64EL-ANDROID: clang
Index: test/CodeGen/xray-attributes-supported.cpp
===
--- test/CodeGen/xray-attributes-supported.cpp
+++ test/CodeGen/xray-attributes-supported.cpp
@@ -11,6 +11,14 @@
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
 // RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnuabi64 | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnuabi64 | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnuabin32 | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnuabin32 | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
 // RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
 
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.
Index: test/CodeGen/target-data.c
===
--- test/CodeGen/target-data.c
+++ test/CodeGen/target-data.c
@@ -42,18 +42,34 @@
 // RUN: FileCheck %s -check-prefix=MIPS-64EL
 // MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
 
+// RUN: %clang_cc1 -triple mips64el-linux-gnuabi64 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=MIPS-64EL
+// MIPS-64EL: target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+
 // RUN: %clang_cc1 -triple mips64el-linux-gnu -o - -emit-llvm -target-abi n32 \
 // RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32
 // MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128"
 
+// RUN: %clang_cc1 -triple mips64el-linux-gnuabin32 -o - -emit-llvm \
+// RUN: %s | FileCheck %s -check-prefix=MIPS-64EL-N32
+// MIPS-64EL-N32: target datalayout = "e-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+
 // RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=MIPS-64EB
 // MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
 
+// RUN: %clang_cc1 -triple mips64-linux-gnuabi64 -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=MIPS-64EB
+// MIPS-64EB: target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+
 // RUN: %clang_cc1 -triple mips64-linux-gnu -o - -emit-llvm %s -target-abi n32 \
 // RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32
 // MIPS-64EB-N32: target datalayout = "E-m:e-p:32:32-i8:8:32-i16:16:32-i64:64-n32:64-S128"
 
+// RUN: %clang_cc1 -triple mips64-linux-gnuabin32 -o - -emit-llvm %s \
+// RUN: | FileCheck %s -check-prefix=MIPS-64EB-N32
+// MIPS-64EB-N32: target datalayout = 

[PATCH] D50850: clang: Add triples support for MIPS r6

[YunQiang Su via Phabricator via cfe-commits]

wzssyqa updated this revision to Diff 166219.

Repository:
  rC Clang

https://reviews.llvm.org/D50850

Files:
  lib/Driver/ToolChains/Arch/Mips.cpp
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/Linux.cpp
  test/CodeGen/atomics-inlining.c
  test/CodeGen/mips-zero-sized-struct.c
  test/CodeGen/target-data.c
  test/CodeGen/xray-attributes-supported.cpp
  test/Driver/clang-translation.c

Index: test/Driver/clang-translation.c
===
--- test/Driver/clang-translation.c
+++ test/Driver/clang-translation.c
@@ -291,13 +291,27 @@
 // MIPS: "-target-cpu" "mips32r2"
 // MIPS: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa32r6-linux-gnu -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPSR6 %s
+// MIPSR6: clang
+// MIPSR6: "-cc1"
+// MIPSR6: "-target-cpu" "mips32r6"
+// MIPSR6: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mipsel-linux-gnu -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPSEL %s
 // MIPSEL: clang
 // MIPSEL: "-cc1"
 // MIPSEL: "-target-cpu" "mips32r2"
 // MIPSEL: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa32r6el-linux-gnu -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPSR6EL %s
+// MIPSR6EL: clang
+// MIPSR6EL: "-cc1"
+// MIPSR6EL: "-target-cpu" "mips32r6"
+// MIPSR6EL: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mipsel-linux-android -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPSEL-ANDROID %s
 // MIPSEL-ANDROID: clang
@@ -323,45 +337,91 @@
 // MIPS64: "-target-cpu" "mips64r2"
 // MIPS64: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa64r6-linux-gnu -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPS64R6 %s
+// MIPS64R6: clang
+// MIPS64R6: "-cc1"
+// MIPS64R6: "-target-cpu" "mips64r6"
+// MIPS64R6: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64el-linux-gnu -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPS64EL %s
 // MIPS64EL: clang
 // MIPS64EL: "-cc1"
 // MIPS64EL: "-target-cpu" "mips64r2"
 // MIPS64EL: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa64r6el-linux-gnu -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPS64R6EL %s
+// MIPS64R6EL: clang
+// MIPS64R6EL: "-cc1"
+// MIPS64R6EL: "-target-cpu" "mips64r6"
+// MIPS64R6EL: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64-linux-gnuabi64 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPS64-GNUABI64 %s
 // MIPS64-GNUABI64: clang
 // MIPS64-GNUABI64: "-cc1"
 // MIPS64-GNUABI64: "-target-cpu" "mips64r2"
 // MIPS64-GNUABI64: "-target-abi" "n64"
 // MIPS64-GNUABI64: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa64r6-linux-gnuabi64 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPS64R6-GNUABI64 %s
+// MIPS64R6-GNUABI64: clang
+// MIPS64R6-GNUABI64: "-cc1"
+// MIPS64R6-GNUABI64: "-target-cpu" "mips64r6"
+// MIPS64R6-GNUABI64: "-target-abi" "n64"
+// MIPS64R6-GNUABI64: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64el-linux-gnuabi64 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPS64EL-GNUABI64 %s
 // MIPS64EL-GNUABI64: clang
 // MIPS64EL-GNUABI64: "-cc1"
 // MIPS64EL-GNUABI64: "-target-cpu" "mips64r2"
 // MIPS64EL-GNUABI64: "-target-abi" "n64"
 // MIPS64EL-GNUABI64: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa64r6el-linux-gnuabi64 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPS64R6EL-GNUABI64 %s
+// MIPS64R6EL-GNUABI64: clang
+// MIPS64R6EL-GNUABI64: "-cc1"
+// MIPS64R6EL-GNUABI64: "-target-cpu" "mips64r6"
+// MIPS64R6EL-GNUABI64: "-target-abi" "n64"
+// MIPS64R6EL-GNUABI64: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64-linux-gnuabin32 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPSN32 %s
 // MIPSN32: clang
 // MIPSN32: "-cc1"
 // MIPSN32: "-target-cpu" "mips64r2"
 // MIPSN32: "-target-abi" "n32"
 // MIPSN32: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa64r6-linux-gnuabin32 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPSN32R6 %s
+// MIPSN32R6: clang
+// MIPSN32R6: "-cc1"
+// MIPSN32R6: "-target-cpu" "mips64r6"
+// MIPSN32R6: "-target-abi" "n32"
+// MIPSN32R6: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64el-linux-gnuabin32 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPSN32EL %s
 // MIPSN32EL: clang
 // MIPSN32EL: "-cc1"
 // MIPSN32EL: "-target-cpu" "mips64r2"
 // MIPSN32EL: "-target-abi" "n32"
 // MIPSN32EL: "-mfloat-abi" "hard"
 
+// RUN: %clang -target mipsisa64r6el-linux-gnuabin32 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=MIPSN32R6EL %s
+// MIPSN32R6EL: clang
+// MIPSN32R6EL: "-cc1"
+// MIPSN32R6EL: "-target-cpu" "mips64r6"
+// MIPSN32R6EL: "-target-abi" "n32"
+// MIPSN32R6EL: "-mfloat-abi" "hard"
+
 // RUN: %clang -target mips64el-linux-android -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=MIPS64EL-ANDROID %s
 // MIPS64EL-ANDROID: clang
Index: test/CodeGen/xray-attributes-supported.cpp
===
--- test/CodeGen/xray-attributes-supported.cpp
+++ test/CodeGen/xray-attributes-supported.cpp
@@ -5,20 

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

This fix has become obvious, but I still prefer that it gets reviewed before I 
commit it.


Repository:
  rC Clang

https://reviews.llvm.org/D52021



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

[PATCH] D52074: [PowerPC] [Clang] Add vector int128 pack/unpack builtins

[Qing Shan Zhang via Phabricator via cfe-commits]

steven.zhang added a comment.

I will commit the patch for you.


Repository:
  rC Clang

https://reviews.llvm.org/D52074



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

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 166216.
owenpan added a comment.

Simplified the fix and improved the test case.


Repository:
  rC Clang

https://reviews.llvm.org/D52021

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,30 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,8 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+if (I[0]->Last->is(tok::l_brace) || I[1]->First->is(tok::l_brace))
+  return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,30 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,8 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+if (I[0]->Last->is(tok::l_brace) || I[1]->First->is(tok::l_brace))
+  return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

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

Szelethus added inline comments.



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:86
+// - Pointer to non-const
+// - Pointer-like type with `operator*` returning non-const reference
+bool isPointeeMutable(const Expr *Exp, const ASTContext ) {

Didn't you mean these to be doxygen comments?


Repository:
  rC Clang

https://reviews.llvm.org/D52219



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

Re: r229575 - clang-cl: Disable frame pointer elimination at -O0

[Nico Weber via cfe-commits]

The generic O flag handling doesn't support 0 either. Would you be ok with
removing this?

Does /Od do what you want?

On Wed, Sep 19, 2018 at 4:52 PM Reid Kleckner 
wrote:

> I was probably using it myself, and was surprised that /O0 and -O0 had
> different behavior, because -O0 will hit the special O0 flag that disables
> FPO, but /O0 will hit the generic 'O' flag handling.
>
> On Wed, Sep 19, 2018 at 7:10 AM Nico Weber  wrote:
>
>> Do you remember why you landed this? cl.exe doesn't support /O0; why does
>> clang-cl?
>>
>> On Tue, Feb 17, 2015 at 5:53 PM Reid Kleckner  wrote:
>>
>>> Author: rnk
>>> Date: Tue Feb 17 16:40:42 2015
>>> New Revision: 229575
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=229575=rev
>>> Log:
>>> clang-cl: Disable frame pointer elimination at -O0
>>>
>>> This wasn't kicking in because the _SLASH_O flag didn't match our check
>>> for OPT_O0. Add an alias that does to keep the logic simple.
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Driver/CLCompatOptions.td
>>>
>>> Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=229575=229574=229575=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
>>> +++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Tue Feb 17
>>> 16:40:42 2015
>>> @@ -80,6 +80,7 @@ def _SLASH_I : CLJoinedOrSeparate<"I">,
>>>Alias;
>>>  def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
>>>Alias;
>>> +def _SLASH_O0 : CLFlag<"O0">, Alias;
>>>  def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">,
>>>MetaVarName<"">, Alias;
>>>  def _SLASH_Ob0 : CLFlag<"Ob0">, HelpText<"Disable inlining">,
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-comm...@cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52259: [CUDA] Rearrange search path ordering to fix two test case failures

[Jiading Gai via Phabricator via cfe-commits]

gaijiading updated this revision to Diff 166206.
gaijiading added a comment.

Apologies for having uploaded the wrong patch in my original review request. 
Please help review this version instead. Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D52259

Files:
  lib/Driver/ToolChains/Cuda.cpp


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -87,6 +87,10 @@
   D.SysRoot + "/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v" +
   Ver);
   } else {
+Candidates.emplace_back(D.SysRoot + "/usr/local/cuda");
+for (const char *Ver : Versions)
+  Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
 if (!Args.hasArg(clang::driver::options::OPT_cuda_path_ignore_env)) {
   // Try to find ptxas binary. If the executable is located in a directory
   // called 'bin/', its parent directory might be a good guess for a valid
@@ -108,10 +112,6 @@
   }
 }
 
-Candidates.emplace_back(D.SysRoot + "/usr/local/cuda");
-for (const char *Ver : Versions)
-  Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
-
 if (Distro(D.getVFS()).IsDebian())
   // Special case for Debian to have nvidia-cuda-toolkit work
   // out of the box. More info on http://bugs.debian.org/882505


Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -87,6 +87,10 @@
   D.SysRoot + "/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v" +
   Ver);
   } else {
+Candidates.emplace_back(D.SysRoot + "/usr/local/cuda");
+for (const char *Ver : Versions)
+  Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
+
 if (!Args.hasArg(clang::driver::options::OPT_cuda_path_ignore_env)) {
   // Try to find ptxas binary. If the executable is located in a directory
   // called 'bin/', its parent directory might be a good guess for a valid
@@ -108,10 +112,6 @@
   }
 }
 
-Candidates.emplace_back(D.SysRoot + "/usr/local/cuda");
-for (const char *Ver : Versions)
-  Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
-
 if (Distro(D.getVFS()).IsDebian())
   // Special case for Debian to have nvidia-cuda-toolkit work
   // out of the box. More info on http://bugs.debian.org/882505
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

[Aaron Puchert via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D51901#1239759, @delesley wrote:

> This looks okay to me, but I have not tested it on a large code base to see 
> if it breaks anything.


On our code base (not as large as Google's) there was one true positive and no 
false positives. At least for templates we should be fine now, since we assume 
that all type-dependent base classes could have any attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D51901



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

r342605 - Thread Safety Analysis: warnings for attributes without arguments

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Wed Sep 19 17:39:27 2018
New Revision: 342605

URL: http://llvm.org/viewvc/llvm-project?rev=342605=rev
Log:
Thread Safety Analysis: warnings for attributes without arguments

Summary:
When thread safety annotations are used without capability arguments,
they are assumed to apply to `this` instead. So we warn when either
`this` doesn't exist, or the class is not a capability type.

This is based on earlier work by Josh Gao that was committed in r310403,
but reverted in r310698 because it didn't properly work in template
classes. See also D36237.

The solution is not to go via the QualType of `this`, which is then a
template type, hence the attributes are not known because it could be
specialized. Instead we look directly at the class in which we are
contained.

Additionally I grouped two of the warnings together. There are two
issues here: the existence of `this`, which requires us to be a
non-static member function, and the appropriate annotation on the class
we are contained in. So we don't distinguish between not being in a
class and being static, because in both cases we don't have `this`.

Fixes PR38399.

Reviewers: aaron.ballman, delesley, jmgao, rtrieu

Reviewed By: delesley

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-capabilities.c
cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=342605=342604=342605=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 19 17:39:27 
2018
@@ -3008,6 +3008,14 @@ def warn_invalid_capability_name : Warni
 def warn_thread_attribute_ignored : Warning<
   "ignoring %0 attribute because its argument is invalid">,
   InGroup, DefaultIgnore;
+def warn_thread_attribute_not_on_non_static_member : Warning<
+  "%0 attribute without capability arguments can only be applied to non-static 
"
+  "methods of a class">,
+  InGroup, DefaultIgnore;
+def warn_thread_attribute_not_on_capability_member : Warning<
+  "%0 attribute without capability arguments refers to 'this', but %1 isn't "
+  "annotated with 'capability' or 'scoped_lockable' attribute">,
+  InGroup, DefaultIgnore;
 def warn_thread_attribute_argument_not_lockable : Warning<
   "%0 attribute requires arguments whose type is annotated "
   "with 'capability' attribute; type here is %1">,

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=342605=342604=342605=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Sep 19 17:39:27 2018
@@ -476,6 +476,29 @@ static const RecordType *getRecordType(Q
   return nullptr;
 }
 
+template 
+static bool checkRecordDeclForAttr(const RecordDecl *RD) {
+  // Check if the record itself has the attribute.
+  if (RD->hasAttr())
+return true;
+
+  // Else check if any base classes have the attribute.
+  if (const auto *CRD = dyn_cast(RD)) {
+CXXBasePaths BPaths(false, false);
+if (CRD->lookupInBases(
+[](const CXXBaseSpecifier *BS, CXXBasePath &) {
+  const auto  = *BS->getType();
+  // If it's type-dependent, we assume it could have the attribute.
+  if (Ty.isDependentType())
+return true;
+  return Ty.getAs()->getDecl()->hasAttr();
+},
+BPaths, true))
+  return true;
+  }
+  return false;
+}
+
 static bool checkRecordTypeForCapability(Sema , QualType Ty) {
   const RecordType *RT = getRecordType(Ty);
 
@@ -491,21 +514,7 @@ static bool checkRecordTypeForCapability
   if (threadSafetyCheckIsSmartPointer(S, RT))
 return true;
 
-  // Check if the record itself has a capability.
-  RecordDecl *RD = RT->getDecl();
-  if (RD->hasAttr())
-return true;
-
-  // Else check if any base classes have a capability.
-  if (const auto *CRD = dyn_cast(RD)) {
-CXXBasePaths BPaths(false, false);
-if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
-  const auto *Type = BS->getType()->getAs();
-  return Type->getDecl()->hasAttr();
-}, BPaths))
-  return true;
-  }
-  return false;
+  return checkRecordDeclForAttr(RT->getDecl());
 }
 
 static bool checkTypedefTypeForCapability(QualType Ty) {
@@ -563,8 +572,27 @@ static bool isCapabilityExpr(Sema , co
 static void checkAttrArgsAreCapabilityObjs(Sema , Decl *D,
const ParsedAttr ,
   

[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342605: Thread Safety Analysis: warnings for attributes 
without arguments (authored by aaronpuchert, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51901?vs=165004=166205#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51901

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-capabilities.c
  test/SemaCXX/warn-thread-safety-parsing.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -476,6 +476,29 @@
   return nullptr;
 }
 
+template 
+static bool checkRecordDeclForAttr(const RecordDecl *RD) {
+  // Check if the record itself has the attribute.
+  if (RD->hasAttr())
+return true;
+
+  // Else check if any base classes have the attribute.
+  if (const auto *CRD = dyn_cast(RD)) {
+CXXBasePaths BPaths(false, false);
+if (CRD->lookupInBases(
+[](const CXXBaseSpecifier *BS, CXXBasePath &) {
+  const auto  = *BS->getType();
+  // If it's type-dependent, we assume it could have the attribute.
+  if (Ty.isDependentType())
+return true;
+  return Ty.getAs()->getDecl()->hasAttr();
+},
+BPaths, true))
+  return true;
+  }
+  return false;
+}
+
 static bool checkRecordTypeForCapability(Sema , QualType Ty) {
   const RecordType *RT = getRecordType(Ty);
 
@@ -491,21 +514,7 @@
   if (threadSafetyCheckIsSmartPointer(S, RT))
 return true;
 
-  // Check if the record itself has a capability.
-  RecordDecl *RD = RT->getDecl();
-  if (RD->hasAttr())
-return true;
-
-  // Else check if any base classes have a capability.
-  if (const auto *CRD = dyn_cast(RD)) {
-CXXBasePaths BPaths(false, false);
-if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
-  const auto *Type = BS->getType()->getAs();
-  return Type->getDecl()->hasAttr();
-}, BPaths))
-  return true;
-  }
-  return false;
+  return checkRecordDeclForAttr(RT->getDecl());
 }
 
 static bool checkTypedefTypeForCapability(QualType Ty) {
@@ -563,8 +572,27 @@
 static void checkAttrArgsAreCapabilityObjs(Sema , Decl *D,
const ParsedAttr ,
SmallVectorImpl ,
-   int Sidx = 0,
+   unsigned Sidx = 0,
bool ParamIdxOk = false) {
+  if (Sidx == AL.getNumArgs()) {
+// If we don't have any capability arguments, the attribute implicitly
+// refers to 'this'. So we need to make sure that 'this' exists, i.e. we're
+// a non-static method, and that the class is a (scoped) capability.
+const auto *MD = dyn_cast(D);
+if (MD && !MD->isStatic()) {
+  const CXXRecordDecl *RD = MD->getParent();
+  // FIXME -- need to check this again on template instantiation
+  if (!checkRecordDeclForAttr(RD) &&
+  !checkRecordDeclForAttr(RD))
+S.Diag(AL.getLoc(),
+   diag::warn_thread_attribute_not_on_capability_member)
+<< AL << MD->getParent();
+} else {
+  S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)
+  << AL;
+}
+  }
+
   for (unsigned Idx = Sidx; Idx < AL.getNumArgs(); ++Idx) {
 Expr *ArgExp = AL.getArgAsExpr(Idx);
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3008,6 +3008,14 @@
 def warn_thread_attribute_ignored : Warning<
   "ignoring %0 attribute because its argument is invalid">,
   InGroup, DefaultIgnore;
+def warn_thread_attribute_not_on_non_static_member : Warning<
+  "%0 attribute without capability arguments can only be applied to non-static "
+  "methods of a class">,
+  InGroup, DefaultIgnore;
+def warn_thread_attribute_not_on_capability_member : Warning<
+  "%0 attribute without capability arguments refers to 'this', but %1 isn't "
+  "annotated with 'capability' or 'scoped_lockable' attribute">,
+  InGroup, DefaultIgnore;
 def warn_thread_attribute_argument_not_lockable : Warning<
   "%0 attribute requires arguments whose type is annotated "
   "with 'capability' attribute; type here is %1">,
Index: test/SemaCXX/warn-thread-safety-parsing.cpp
===
--- test/SemaCXX/warn-thread-safety-parsing.cpp
+++ test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -572,11 +572,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void elf_function() EXCLUSIVE_LOCK_FUNCTION();
+void elf_function() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' 

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

[Davide Italiano via Phabricator via cfe-commits]

davide added a comment.

This broke all the lldb bots as well, FWIW.


https://reviews.llvm.org/D44263



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

[clang-tools-extra] r342601 - [Clang-tidy] Alphabetical sort of files/checks. Add space after clang-tidy in source code headers.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed Sep 19 17:02:55 2018
New Revision: 342601

URL: http://llvm.org/viewvc/llvm-project?rev=342601=rev
Log:
[Clang-tidy] Alphabetical sort of files/checks. Add space after clang-tidy in 
source code headers.

Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/add_new_check.py
clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/android/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=342601=342600=342601=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Wed Sep 19 
17:02:55 2018
@@ -34,10 +34,10 @@ public:
 CheckFactories.registerCheck("abseil-no-namespace");
 CheckFactories.registerCheck(
 "abseil-redundant-strcat-calls");
-CheckFactories.registerCheck(
-"abseil-string-find-startswith");
 CheckFactories.registerCheck(
 "abseil-str-cat-append");
+CheckFactories.registerCheck(
+"abseil-string-find-startswith");
   }
 };
 

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=342601=342600=342601=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Wed Sep 19 
17:02:55 2018
@@ -7,8 +7,8 @@ add_clang_library(clangTidyAbseilModule
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp
   RedundantStrcatCallsCheck.cpp
-  StringFindStartswithCheck.cpp
   StrCatAppendCheck.cpp
+  StringFindStartswithCheck.cpp
 
   LINK_LIBS
   clangAST

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=342601=342600=342601=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Wed Sep 19 17:02:55 2018
@@ -56,8 +56,8 @@ def write_header(module_path, module, ch
 + check_name_camel.upper() + '_H')
 f.write('//===--- ')
 f.write(os.path.basename(filename))
-f.write(' - clang-tidy')
-f.write('-' * max(0, 43 - len(os.path.basename(filename
+f.write(' - clang-tidy ')
+f.write('-' * max(0, 42 - len(os.path.basename(filename
 f.write('*- C++ -*-===//')
 f.write("""
 //
@@ -107,8 +107,8 @@ def write_implementation(module_path, mo
   with open(filename, 'w') as f:
 f.write('//===--- ')
 f.write(os.path.basename(filename))
-f.write(' - clang-tidy')
-f.write('-' * max(0, 52 - len(os.path.basename(filename
+f.write(' - clang-tidy ')
+f.write('-' * max(0, 51 - len(os.path.basename(filename
 f.write('-===//')
 f.write("""
 //

Modified: clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp?rev=342601=342600=342601=diff
==
--- clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/android/AndroidTidyModule.cpp Wed Sep 19 
17:02:55 2018
@@ -37,16 +37,16 @@ public:
 
CheckFactories.registerCheck("android-cloexec-accept4");
 CheckFactories.registerCheck("android-cloexec-accept");
 CheckFactories.registerCheck("android-cloexec-creat");
+CheckFactories.registerCheck("android-cloexec-dup");
 CheckFactories.registerCheck(
 "android-cloexec-epoll-create1");
 CheckFactories.registerCheck(
 "android-cloexec-epoll-create");
-CheckFactories.registerCheck("android-cloexec-dup");
 CheckFactories.registerCheck("android-cloexec-fopen");
-CheckFactories.registerCheck(
-"android-cloexec-inotify-init");
 CheckFactories.registerCheck(
 "android-cloexec-inotify-init1");
+CheckFactories.registerCheck(
+"android-cloexec-inotify-init");
 

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342600: Thread safety analysis: Handle ObjCIvarRefExpr in 
SExprBuilder::translate (authored by aaronpuchert, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52200?vs=166051=166197#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52200

Files:
  include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  lib/Analysis/ThreadSafetyCommon.cpp
  test/SemaObjCXX/warn-thread-safety-analysis.mm

Index: include/clang/Analysis/Analyses/ThreadSafetyCommon.h
===
--- include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -397,6 +397,8 @@
CallingContext *Ctx) ;
   til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext *Ctx);
   til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx);
+  til::SExpr *translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+   CallingContext *Ctx);
   til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx,
 const Expr *SelfE = nullptr);
   til::SExpr *translateCXXMemberCallExpr(const CXXMemberCallExpr *ME,
Index: test/SemaObjCXX/warn-thread-safety-analysis.mm
===
--- test/SemaObjCXX/warn-thread-safety-analysis.mm
+++ test/SemaObjCXX/warn-thread-safety-analysis.mm
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -Wno-objc-root-class %s
+
+class __attribute__((lockable)) Lock {
+public:
+  void Acquire() __attribute__((exclusive_lock_function())) {}
+  void Release() __attribute__((unlock_function())) {}
+};
+
+class __attribute__((scoped_lockable)) AutoLock {
+public:
+  AutoLock(Lock ) __attribute__((exclusive_lock_function(lock)))
+  : lock_(lock) {
+lock.Acquire();
+  }
+  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
+
+private:
+  Lock _;
+};
+
+@interface MyInterface {
+@private
+  Lock lock_;
+  int value_;
+}
+
+- (void)incrementValue;
+- (void)decrementValue;
+
+@end
+
+@implementation MyInterface
+
+- (void)incrementValue {
+  AutoLock lock(lock_);
+  value_ += 1;
+}
+
+- (void)decrementValue {
+  lock_.Acquire(); // expected-note{{mutex acquired here}}
+  value_ -= 1;
+} // expected-warning{{mutex 'self->lock_' is still held at the end of function}}
+
+@end
Index: lib/Analysis/ThreadSafetyCommon.cpp
===
--- lib/Analysis/ThreadSafetyCommon.cpp
+++ lib/Analysis/ThreadSafetyCommon.cpp
@@ -211,6 +211,8 @@
 return translateCXXThisExpr(cast(S), Ctx);
   case Stmt::MemberExprClass:
 return translateMemberExpr(cast(S), Ctx);
+  case Stmt::ObjCIvarRefExprClass:
+return translateObjCIVarRefExpr(cast(S), Ctx);
   case Stmt::CallExprClass:
 return translateCallExpr(cast(S), Ctx);
   case Stmt::CXXMemberCallExprClass:
@@ -311,9 +313,9 @@
   return nullptr;
 }
 
-static bool hasCppPointerType(const til::SExpr *E) {
+static bool hasAnyPointerType(const til::SExpr *E) {
   auto *VD = getValueDeclFromSExpr(E);
-  if (VD && VD->getType()->isPointerType())
+  if (VD && VD->getType()->isAnyPointerType())
 return true;
   if (const auto *C = dyn_cast(E))
 return C->castOpcode() == til::CAST_objToPtr;
@@ -344,7 +346,20 @@
 D = getFirstVirtualDecl(VD);
 
   til::Project *P = new (Arena) til::Project(E, D);
-  if (hasCppPointerType(BE))
+  if (hasAnyPointerType(BE))
+P->setArrow(true);
+  return P;
+}
+
+til::SExpr *SExprBuilder::translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+   CallingContext *Ctx) {
+  til::SExpr *BE = translate(IVRE->getBase(), Ctx);
+  til::SExpr *E = new (Arena) til::SApply(BE);
+
+  const auto *D = cast(IVRE->getDecl()->getCanonicalDecl());
+
+  til::Project *P = new (Arena) til::Project(E, D);
+  if (hasAnyPointerType(BE))
 P->setArrow(true);
   return P;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342600 - Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Wed Sep 19 16:57:38 2018
New Revision: 342600

URL: http://llvm.org/viewvc/llvm-project?rev=342600=rev
Log:
Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

Summary:
This imitates the code for MemberExpr.

Fixes PR38896.

Reviewers: aaron.ballman, delesley, lukasza, rjmccall

Reviewed By: delesley

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h?rev=342600=342599=342600=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h Wed Sep 19 
16:57:38 2018
@@ -397,6 +397,8 @@ private:
CallingContext *Ctx) ;
   til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext *Ctx);
   til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx);
+  til::SExpr *translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+   CallingContext *Ctx);
   til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx,
 const Expr *SelfE = nullptr);
   til::SExpr *translateCXXMemberCallExpr(const CXXMemberCallExpr *ME,

Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=342600=342599=342600=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Wed Sep 19 16:57:38 2018
@@ -211,6 +211,8 @@ til::SExpr *SExprBuilder::translate(cons
 return translateCXXThisExpr(cast(S), Ctx);
   case Stmt::MemberExprClass:
 return translateMemberExpr(cast(S), Ctx);
+  case Stmt::ObjCIvarRefExprClass:
+return translateObjCIVarRefExpr(cast(S), Ctx);
   case Stmt::CallExprClass:
 return translateCallExpr(cast(S), Ctx);
   case Stmt::CXXMemberCallExprClass:
@@ -311,9 +313,9 @@ static const ValueDecl *getValueDeclFrom
   return nullptr;
 }
 
-static bool hasCppPointerType(const til::SExpr *E) {
+static bool hasAnyPointerType(const til::SExpr *E) {
   auto *VD = getValueDeclFromSExpr(E);
-  if (VD && VD->getType()->isPointerType())
+  if (VD && VD->getType()->isAnyPointerType())
 return true;
   if (const auto *C = dyn_cast(E))
 return C->castOpcode() == til::CAST_objToPtr;
@@ -344,7 +346,20 @@ til::SExpr *SExprBuilder::translateMembe
 D = getFirstVirtualDecl(VD);
 
   til::Project *P = new (Arena) til::Project(E, D);
-  if (hasCppPointerType(BE))
+  if (hasAnyPointerType(BE))
+P->setArrow(true);
+  return P;
+}
+
+til::SExpr *SExprBuilder::translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+   CallingContext *Ctx) {
+  til::SExpr *BE = translate(IVRE->getBase(), Ctx);
+  til::SExpr *E = new (Arena) til::SApply(BE);
+
+  const auto *D = cast(IVRE->getDecl()->getCanonicalDecl());
+
+  til::Project *P = new (Arena) til::Project(E, D);
+  if (hasAnyPointerType(BE))
 P->setArrow(true);
   return P;
 }

Added: cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm?rev=342600=auto
==
--- cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm (added)
+++ cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm Wed Sep 19 
16:57:38 2018
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta 
-Wno-objc-root-class %s
+
+class __attribute__((lockable)) Lock {
+public:
+  void Acquire() __attribute__((exclusive_lock_function())) {}
+  void Release() __attribute__((unlock_function())) {}
+};
+
+class __attribute__((scoped_lockable)) AutoLock {
+public:
+  AutoLock(Lock ) __attribute__((exclusive_lock_function(lock)))
+  : lock_(lock) {
+lock.Acquire();
+  }
+  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
+
+private:
+  Lock _;
+};
+
+@interface MyInterface {
+@private
+  Lock lock_;
+  int value_;
+}
+
+- (void)incrementValue;
+- (void)decrementValue;
+
+@end
+
+@implementation MyInterface
+
+- (void)incrementValue {
+  AutoLock lock(lock_);
+  value_ += 1;
+}
+
+- (void)decrementValue {
+  lock_.Acquire(); // expected-note{{mutex acquired here}}
+  value_ -= 1;
+} // expected-warning{{mutex 'self->lock_' is still held at the end of 
function}}
+
+@end

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

It seems that `self` is an ordinary `DeclRefExpr` unlike `this`, which is a 
`CXXThisExpr`. Which means we'd have to make it dependent on the name whether 
we drop it, but `self` in C/C++ is just an ordinary variable. So I think I'll 
leave the `self->` part for now. It certainly doesn't hurt.


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Reverted in r342599 .


https://reviews.llvm.org/D44263



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

[PATCH] D51187: [RFC] Thread safety analysis: Track status of scoped capability

[Delesley Hutchins via Phabricator via cfe-commits]

delesley added inline comments.



Comment at: lib/Analysis/ThreadSafety.cpp:929
+  return Handler.handleDoubleLock(DiagKind, entry.toString(), entry.loc());
+Locked = true;
+

It's been a while since I last looked at this code, but I don't think you can 
use mutable fields in a FactEntry.  The analysis creates a FactSet for each 
program point, but each FactSet simply has pointers (FactIDs) for the 
underlying FactEntries.  If you change the definition of a FactEntry, it will 
change that definition for every program point.  So if you do an unlock on one 
side of a branch, and change the underlying FactEntry to reflect that, then 
analysis will then think you have also done the unlock on the other side of the 
branch.  

Any changes should always be done by adding or removing entries from the 
FactSet, not by mutating the underlying FactEntries.



Repository:
  rC Clang

https://reviews.llvm.org/D51187



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

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Thanks for this! LGTM after erichkeane's comments are resolved.

> I did a little digging on this, and it seems to be to keep track of a 
> declarations linkage for caching sake

Yeah, otherwise, we get exponential behavior on some pathological template-y 
patterns.




Comment at: lib/AST/Linkage.h:93
   static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
   }

(FWIW, it looks like `PointerIntPairInfo::UpdateInt` asserts that 
`Kind.toBits()` fits nicely in `NumLVComputationKindBits`. So if anything gets 
added, it'll yell and we can just revert to the current way of doing this :) )


Repository:
  rC Clang

https://reviews.llvm.org/D52268



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

[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Looks like you patch is not based on trunk. Please rebase.




Comment at: docs/ReleaseNotes.rst:60
 
+- New `modernize-replace-generic-functor-call
+  
`_
 check

Eugene.Zelenko wrote:
> Please sort new checks list in alphabetical order.
Please take a look on trunk Release Notes and use :doc: for link.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



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

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Seems like revert didn't go through. I suspect it is causing build failure in 
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/49593/consoleText

  SANITIZER_TEST_OBJECTS.sanitizer_vector_test.cc.i386.o 
SANITIZER_TEST_OBJECTS.gtest-all.cc.i386.o 
/Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/sanitizer_common/tests/libRTSanitizerCommon.test.osx.a
 -o 
/Users/buildslave/jenkins/workspace/clang-stage1-configure-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/sanitizer_common/tests/./Sanitizer-i386-Test
 -g --driver-mode=g++ -stdlib=libc++ -lc++ -lc++abi -fapplication-extension 
-mmacosx-version-min=10.9 -isysroot 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk
 -Wl,-U,___sanitizer_free_hook -Wl,-U,___sanitizer_malloc_hook 
-Wl,-U,___sanitizer_report_error_summary -Wl,-U,___sanitizer_sandbox_on_notify 
-Wl,-U,___sanitizer_symbolize_code -Wl,-U,___sanitizer_symbolize_data 
-Wl,-U,___sanitizer_symbolize_demangle -Wl,-U,___sanitizer_symbolize_flush -ldl 
-pthread -arch i386
  Undefined symbols for architecture i386:
"std::__1::basic_ostream 
>::operator<<(std::nullptr_t)", referenced from:
std::__1::basic_string, 
std::__1::allocator > 
testing::PrintToString(std::nullptr_t const&) in 
SANITIZER_TEST_OBJECTS.sanitizer_quarantine_test.cc.i386.o
std::__1::basic_string, 
std::__1::allocator > 
testing::PrintToString(std::nullptr_t const&) in 
SANITIZER_TEST_OBJECTS.sanitizer_stacktrace_printer_test.cc.i386.o
  ld: symbol(s) not found for architecture i386
  clang-8: error: linker command failed with exit code 1 (use -v to see 
invocation)


https://reviews.llvm.org/D44263



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

[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:114
+}
+} // namespace modernize
+} // namespace tidy

Please insert empty line above.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



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

[PATCH] D52281: Use std::invoke in generic code

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:1
+//===--- ReplaceGenericFunctorCallCheck.cpp - 
clang-tidy---===//
+//

Please add space after clang-tidy.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:48
+const MatchFinder::MatchResult ) {
+
+  const auto *Source = Result.SourceManager;

Please remove empty line.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:49
+
+  const auto *Source = Result.SourceManager;
+

Please don't use auto when type could not be deduced from same statement.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:53
+  if (Functor && Functor->isTypeDependent()) {
+const auto EndLoc = Functor->getNumArgs() == 0
+? Functor->getRParenLoc()

Please don't use auto when type could not be deduced from same statement.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+  if (MFunctor && MFunctor->isTypeDependent()) {
+const auto *Paren = static_cast(MFunctor->getCallee());
+const auto *BinOp =

I think you should use LLVM's cast instead, but I'm not sure which one. Same 
for other places.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:78
+
+const auto *MFunc = BinOp->getRHS();
+const char *MFuncEnd = Source->getCharacterData(Paren->getRParen());

Please don't use auto when type could not be deduced from same statement.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:96
+const std::string ) {
+
+  std::ostringstream Replace;

Please remove empty line.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h:1
+//===--- ReplaceGenericFunctorCallCheck.h - clang-tidy---*- C++ 
-*-===//
+//

Please add space after clang-tidy.



Comment at: docs/ReleaseNotes.rst:60
 
+- New `modernize-replace-generic-functor-call
+  
`_
 check

Please sort new checks list in alphabetical order.



Comment at: docs/ReleaseNotes.rst:63
+
+  Replace type dependent functor, function pointer and pointer to member call
+  to the proper ``std::invoke()`` in C++17 or newer codebases.

Replace -> Replaces.



Comment at: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst:6
+
+This check finds and replace the functor, function pointer and pointer to 
member
+calls to the proper ``std::invoke()`` call. It works only in C++17 or newer.

Please make first statement same as in Release Notes.



Comment at: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst:15
+  void f(T1 func, T2 mfp) {
+func(2); // Replace to ::std::invoke(func, 2)
+Foo foo;

Please use more descriptive identifiers and separate statements with empty 
lines. Same for other places.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

As per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868, I thought GCC also 
emits this error but only with -pedantic. So probably should keep this error 
but under -Wextra or another appropriate group?


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

Re: r342525 - [NFC] Fix uncompilable test cases of ExprMutationAnalyzer.

[David Green via cfe-commits]

Thanks!


That sorted us right out.


Dave



From: Shuai Wang
Sent: 19 September 2018 21:32
To: David Green
Cc: cfe-commits@lists.llvm.org
Subject: Re: r342525 - [NFC] Fix uncompilable test cases of 
ExprMutationAnalyzer.



On Wed, Sep 19, 2018 at 3:10 AM David Green 
mailto:david.gr...@arm.com>> wrote:

Hello!


You have some code in here that looks like

 A operator+(A&&, int) {}


which is a non-void function without a return statement. Any reason to use "{}" 
and not ";"? You seem to have deliberately changed them, but that causes us 
some problems downstream. Mind if we change them to A operator+(A&&, int);? or 
will that cause you problems in other places?

You're right, I don't need to define it, fixed in 
https://reviews.llvm.org/rC342586
Thanks for pointing it out!


Cheers

Dave

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

[PATCH] D52281: Use std::invoke in generic code

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

boga95 created this revision.
boga95 added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgorny.

In generic code (e.g.: in a type dependent expression), the `std::invoke` 
should be used, in order to support functors, function pointers, pointers to 
members, regular functions, and other C++ and STL features: link 

The check replace the appropriate calls with calls to std::invoke. This should 
work only in C++17 and newer codebases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
  test/clang-tidy/modernize-replace-generic-functor-call.cpp

Index: test/clang-tidy/modernize-replace-generic-functor-call.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-replace-generic-functor-call.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s modernize-replace-generic-functor-call %t -- -- -std=c++17
+
+namespace std {
+
+template 
+T max(T a, T b) {
+  return a < b ? b : a;
+}
+
+struct function {
+  void operator()();
+};
+
+} // namespace std
+
+struct Foo {
+  static void print_() {}
+  void print() const {}
+  int num_;
+};
+
+// Something that triggers the check
+template 
+void func2(T func) {
+  func(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func, 1);
+  func();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func);
+
+  Foo foo;
+  (foo.*func)(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func, 1);
+  (foo.*func)();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func);
+  (Foo().*func)(1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(Foo(), func, 1, 2);
+}
+
+// Something that doesn't triggers the check
+void func3() {}
+
+void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+  func3();// Regular function call
+  std::max(1, 2); // Template function call
+  Foo::print_();  // Static function call
+  []() {}();  // Lambda call
+  func(); // Call through std::function
+  fp(3);  // Call through function pointer
+  Foo foo;
+  (foo.*(mfp))(1); // Call through member function pointer
+}
Index: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - modernize-replace-generic-functor-call
+
+modernize-replace-generic-functor-call
+==
+
+This check finds and replace the functor, function pointer and pointer to member
+calls to the proper ``std::invoke()`` call. It works only in C++17 or newer.
+
+Examples:
+
+.. code-block:: c++
+
+  template
+  void f(T1 func, T2 mfp) {
+func(2); // Replace to ::std::invoke(func, 2)
+Foo foo;
+(foo.*mfp)(1, 2); // Replace to ::std::invoke(foo, mfp, 1, 2)
+(Foo().*mfp)(3); // Replace to ::std::invoke(Foo(), mfp, 3)
+  }
+
+  // Neither of them is type dependent, no diagnose
+  void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+func3();
+std::max(1,2);
+Foo::print_();
+[](){}();
+func();
+fp(3);
+Foo foo;
+(foo.*(mfp))(1);
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -163,6 +163,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-generic-functor-call
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
 Improvements to clang-tidy
 --
 
+- New `modernize-replace-generic-functor-call
+  

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

[Jorge Gorbe via Phabricator via cfe-commits]

jgorbe created this revision.
jgorbe added reviewers: christylee, rsmith, aaron.ballman.

r342177  introduced a hint in cases where an 
#included file is not found. It tries to find a suggestion by removing leading 
or trailing non-alphanumeric characters and checking if a matching file exists, 
then it reports an error like:

  include-likely-typo.c:3:10: error: '' file not 
found, did you mean 'empty_file_to_include.h'?
  #include "" // expected-error 
{{'' file not found, did you mean 
'empty_file_to_include.h'?}}
   ^~~
   "empty_file_to_include.h"
  1 error generated.

However, if a hint is **not** found, the error message will show only the 
trimmed name we use to look for a hint, so:

  #include "/non_existing_file."

will result in:

  include-leading-nonalpha-no-suggest.c:3:10: fatal error: 
'non_existing_file_to_include.h' file not found
  #include "/non_existing_file_to_include.h" // expected-error 
{{'/non_existing_file_to_include.h' file not found}}
   ^~~~
  1 error generated.

where the name reported after `"fatal error:"` doesn't match what the user 
wrote.

This change reports the original file name instead of the trimmed one when a 
suggestion is not found.


Repository:
  rC Clang

https://reviews.llvm.org/D52280

Files:
  lib/Lex/PPDirectives.cpp
  test/Preprocessor/include-leading-nonalpha-no-suggest.c
  test/Preprocessor/include-leading-nonalpha-suggest.c


Index: test/Preprocessor/include-leading-nonalpha-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/empty_file_to_include.h" // expected-error 
{{'/empty_file_to_include.h' file not found, did you mean 
'empty_file_to_include.h'?}}
Index: test/Preprocessor/include-leading-nonalpha-no-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-no-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/non_existing_file_to_include.h" // expected-error 
{{'/non_existing_file_to_include.h' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


Index: test/Preprocessor/include-leading-nonalpha-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/empty_file_to_include.h" // expected-error {{'/empty_file_to_include.h' file not found, did you mean 'empty_file_to_include.h'?}}
Index: test/Preprocessor/include-leading-nonalpha-no-suggest.c
===
--- /dev/null
+++ test/Preprocessor/include-leading-nonalpha-no-suggest.c
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify
+
+#include "/non_existing_file_to_include.h" // expected-error {{'/non_existing_file_to_include.h' file not found}}
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Symbol added: _ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEDn

  {'is_defined': True, 'type': 'FUNC', 'name': 
'_ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEDn'}

Symbol added: _ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEElsEDn

  {'is_defined': True, 'type': 'FUNC', 'name': 
'_ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEElsEDn'}

Symbol added: 
_ZNSt3__1lsIwNS_11char_traitsIwRNS_13basic_ostreamIT_T0_EES7_PKc

  {'is_defined': True, 'type': 'FUNC', 'name': 
'_ZNSt3__1lsIwNS_11char_traitsIwRNS_13basic_ostreamIT_T0_EES7_PKc'}

Symbol added: 
_ZNSt3__124__put_character_sequenceIcNS_11char_traitsIcRNS_13basic_ostreamIT_T0_EES7_PKS4_m

  {'is_defined': True, 'type': 'FUNC', 'name': 
'_ZNSt3__124__put_character_sequenceIcNS_11char_traitsIcRNS_13basic_ostreamIT_T0_EES7_PKS4_m'}


https://reviews.llvm.org/D44263



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

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists reopened this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.

Reverted in r342590 while I investigate additions to the dylib.


https://reviews.llvm.org/D44263



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

r342586 - [NFC] Declare instead of define non-void functions in unit tests.

[Shuai Wang via cfe-commits]

Author: shuaiwang
Date: Wed Sep 19 13:27:25 2018
New Revision: 342586

URL: http://llvm.org/viewvc/llvm-project?rev=342586=rev
Log:
[NFC] Declare instead of define non-void functions in unit tests.

Modified:
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Modified: cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp?rev=342586=342585=342586=diff
==
--- cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp (original)
+++ cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp Wed Sep 19 
13:27:25 2018
@@ -203,7 +203,7 @@ TEST(ExprMutationAnalyzerTest, ByValueAr
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode(
-  "struct A {}; A operator+(A, int) {} void f() { A x; x + 1; }");
+  "struct A {}; A operator+(A, int); void f() { A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
@@ -239,7 +239,7 @@ TEST(ExprMutationAnalyzerTest, ByConstVa
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode(
-  "struct A {}; A operator+(const A, int) {} void f() { A x; x + 1; }");
+  "struct A {}; A operator+(const A, int); void f() { A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
@@ -289,7 +289,7 @@ TEST(ExprMutationAnalyzerTest, ByNonCons
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
 
   AST = buildASTFromCode(
-  "struct A {}; A operator+(A&, int) {} void f() { A x; x + 1; }");
+  "struct A {}; A operator+(A&, int); void f() { A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x + 1"));
 
@@ -333,7 +333,7 @@ TEST(ExprMutationAnalyzerTest, ByConstRe
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode(
-  "struct A {}; A operator+(const A&, int) {} void f() { A x; x + 1; }");
+  "struct A {}; A operator+(const A&, int); void f() { A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
@@ -356,7 +356,7 @@ TEST(ExprMutationAnalyzerTest, ByNonCons
   EXPECT_THAT(mutatedBy(Results, AST.get()),
   ElementsAre("g(static_cast(x))"));
 
-  AST = buildASTFromCode("struct A {}; A operator+(A&&, int) {}"
+  AST = buildASTFromCode("struct A {}; A operator+(A&&, int);"
  "void f() { A x; static_cast(x) + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
@@ -382,7 +382,7 @@ TEST(ExprMutationAnalyzerTest, ByConstRR
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
-  AST = buildASTFromCode("struct A {}; A operator+(const A&&, int) {}"
+  AST = buildASTFromCode("struct A {}; A operator+(const A&&, int);"
  "void f() { A x; static_cast(x) + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));


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

[PATCH] D52259: [CUDA] Rearrange search path ordering to fix two test case failures

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

The patch does not seem to match the description and appears to have nothing to 
do with rearranging include paths. Could you check if these are the changes you 
intended to send for review.


Repository:
  rC Clang

https://reviews.llvm.org/D52259



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

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Delesley Hutchins via Phabricator via cfe-commits]

delesley added inline comments.



Comment at: lib/Analysis/ThreadSafetyCommon.cpp:362
+  til::Project *P = new (Arena) til::Project(E, D);
+  if (hasCppPointerType(BE))
+P->setArrow(true);

aaronpuchert wrote:
> rjmccall wrote:
> > aaron.ballman wrote:
> > > I feel like this will always return false. However, I wonder if 
> > > `IVRE->getBase()->isArrow()` is more appropriate here?
> > The base of an `ObjCIvarRefExpr` will never directly be a C pointer type.  
> > I don't know whether this data structure looks through casts, but it's 
> > certainly *possible* to cast arbitrarily weird C stuff to ObjC pointer 
> > type.  On the other hand, by and large nobody actually ever does that, so I 
> > wouldn't make a special case for it here.
> > 
> > `ObjCIvarRefExpr` just has an `isArrow()` method directly if this is just 
> > supposed to be capturing the difference between `.` and `->`.  But then, so 
> > does `MemberExpr`, and in that case you're doing this odd analysis of the 
> > base instead of trusting `isArrow()`, so I don't know what to tell you to 
> > do.
> > 
> > Overall it is appropriate to treat an ObjC ivar reference as a perfectly 
> > ordinary projection.  The only thing that's special about it is that the 
> > actual offset isn't necessarily statically known, but ivars still behave as 
> > if they're all independently declared, so I wouldn't worry about it.
> I had wondered about this as well, but when I changed `hasCppPointerType(BE)` 
> to `ME->isArrow()` in the `MemberExpr` case, I got some test failures. For 
> example:
> 
> ```
> struct Foo {
>   int a __attribute__((guarded_by(mu)));
>   Mutex mu;
> };
> 
> void f() {
>   Foo foo;
>   foo.a = 5; // \
> // expected-warning{{writing variable 'a' requires holding mutex 'foo.mu' 
> exclusively}}
> }
> ```
> 
> Instead we warn `writing variable 'a' requires holding mutex 'foo->mu' 
> exclusively`. That's not right.
> 
> The expression (`ME`) we are processing is `mu` from the attribute on `a`, 
> which the compiler sees as `this->mu`. So we get `ME->isArrow() == true` and 
> `ME->getBase()` is a `CXXThisExpr`.
> 
> Basically the `translate*` functions do a substitution. They try to derive 
> `foo.mu` from `this->mu` on the `a` member and the expression `foo.a`. The 
> annotation `this->mu` is the expression we get as parameter, and `foo.a` is 
> encoded in the `CallingContext`.
> 
> The information whether `foo.a` is an arrow (it isn't) seems to be in the 
> `CallingContext`'s `SelfArrow` member.
This is a recurring problem.  The fundamental issue is the analysis must 
compare expressions for equality, but many C++ expressions are syntactically 
different but semantically the same, like  ()->mu  and  foo.mu.  It's 
particularly problematic because (as Aaron notes) we frequently substitute 
arguments when constructing expressions, which makes it easy to get things like 
()->mu instead of foo.mu, when substituting  for a pointer argument. 

The purpose of the TIL is to translate C++ expressions into a simpler, 
lower-level IR, where syntactic equality in the IR corresponds to semantic 
equality between expressions.  The TIL doesn't distinguish between pointers and 
references, doesn't distinguish between -> and ., and ignores * and & as 
no-ops.  But then we have to recover the distinction between -> and . when we 
generate an error message.

In the presence of substitution, you can't go by whether the source syntax has 
an ->.  You have to look at whether the type of the underlying argument (after 
stripping away * and &) requires an arrow.

I know nothing about objective C, so I don't know what the right answer is here.


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

It's the responsibility of the caller to provide a corrected expression, this 
keeps the completion function simpler. I suggest we stick to this contract.
We do something in `getLangOpts().CPlusPlus` case to create a corrected 
expression, let's just implement a similar fallback for C at the single 
callsite that we have (`ParsePostfixExpressionSuffix`).


https://reviews.llvm.org/D52261



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

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande added a comment.

I did try a copy of https://reviews.llvm.org/D50949 again, locally and without 
this diff, to see if I can avoid messing with this structure altogether.  But 
then I get a `SEGV`.

The reason behind this diff is to try to make the logic a little safer, though 
it's probably still not bulletproof.  It at least does seem to fix, or "fix" 
(i.e. hide) the crash below.

I admit I didn't take the time to really dig into that one, and I'm not yet 
experienced enough in Clang's code to do so confidently, so it's a bit scary to 
continue going down rabbit holes :)

But I'm happy to revise (or go fix that problem) if you think that's best.  
Thanks again!

  Stack dump:
  0.Program arguments: /opt/llvm/build/bin/clang -cc1 -internal-isystem 
/opt/llvm/build/lib/clang/8.0.0/include -nostdsysteminc -fmodules -verify 
/opt/llvm/llvm/tools/clang/test/Modules/merge-lambdas.cpp -emit-llvm-only
  1./opt/llvm/llvm/tools/clang/test/Modules/merge-lambdas.cpp:73:1: at 
annotation token
  2./opt/llvm/build/tools/clang/test/Modules/PR38531.map:11:14: LLVM IR 
generation of declaration 'foo(Fun)::(anonymous class)::'
  0  clang0x0001032cffe8 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
  1  clang0x0001032cf165 llvm::sys::RunSignalHandlers() 
+ 85
  2  clang0x0001032d05f2 SignalHandler(int) + 258
  3  libsystem_platform.dylib 0x7fff79709f5a _sigtramp + 26
  4  libsystem_platform.dylib 0x7ffeed8dbe3f _sigtramp + 1948065535
  5  clang0x000104b865b1 (anonymous 
namespace)::CXXNameMangler::manglePrefix(clang::DeclContext const*, bool) + 545
  6  clang0x000104b877d5 (anonymous 
namespace)::CXXNameMangler::mangleNestedName(clang::NamedDecl const*, 
clang::DeclContext const*, llvm::SmallVector const*, bool) 
+ 453
  7  clang0x000104b8733d (anonymous 
namespace)::CXXNameMangler::mangleLocalName(clang::Decl const*, 
llvm::SmallVector const*) + 1613
  8  clang0x000104b77bf7 (anonymous 
namespace)::CXXNameMangler::mangleFunctionEncoding(clang::FunctionDecl const*) 
+ 87
  9  clang0x000104b760c5 (anonymous 
namespace)::ItaniumMangleContextImpl::mangleCXXCtor(clang::CXXConstructorDecl 
const*, clang::CXXCtorType, llvm::raw_ostream&) + 245
  10 clang0x00010368eb21 
getMangledNameImpl(clang::CodeGen::CodeGenModule const&, clang::GlobalDecl, 
clang::NamedDecl const*, bool) + 305
  11 clang0x00010368a83b 
clang::CodeGen::CodeGenModule::getMangledName(clang::GlobalDecl) + 427
  12 clang0x000103697135 
clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) + 1941
  13 clang0x0001036f7529 (anonymous 
namespace)::ItaniumCXXABI::EmitCXXConstructors(clang::CXXConstructorDecl 
const*) + 41
  14 clang0x00010371401f (anonymous 
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 143
  15 clang0x00010367a151 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 177
  16 clang0x00010367a244 
clang::BackendConsumer::HandleInterestingDecl(clang::DeclGroupRef) + 20
  17 clang0x00010427c2db 
clang::ASTReader::PassInterestingDeclsToConsumer() + 123
  18 clang0x000104243f8b 
clang::ASTReader::FinishedDeserializing() + 1867
  19 clang0x00010427be48 
clang::ASTReader::ReadDeclRecord(unsigned int) + 2808
  20 clang0x00010422b219 non-virtual thunk to 
clang::ASTReader::GetExternalDecl(unsigned int) + 89
  21 clang0x000104afe7a1 
clang::RedeclarableTemplateDecl::loadLazySpecializationsImpl() const + 241
  22 clang0x000104aff1dc 
clang::ClassTemplateDecl::findSpecialization(llvm::ArrayRef,
 void*&) + 28
  23 clang0x0001048096a2 
clang::Sema::CheckTemplateIdType(clang::TemplateName, clang::SourceLocation, 
clang::TemplateArgumentListInfo&) + 1170
  24 clang0x00010480c900 
clang::Sema::ActOnTemplateIdType(clang::CXXScopeSpec&, clang::SourceLocation, 
clang::OpaquePtr, clang::IdentifierInfo*, 
clang::SourceLocation, clang::SourceLocation, 
llvm::MutableArrayRef, clang::SourceLocation, 
bool, bool) + 1552
  25 clang0x0001041df3b1 
clang::Parser::AnnotateTemplateIdTokenAsType(bool) + 129
  26 clang0x00010416449a 
clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&, 
clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier, 
clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*) + 10410
  27 clang0x0001041eb69b 
clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&,
 clang::ParsingDeclSpec&, 

r342581 - test: actually fix the condition properly

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Sep 19 12:20:30 2018
New Revision: 342581

URL: http://llvm.org/viewvc/llvm-project?rev=342581=rev
Log:
test: actually fix the condition properly

I had locally changed the test to add an explicit triple to figure out the issue
with the SCEI buildbots, and that hid the error.  This now works with and
without the explicit triple.

Modified:
cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/test/Sema/format-strings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=342581=342580=342581=diff
==
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Wed Sep 19 12:20:30 2018
@@ -401,7 +401,7 @@ void bug7377_bad_length_mod_usage() {
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_TYPE__ == int
+#if __WINT_WIDTH__ == 32
   printf("%lc", (char) 1); // no-warning
 #else
   printf("%lc", (char) 1); // expected-warning{{the argument has type 'char'}}


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

[PATCH] D51901: Thread Safety Analysis: warnings for attributes without arguments

[Delesley Hutchins via Phabricator via cfe-commits]

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

This looks okay to me, but I have not tested it on a large code base to see if 
it breaks anything.




Comment at: lib/Sema/SemaDeclAttr.cpp:589
+<< AL << MD->getParent();
+} else
+  S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)

Curly braces around the else section.


Repository:
  rC Clang

https://reviews.llvm.org/D51901



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

[PATCH] D52276: [clangd] Add type boosting in code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
ilya-biryukov added a dependency: D52274: [clangd] Collect and store expected 
types in the index.
ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Ranking-related code should be moved to `Quality.cpp`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52276

Files:
  clangd/CodeComplete.cpp
  clangd/Quality.cpp
  clangd/Quality.h


Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -27,7 +27,7 @@
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
-
+#include "ExpectedTypes.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
@@ -90,6 +90,8 @@
   /// This is used to calculate proximity between the index symbol and the
   /// query.
   llvm::StringRef SymbolURI;
+  /// A multiplier to be applied due to matching type.
+  float TypeMatchMultiplier = 1.0;
   /// Proximity between best declaration and the query. [0-1], 1 is closest.
   /// FIXME: unify with index proximity score - signals should be
   /// source-independent.
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -348,6 +348,8 @@
   if (NeedsFixIts)
 Score *= 0.5;
 
+  Score *= TypeMatchMultiplier;
+
   return Score;
 }
 
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -23,6 +23,7 @@
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
+#include "ExpectedTypes.h"
 #include "FileDistance.h"
 #include "FuzzyMatch.h"
 #include "Headers.h"
@@ -1224,6 +1225,7 @@
   bool Incomplete = false; // Would more be available with a higher limit?
   llvm::Optional Filter;   // Initialized once Sema runs.
   std::vector QueryScopes;  // Initialized once Sema runs.
+  llvm::DenseMap ExpectedTypes; // Initialized once Sema runs.
   // Include-insertion and proximity scoring rely on the include structure.
   // This is available after Sema has run.
   llvm::Optional Inserter;  // Available during runWithSema.
@@ -1349,6 +1351,10 @@
 Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
 QueryScopes = getQueryScopes(Recorder->CCContext,
  Recorder->CCSema->getSourceManager());
+ExpectedTypes =
+SType::forCopyInitOf(Recorder->CCSema->getASTContext(),
+ Recorder->CCContext.getPreferredType());
+vlog("Added {0} expected types", ExpectedTypes.size());
 // Sema provides the needed context to query the index.
 // FIXME: in addition to querying for extra/overlapping symbols, we should
 //explicitly request symbols corresponding to Sema results.
@@ -1511,10 +1517,32 @@
 Relevance.merge(*Candidate.IndexResult);
 Origin |= Candidate.IndexResult->Origin;
 FromIndex = true;
+if (!ExpectedTypes.empty()) {
+  // FIXME(ibiryukov): this should live in Quality.cpp
+  auto TypeMatchMult =
+  typesMatch(ExpectedTypes, Candidate.IndexResult->Types);
+  if (TypeMatchMult) {
+vlog("Types matched in sema for {0}", Candidate.Name);
+Relevance.TypeMatchMultiplier =
+std::max(*TypeMatchMult, Relevance.TypeMatchMultiplier);
+  }
+}
   }
   if (Candidate.SemaResult) {
 Quality.merge(*Candidate.SemaResult);
 Relevance.merge(*Candidate.SemaResult);
+if (!ExpectedTypes.empty()) {
+  // FIXME(ibiryukov): this should live in Quality.cpp
+  auto TypeMatchMult = typesMatch(
+  ExpectedTypes,
+  SType::fromCompletionResult(Recorder->CCSema->getASTContext(),
+  *Candidate.SemaResult));
+  if (TypeMatchMult) {
+vlog("Types matched in sema for {0}", Candidate.Name);
+Relevance.TypeMatchMultiplier =
+std::max(*TypeMatchMult, Relevance.TypeMatchMultiplier);
+  }
+}
 Origin |= SymbolOrigin::AST;
   }
 }


Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -27,7 +27,7 @@
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_QUALITY_H
-
+#include "ExpectedTypes.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
@@ -90,6 +90,8 @@
   /// This is used to calculate proximity between the index symbol and the
   /// query.
   llvm::StringRef SymbolURI;
+  /// A multiplier to be 

[PATCH] D52276: [clangd] Add type boosting in code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Ranking-related code should be moved to `Quality.cpp`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52276



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

[PATCH] D52275: [Index] Expose USR generation for types

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added a subscriber: kadircet.

Used in clangd.


Repository:
  rC Clang

https://reviews.llvm.org/D52275

Files:
  include/clang/Index/USRGeneration.h
  lib/Index/USRGeneration.cpp


Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -1094,3 +1094,13 @@
   Out << MacroName;
   return false;
 }
+
+bool clang::index::generateUSRForType(QualType T, ASTContext , 
SmallVectorImpl ) {
+  if (T.isNull())
+return true;
+  T = T.getCanonicalType();
+
+  USRGenerator UG(, Buf);
+  UG.VisitType(T);
+  return UG.ignoreResults();
+}
Index: include/clang/Index/USRGeneration.h
===
--- include/clang/Index/USRGeneration.h
+++ include/clang/Index/USRGeneration.h
@@ -14,10 +14,12 @@
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
+class ASTContext;
 class Decl;
 class MacroDefinitionRecord;
 class SourceLocation;
 class SourceManager;
+class QualType;
 
 namespace index {
 
@@ -70,6 +72,12 @@
 bool generateUSRForMacro(StringRef MacroName, SourceLocation Loc,
  const SourceManager , SmallVectorImpl );
 
+
+/// Generates a USR for a type.
+///
+/// \return true on error, false on success.
+bool generateUSRForType(QualType T, ASTContext , SmallVectorImpl 
);
+
 } // namespace index
 } // namespace clang
 


Index: lib/Index/USRGeneration.cpp
===
--- lib/Index/USRGeneration.cpp
+++ lib/Index/USRGeneration.cpp
@@ -1094,3 +1094,13 @@
   Out << MacroName;
   return false;
 }
+
+bool clang::index::generateUSRForType(QualType T, ASTContext , SmallVectorImpl ) {
+  if (T.isNull())
+return true;
+  T = T.getCanonicalType();
+
+  USRGenerator UG(, Buf);
+  UG.VisitType(T);
+  return UG.ignoreResults();
+}
Index: include/clang/Index/USRGeneration.h
===
--- include/clang/Index/USRGeneration.h
+++ include/clang/Index/USRGeneration.h
@@ -14,10 +14,12 @@
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
+class ASTContext;
 class Decl;
 class MacroDefinitionRecord;
 class SourceLocation;
 class SourceManager;
+class QualType;
 
 namespace index {
 
@@ -70,6 +72,12 @@
 bool generateUSRForMacro(StringRef MacroName, SourceLocation Loc,
  const SourceManager , SmallVectorImpl );
 
+
+/// Generates a USR for a type.
+///
+/// \return true on error, false on success.
+bool generateUSRForType(QualType T, ASTContext , SmallVectorImpl );
+
 } // namespace index
 } // namespace clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52274: [clangd] Collect and store expected types in the index

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Need to store the types in RIFF format too


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52274



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

[PATCH] D52274: [clangd] Collect and store expected types in the index

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.

And add a hidden option to control whether the types are collected.
For experiments, will be removed when expected types implementation
is stabilized.

The index size is almost unchanged, e.g. the YAML index for all clangd
sources increased from 53MB to 54MB.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52274

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  clangd/indexer/IndexerMain.cpp

Index: clangd/indexer/IndexerMain.cpp
===
--- clangd/indexer/IndexerMain.cpp
+++ clangd/indexer/IndexerMain.cpp
@@ -67,6 +67,11 @@
  clEnumValN(Binary, "binary", "binary RIFF format")),
 llvm::cl::init(YAML));
 
+static llvm::cl::opt
+CollectTypes("collect-types",
+ llvm::cl::desc("Collect type information during indexing"),
+ llvm::cl::init(false), llvm::cl::Hidden);
+
 /// Responsible for aggregating symbols from each processed file and producing
 /// the final results. All methods in this class must be thread-safe,
 /// 'consumeSymbols' may be called from multiple threads.
@@ -142,6 +147,7 @@
 CollectorOpts.CollectIncludePath = true;
 CollectorOpts.CountReferences = true;
 CollectorOpts.Origin = SymbolOrigin::Static;
+CollectorOpts.CollectTypes = CollectTypes;
 auto Includes = llvm::make_unique();
 addSystemHeadersMapping(Includes.get());
 CollectorOpts.Includes = Includes.get();
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "SymbolYAML.h"
+#include "ExpectedTypes.h"
 #include "Index.h"
 #include "Serialization.h"
 #include "Trace.h"
@@ -21,10 +22,12 @@
 
 LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(clang::clangd::Symbol)
 LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Symbol::IncludeHeaderWithReferences)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::SType);
 
 namespace llvm {
 namespace yaml {
 
+using clang::clangd::SType;
 using clang::clangd::Symbol;
 using clang::clangd::SymbolID;
 using clang::clangd::SymbolLocation;
@@ -94,6 +97,44 @@
   }
 };
 
+template <> struct ScalarTraits {
+  static void output(const SType , void *, llvm::raw_ostream ) {
+Out << Value.toHexStr();
+  }
+  static StringRef input(StringRef Scalar, void *, SType ) {
+Type = SType::fromHexStr(Scalar);
+return StringRef();
+  }
+
+  static QuotingType mustQuote(StringRef) { return QuotingType::None; }
+};
+
+// A YamlIO normalizer for fields of type "ArrayRef" with underlying
+// array allocated on an arena. Normalizes to std::vector, so traits
+// should be provided for std::vector.
+template  struct ArenaArrayPtr {
+  ArenaArrayPtr(IO &) {}
+  ArenaArrayPtr(IO &, llvm::ArrayRef D) {
+Normalized.assign(D.begin(), D.end());
+  }
+
+  llvm::ArrayRef denormalize(IO ) {
+assert(IO.getContext() && "Expecting an arena (as context) to allocate "
+  "data for read symbols.");
+if (Normalized.empty())
+  return llvm::ArrayRef();
+// Allocate an array on the Arena and copy-construct the objects.
+auto *Allocator = static_cast(IO.getContext());
+T *Items = Allocator->Allocate(Normalized.size());
+for (size_t I = 0, Size = Normalized.size(); I < Size; ++I)
+  new (Items + I) T(Normalized[I]);
+// Return a reference to the array.
+return llvm::ArrayRef(Items, Items + Normalized.size());
+  }
+
+  std::vector Normalized;
+};
+
 template <> struct MappingTraits {
   static void mapping(IO , Symbol ) {
 MappingNormalization NSymbolID(IO, Sym.ID);
@@ -113,6 +154,9 @@
 IO.mapOptional("Documentation", Sym.Documentation);
 IO.mapOptional("ReturnType", Sym.ReturnType);
 IO.mapOptional("IncludeHeaders", Sym.IncludeHeaders);
+MappingNormalization, llvm::ArrayRef> NTypes(
+IO, Sym.Types);
+IO.mapOptional("Types", NTypes->Normalized);
   }
 };
 
@@ -169,7 +213,9 @@
 namespace clangd {
 
 SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent) {
-  llvm::yaml::Input Yin(YAMLContent);
+  // Store data of pointer fields (excl. StringRef) like `Types`.
+  llvm::BumpPtrAllocator Arena;
+  llvm::yaml::Input Yin(YAMLContent, );
   std::vector S;
   Yin >> S;
 
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -62,6 +62,8 @@
 /// collect macros. For example, `indexTopLevelDecls` will not index any
 /// macro even if this is true.
 bool CollectMacro = 

[PATCH] D52273: [clangd] Initial implementation of expected types

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ExpectedTypes.h:119
+  explicit SType(SHA1Array Data);
+  SHA1Array Data;
+};

I assume this will be controversial. Happy to discuss/change.
We are currently building this representation based on USRs for types, the 
alternative is to store the USRs directly. Would be a bit more 
debuggable/explainable in case of failures, but also not particularly readable.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52273



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

[PATCH] D52273: [clangd] Initial implementation of expected types

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The implementation might look a bit scary, please feel free to ask for 
comments/clarifications!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52273



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

[PATCH] D52273: [clangd] Initial implementation of expected types

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.

Provides facilities to model the C++ conversion rules without the AST.
The introduced representation can be stored in the index and used to
implement type-based ranking improvements for index-based completions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52273

Files:
  clangd/CMakeLists.txt
  clangd/ExpectedTypes.cpp
  clangd/ExpectedTypes.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ExpectedTypeTest.cpp

Index: unittests/clangd/ExpectedTypeTest.cpp
===
--- /dev/null
+++ unittests/clangd/ExpectedTypeTest.cpp
@@ -0,0 +1,475 @@
+//===-- SimpleTypeTests.cpp  *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangdUnit.h"
+#include "ExpectedTypes.h"
+#include "TestTU.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Type.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using detail::MockExpr;
+using detail::PartialConv;
+using detail::ValueCategory;
+
+using ::testing::ElementsAre;
+using ::testing::Field;
+using ::testing::Matcher;
+using ::testing::UnorderedElementsAre;
+using ::testing::UnorderedElementsAreArray;
+
+class ASTTest : public ::testing::Test {
+protected:
+  void build(llvm::StringRef Code) {
+assert(!AST && "AST built twice");
+AST = TestTU::withCode(Code).build();
+  }
+
+  const ValueDecl *decl(llvm::StringRef Name) {
+return ::cast(findDecl(*AST, Name));
+  }
+
+  QualType typeOf(llvm::StringRef Name) {
+return decl(Name)->getType().getCanonicalType();
+  }
+
+  ASTContext () { return AST->getASTContext(); }
+
+private:
+  // Set after calling build().
+  llvm::Optional AST;
+};
+
+class ExpectedTypeCollectorTest : public ASTTest {
+protected:
+  std::vector convertibleTo(QualType To) {
+std::vector Result;
+detail::collectConvertibleTo(ASTCtx(), To,
+ [&](PartialConv C) { Result.push_back(C); });
+return Result;
+  }
+
+  std::vector convertibleFrom(const NamedDecl *D) {
+std::vector Result;
+detail::collectConvertibleFrom(
+ASTCtx(),
+*MockExpr::forCompletion(CodeCompletionResult(D, CCP_Declaration)),
+[&](PartialConv C) { Result.push_back(C); });
+return Result;
+  }
+};
+
+// Matchers for l-values and r-values, which don't come from user-defined
+// conversions.
+MATCHER_P(lv, TypeStr, "") {
+  return arg.Cat == ValueCategory::LVal && arg.Type.getAsString() == TypeStr;
+}
+MATCHER_P(rv, TypeStr, "") {
+  return arg.Cat == ValueCategory::RVal && arg.Type.getAsString() == TypeStr;
+}
+
+Matcher converted(Matcher M) {
+  return AllOf(M, Field(::AfterUserConv, true));
+}
+
+std::vector>
+alsoConverted(std::vector> Matchers) {
+  std::vector> Result;
+  Result.reserve(Matchers.size() * 2);
+  for (auto M : Matchers) {
+Result.push_back(M);
+Result.push_back(converted(M));
+  }
+  return Result;
+}
+
+template 
+Matcher> stdConversions(StrT... TypeStrs) {
+  return UnorderedElementsAreArray(
+  alsoConverted({lv(TypeStrs)..., rv(TypeStrs)...}));
+}
+
+std::vector> concat(std::vector> L,
+ std::vector> R) {
+  L.reserve(L.size() + R.size());
+  L.insert(L.end(), R.begin(), R.end());
+  return L;
+}
+
+TEST_F(ExpectedTypeCollectorTest, NumericTypes) {
+  build(R"cpp(
+bool b;
+int i;
+unsigned int ui;
+long long ll;
+float f;
+double d;
+  )cpp");
+
+  EXPECT_THAT(convertibleTo(decl("i")->getType()),
+  stdConversions("int", "float", "const int", "const float"));
+  EXPECT_THAT(convertibleFrom(decl("i")), UnorderedElementsAre(lv("int")));
+
+  const ValueDecl *Ints[] = {decl("b"), decl("ui"), decl("ll")};
+  for (const auto *D : Ints) {
+std::string DType = D->getType().getAsString();
+EXPECT_THAT(
+convertibleTo(D->getType()),
+stdConversions(DType, "int", "float", "const int", "const float"));
+EXPECT_THAT(convertibleFrom(D), UnorderedElementsAre(lv(DType), rv("int")));
+  }
+
+  // Check float.
+  EXPECT_THAT(convertibleTo(decl("f")->getType()),
+  stdConversions("int", "float", "const int", "const float"));
+  EXPECT_THAT(convertibleFrom(decl("f")), UnorderedElementsAre(lv("float")));
+  // Check double.
+  EXPECT_THAT(
+  convertibleTo(decl("d")->getType()),
+  stdConversions("int", "double", "float", "const int", "const float"));
+ 

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

[Wojtek Gumuła via Phabricator via cfe-commits]

wgml marked 2 inline comments as done.
wgml added inline comments.



Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:102-108
+  if (childrenCount(ND.decls()) == 0) {
+SourceRange Removal(Namespaces.front().Begin, Namespaces.front().RBrace);
+
+diag(Namespaces.front().Begin,
+ "Nested namespaces are empty and can be removed",
+ DiagnosticIDs::Warning)
+<< FixItHint::CreateRemoval(Removal);

alexfh wrote:
> wgml wrote:
> > wgml wrote:
> > > lebedev.ri wrote:
> > > > 1. This is unrelated to the main check, so it should be in a separate 
> > > > check.
> > > > 2. This is really fragile i think. What about preprocessor-driven 
> > > > emptiness?
> > > 1. It seems to me that if I produce fixit to concat empty namespace (that 
> > > is, execute the `else` branch), the fix is not applied as I intended - 
> > > the namespace gets deleted.
> > > I do not fully understand this behavior and did not manage to find bug in 
> > > the code. I also checked what will happen if I try replace the entire 
> > > namespace with `namespace {}` and it is not getting inserted but simply 
> > > deleted. On the other hand, I if replace with `namespace {;}`, I see the 
> > > expected output. So, either it is really sneaky off-by-one somewhere, or 
> > > clang does it's own thing.
> > > Truth be told, I added that `if` branch to handle such behavior 
> > > explicitly instead of silently deleting namespace.
> > > 
> > > 2. Can you provide an example when it will matter?
> > I dropped the "support" for removing. However, empty namespaces are still 
> > being removed implicitly, by a fixit applying tool, I believe.
> > 
> > I added entries to the test to make sure preprocessor stuff is never 
> > removed.
> Removal of empty namespaces is intentional. Clang-tidy calls 
> clang::format::cleanupAroundReplacements when applying fixes, which, in 
> particular, removes empty namespaces. See code around 
> clang/lib/Format/Format.cpp:1309. The motivation is that when different 
> checks remove different parts of code inside a namespace and this results in 
> an empty namespace, it's better to remove it. Other cleanups are, for 
> example, removal of commas and the colon in constructor initializer lists.
That is good to know! Everything looks good to me then.


https://reviews.llvm.org/D52136



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

r342569 - test: improve condition for the check

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Sep 19 11:41:07 2018
New Revision: 342569

URL: http://llvm.org/viewvc/llvm-project?rev=342569=rev
Log:
test: improve condition for the check

When the type of `wint_t` is `int`, the promotion will allow this to pass.
Check this explicitly rather than using the size.

Modified:
cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/test/Sema/format-strings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=342569=342568=342569=diff
==
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Wed Sep 19 11:41:07 2018
@@ -401,7 +401,7 @@ void bug7377_bad_length_mod_usage() {
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_WIDTH__ == 4
+#if __WINT_TYPE__ == int
   printf("%lc", (char) 1); // no-warning
 #else
   printf("%lc", (char) 1); // expected-warning{{the argument has type 'char'}}


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

Re: r342214 - remove 11 years old videos from the homepage. if you have a suggestion, please drop me an email

[Sylvestre Ledru via cfe-commits]

Both videos return 404  anyway :)

I created a redirect to llvm.org

S


Le 17/09/2018 à 22:02, David Blaikie a écrit :
> If you're going to remove these, might as well remove the html files
> they link to?
>
> On the other hand, might be worth keeping these around/linked to, but
> for posterity/archival purposes, rather than as sources of
> legitimately useful information for people trying to use LLVM today.
>
> On Fri, Sep 14, 2018 at 2:02 AM Sylvestre Ledru via cfe-commits
> mailto:cfe-commits@lists.llvm.org>> wrote:
>
> Author: sylvestre
> Date: Fri Sep 14 02:00:48 2018
> New Revision: 342214
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342214=rev
> Log:
> remove 11 years old videos from the homepage. if you have a
> suggestion, please drop me an email
>
> Modified:
>     cfe/trunk/www/index.html
>
> Modified: cfe/trunk/www/index.html
> URL:
> 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/www/index.html?rev=342214=342213=342214=diff
> 
> ==
> --- cfe/trunk/www/index.html (original)
> +++ cfe/trunk/www/index.html Fri Sep 14 02:00:48 2018
> @@ -74,16 +74,6 @@
>       motivations for starting work on a new front-end that could
>       meet these needs.
>
> -  A good (but quite dated) introduction to Clang can be found
> in the
> -     following video lectures:
> -
> -  
> -    Clang Introduction
> -        (May 2007)
> -    Features and
> Performance of
> -        Clang  (July 2007)
> -  
> -
>    For a more detailed comparison between Clang and other
> compilers, please
>       see the Clang comparison page.
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342568 - redirecting to llvm.org/devmtg

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Wed Sep 19 11:39:15 2018
New Revision: 342568

URL: http://llvm.org/viewvc/llvm-project?rev=342568=rev
Log:
redirecting to llvm.org/devmtg

Modified:
cfe/trunk/www/clang_video-05-25-2007.html
cfe/trunk/www/clang_video-07-25-2007.html

Modified: cfe/trunk/www/clang_video-05-25-2007.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/clang_video-05-25-2007.html?rev=342568=342567=342568=diff
==
--- cfe/trunk/www/clang_video-05-25-2007.html (original)
+++ cfe/trunk/www/clang_video-05-25-2007.html Wed Sep 19 11:39:15 2018
@@ -6,22 +6,10 @@
2007 LLVM Developer's Meeting


+https://llvm.org/devmtg/2007-05/;>
 
 

-   
-   2007 LLVM Developer's Meeting
-   Discussion about Clang at the http://llvm.org/devmtg/2007-05/;>2007 LLVM Developer's Meeting.
-   About:
-   In this video, Steve Naroff introduces the Clang project and 
talks about some of the goals and motivations for starting the project.
-   
-   Details: New LLVM C Front-end - This talk describes a 
new from-scratch C frontend (which is aiming to support Objective C and C++ 
someday) for LLVM, built as a native part of the LLVM system and in the LLVM 
design style.
-   The Presentation:
-   You can download a copy of the presentation in mov format.  
However, due to the picture quality, it is recommended that you also download 
the lecture slides for viewing while you watch the video.
-   
-   http://llvm.org/devmtg/2007-05/09-Naroff-CFE.mov;>Video (mov file)
-   http://llvm.org/devmtg/2007-05/09-Naroff-CFE.pdf;>Lecture slides (PDF)
-   
-   
+Redirecting to the new page
 
 

Modified: cfe/trunk/www/clang_video-07-25-2007.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/clang_video-07-25-2007.html?rev=342568=342567=342568=diff
==
--- cfe/trunk/www/clang_video-07-25-2007.html (original)
+++ cfe/trunk/www/clang_video-07-25-2007.html Wed Sep 19 11:39:15 2018
@@ -6,25 +6,10 @@
LLVM 2.0 and Beyond!


+https://llvm.org/devmtg/2007-05/;>
 
 

-   
-   LLVM 2.0 and Beyond!
-   A Google Techtalk by http://www.nondot.org/sabre/;>Chris Lattner
-   About:
-   In this video, Chris Lattner talks about some of the 
features of Clang, especially in regards to performance.
-   
-   Details: The LLVM 2.0 release brings a number of new 
features and capabilities to the LLVM toolset. This talk briefly describes 
those features, then moves on to talk about what is next: llvm 2.1, llvm-gcc 
4.2, and puts a special emphasis on the 'clang' C front-end. This describes how 
the 'clang' preprocessor can be used to improve the scalability of distcc by up 
to 4.4x.
-   The Presentation:
-   You can view the presentation through google video. In 
addition, the slides from the presentation are also available, if you wish to 
retain a copy.
-   
-   http://video.google.com/videoplay?docid=1921156852099786640;>Google Tech 
Talk Video (19:00-) (Note: the Clang lecture starts at 19 minutes into the 
video.)
-   http://llvm.org/pubs/2007-07-25-LLVM-2.0-and-Beyond.pdf;>LLVM 2.0 and 
Beyond! slides (PDF)
-   
-   Publishing Information:
-   "LLVM 2.0 and Beyond!", Chris Lattner,
-   Google Tech Talk, Mountain View, CA, July 2007.
-   
+Redirecting to the new page
 
 


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

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists closed this revision.
mclow.lists added a comment.

Landed as revision 342566


https://reviews.llvm.org/D44263



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

[PATCH] D52271: [Sema] Ensure that we retain __restrict qualifiers when substituting a reference type.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, rjmccall.
Herald added a subscriber: dexonsmith.

Fixes rdar://43760099. Thanks for taking a look!


Repository:
  rC Clang

https://reviews.llvm.org/D52271

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/subst-restrict.cpp


Index: clang/test/SemaCXX/subst-restrict.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  // Presumably, [dcl.fct]p7 would have applied to __restrict (and all the 
other
+  // qualifiers we support).
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+Quals.removeConst();
+Quals.removeVolatile();
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.


Index: clang/test/SemaCXX/subst-restrict.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/subst-restrict.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+template  struct add_restrict {
+  typedef T __restrict type;
+};
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(is_same::type>::value, "");
+static_assert(is_same::type>::value, "");
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4252,14 +4252,20 @@
   // C++ [dcl.fct]p7:
   //   [When] adding cv-qualifications on top of the function type [...] the
   //   cv-qualifiers are ignored.
+  // Presumably, [dcl.fct]p7 would have applied to __restrict (and all the other
+  // qualifiers we support).
+  if (T->isFunctionType())
+return T;
+
   // C++ [dcl.ref]p1:
   //   when the cv-qualifiers are introduced through the use of a typedef-name
   //   or decltype-specifier [...] the cv-qualifiers are ignored.
   // Note that [dcl.ref]p1 lists all cases in which cv-qualifiers can be
   // applied to a reference type.
-  // FIXME: This removes all qualifiers, not just cv-qualifiers!
-  if (T->isFunctionType() || T->isReferenceType())
-return T;
+  if (T->isReferenceType()) {
+Quals.removeConst();
+Quals.removeVolatile();
+  }
 
   // Suppress Objective-C lifetime qualifiers if they don't make sense for the
   // resulting type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added inline comments.



Comment at: test/support/verbose_assert.h:26
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };

mclow.lists wrote:
> > The renaming is to clarify that an `stderr` stream is being selected.
> And yet, there is `clog`, right there.
> 
I guess because that’s logging a programmer error (trying to print a type that 
isn’t streamable) whereas the others are test-related diagnostics? Not 
completely sure though.


https://reviews.llvm.org/D51868



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

r342565 - Sema: handle `wint_t` more carefully for printf checking

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Sep 19 11:13:34 2018
New Revision: 342565

URL: http://llvm.org/viewvc/llvm-project?rev=342565=rev
Log:
Sema: handle `wint_t` more carefully for printf checking

In the case that `win_t` is an `unsigned short` (e.g. on Windows), we would
previously incorrectly diagnose the conversion because we would immediately
promote the argument type from `wint_t` (aka `unsigned short`) to `int` before
checking if the type matched.  This should repair the Windows hosted bots.

Modified:
cfe/trunk/lib/Analysis/FormatString.cpp
cfe/trunk/test/Sema/format-strings-ms.c
cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/lib/Analysis/FormatString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/FormatString.cpp?rev=342565=342564=342565=diff
==
--- cfe/trunk/lib/Analysis/FormatString.cpp (original)
+++ cfe/trunk/lib/Analysis/FormatString.cpp Wed Sep 19 11:13:34 2018
@@ -406,12 +406,14 @@ ArgType::matchesType(ASTContext , Qual
 }
 
 case WIntTy: {
+  QualType WInt = C.getCanonicalType(C.getWIntType()).getUnqualifiedType();
 
-  QualType PromoArg =
-argTy->isPromotableIntegerType()
-  ? C.getPromotedIntegerType(argTy) : argTy;
+  if (C.getCanonicalType(argTy).getUnqualifiedType() == WInt)
+return Match;
 
-  QualType WInt = C.getCanonicalType(C.getWIntType()).getUnqualifiedType();
+  QualType PromoArg = argTy->isPromotableIntegerType()
+  ? C.getPromotedIntegerType(argTy)
+  : argTy;
   PromoArg = C.getCanonicalType(PromoArg).getUnqualifiedType();
 
   // If the promoted argument is the corresponding signed type of the

Modified: cfe/trunk/test/Sema/format-strings-ms.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-ms.c?rev=342565=342564=342565=diff
==
--- cfe/trunk/test/Sema/format-strings-ms.c (original)
+++ cfe/trunk/test/Sema/format-strings-ms.c Wed Sep 19 11:13:34 2018
@@ -13,7 +13,6 @@ void non_iso_warning_test(__int32 i32, _
   printf("%I32d", i32); // expected-warning{{'I32' length modifier is not 
supported by ISO C}}
   printf("%I64d", i64); // expected-warning{{'I64' length modifier is not 
supported by ISO C}}
   printf("%wc", c); // expected-warning{{'w' length modifier is not supported 
by ISO C}}
-  // expected-warning@-1{{format specifies type 'wint_t' (aka 'unsigned 
short') but the argument has type 'wchar_t' (aka 'unsigned short')}}
   printf("%Z", p); // expected-warning{{'Z' conversion specifier is not 
supported by ISO C}}
 }
 
@@ -36,7 +35,7 @@ void unsigned_test() {
 }
 
 void w_test(wchar_t c, wchar_t *s) {
-  printf("%wc", c); // expected-warning{{format specifies type 'wint_t' (aka 
'unsigned short') but the argument has type 'wchar_t' (aka 'unsigned short')}}
+  printf("%wc", c);
   printf("%wC", c);
   printf("%C", c);
   printf("%ws", s);

Modified: cfe/trunk/test/Sema/format-strings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=342565=342564=342565=diff
==
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Wed Sep 19 11:13:34 2018
@@ -401,7 +401,11 @@ void bug7377_bad_length_mod_usage() {
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
+#if __WINT_WIDTH__ == 4
   printf("%lc", (char) 1); // no-warning
+#else
+  printf("%lc", (char) 1); // expected-warning{{the argument has type 'char'}}
+#endif
   printf("%lc", ); // expected-warning{{the argument has type 'wint_t *'}}
   // If wint_t and wchar_t are the same width and wint_t is signed where
   // wchar_t is unsigned, an implicit conversion isn't possible.


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

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande updated this revision to Diff 166148.
elsteveogrande added a comment.

Change `Optional` to `unique_ptr` instead, to at least save an allocation and 
keep the same semantics.  Reverified with `ninja check-clang`.


Repository:
  rC Clang

https://reviews.llvm.org/D50948

Files:
  include/clang/AST/DeclCXX.h
  lib/AST/DeclCXX.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp

Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -6010,7 +6010,7 @@
 
 void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   auto  = D->data();
-  Record->push_back(Data.IsLambda);
+  Record->push_back(Data.hasLambdaData());  // lambda-specific data added below
   Record->push_back(Data.UserDeclaredConstructor);
   Record->push_back(Data.UserDeclaredSpecialMembers);
   Record->push_back(Data.Aggregate);
@@ -6068,8 +6068,6 @@
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
 
-  // IsLambda bit is already saved.
-
   Record->push_back(Data.NumBases);
   if (Data.NumBases > 0)
 AddCXXBaseSpecifiers(Data.bases());
@@ -6085,8 +6083,8 @@
   AddDeclRef(D->getFirstFriend());
 
   // Add lambda-specific data.
-  if (Data.IsLambda) {
-auto  = D->getLambdaData();
+  if (Data.hasLambdaData()) {
+auto  = *Data.LambdaData;
 Record->push_back(Lambda.Dependent);
 Record->push_back(Lambda.IsGenericLambda);
 Record->push_back(Lambda.CaptureDefault);
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1636,7 +1636,7 @@
 
 void ASTDeclReader::ReadCXXDefinitionData(
 struct CXXRecordDecl::DefinitionData , const CXXRecordDecl *D) {
-  // Note: the caller has deserialized the IsLambda bit already.
+  // Note: the caller has deserialized the bit for `isLambda()` already.
   Data.UserDeclaredConstructor = Record.readInt();
   Data.UserDeclaredSpecialMembers = Record.readInt();
   Data.Aggregate = Record.readInt();
@@ -1703,10 +1703,10 @@
   assert(Data.Definition && "Data.Definition should be already set!");
   Data.FirstFriend = ReadDeclID();
 
-  if (Data.IsLambda) {
+  if (Data.hasLambdaData()) {
 using Capture = LambdaCapture;
 
-auto  = static_cast(Data);
+auto  = *Data.LambdaData;
 Lambda.Dependent = Record.readInt();
 Lambda.IsGenericLambda = Record.readInt();
 Lambda.CaptureDefault = Record.readInt();
@@ -1761,7 +1761,8 @@
   PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) {
 // We faked up this definition data because we found a class for which we'd
 // not yet loaded the definition. Replace it with the real thing now.
-assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?");
+assert(!DD.hasLambdaData() && !MergeDD.hasLambdaData() &&
+   "faked up lambda definition?");
 PFDI->second = ASTReader::PendingFakeDefinitionKind::FakeLoaded;
 
 // Don't change which declaration is the definition; that is required
@@ -1826,7 +1827,6 @@
   MATCH_FIELD(ImplicitCopyAssignmentHasConstParam)
   OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
   OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
-  MATCH_FIELD(IsLambda)
 #undef OR_FIELD
 #undef MATCH_FIELD
 
@@ -1845,7 +1845,7 @@
   // FIXME: Issue a diagnostic if FirstFriend doesn't match when we come to
   // lazily load it.
 
-  if (DD.IsLambda) {
+  if (DD.hasLambdaData()) {
 // FIXME: ODR-checking for merging lambdas (this happens, for instance,
 // when they occur within the body of a function template specialization).
   }
@@ -1860,17 +1860,17 @@
 }
 
 void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) {
-  struct CXXRecordDecl::DefinitionData *DD;
   ASTContext  = Reader.getContext();
 
+  struct CXXRecordDecl::DefinitionData *DD =
+  new (C) struct CXXRecordDecl::DefinitionData(D);
+
   // Determine whether this is a lambda closure type, so that we can
-  // allocate the appropriate DefinitionData structure.
-  bool IsLambda = Record.readInt();
-  if (IsLambda)
-DD = new (C) CXXRecordDecl::LambdaDefinitionData(D, nullptr, false, false,
- LCD_None);
-  else
-DD = new (C) struct CXXRecordDecl::DefinitionData(D);
+  // set the LambdaDefinitionData (into optional \c LambdaData field) as well.
+  bool isLambda = Record.readInt();
+  if (isLambda) {
+DD->SetLambdaDefinitionData(nullptr, false, false, LCD_None);
+  }
 
   CXXRecordDecl *Canon = D->getCanonicalDecl();
   // Set decl definition data before reading it, so that during deserialization
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -102,7 +102,7 @@
   

r342562 - [analyzer] Fix nullptr access when processing instantiated function in ExprMutationAnalyzer.

[Shuai Wang via cfe-commits]

Author: shuaiwang
Date: Wed Sep 19 11:00:55 2018
New Revision: 342562

URL: http://llvm.org/viewvc/llvm-project?rev=342562=rev
Log:
[analyzer] Fix nullptr access when processing instantiated function in 
ExprMutationAnalyzer.

Modified:
cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Modified: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp?rev=342562=342561=342562=diff
==
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp (original)
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp Wed Sep 19 11:00:55 2018
@@ -379,7 +379,7 @@ const Stmt *ExprMutationAnalyzer::findFu
   for (const auto  : Matches) {
 const auto *Exp = Nodes.getNodeAs(NodeID::value);
 const auto *Func = Nodes.getNodeAs("func");
-if (!Func->getBody())
+if (!Func->getBody() || !Func->getPrimaryTemplate())
   return Exp;
 
 const auto *Parm = Nodes.getNodeAs("parm");

Modified: cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp?rev=342562=342561=342562=diff
==
--- cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp (original)
+++ cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp Wed Sep 19 
11:00:55 2018
@@ -215,6 +215,12 @@ TEST(ExprMutationAnalyzerTest, ByValueAr
  "void f() { A x, y; y = x; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = buildASTFromCode(
+  "template  struct A { A(); A(const A&); static void mf(A) {} };"
+  "void f() { A<0> x; A<0>::mf(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, ByConstValueArgument) {
@@ -241,6 +247,12 @@ TEST(ExprMutationAnalyzerTest, ByConstVa
   "void f() { struct A { A(const int); }; int x; A y(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = buildASTFromCode("template  struct A { A(); A(const A&);"
+ "static void mf(const A&) {} };"
+ "void f() { A<0> x; A<0>::mf(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, ByNonConstRefArgument) {
@@ -288,6 +300,12 @@ TEST(ExprMutationAnalyzerTest, ByNonCons
   AST = buildASTFromCode("void f() { struct A { A(); A(A&); }; A x; A y(x); 
}");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
+  AST = buildASTFromCode(
+  "template  struct A { A(); A(const A&); static void mf(A&) {} };"
+  "void f() { A<0> x; A<0>::mf(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("A<0>::mf(x)"));
 }
 
 TEST(ExprMutationAnalyzerTest, ByConstRefArgument) {
@@ -686,6 +704,12 @@ TEST(ExprMutationAnalyzerTest, FollowFun
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
 
+  AST = buildASTFromCode("template  struct S {"
+ "template  S(T&& t) : m(++t) { } U m; };"
+ "void f() { int x; S s(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
   AST = buildASTFromCode(StdRemoveReference + StdForward +
  "template  void u(Args&...);"
  "template  void h(Args&&... args)"
@@ -737,6 +761,12 @@ TEST(ExprMutationAnalyzerTest, FollowFun
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
+  AST = buildASTFromCode("template  struct S {"
+ "template  S(T&& t) : m(t) { } U m; };"
+ "void f() { int x; S s(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
   AST = buildASTFromCode(StdRemoveReference + StdForward +
  "template  void u(Args...);"
  "template  void h(Args&&... args)"


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

[PATCH] D50551: [libcxx] [test] Add missing to several tests.

[Marshall Clow via Phabricator via cfe-commits]

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

LFTM


https://reviews.llvm.org/D50551



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

[PATCH] D50799: Fix for PR 38495: no longer compiles on FreeBSD, due to lack of timespec_get()

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists closed this revision.
mclow.lists added a comment.

Landed as r339816


https://reviews.llvm.org/D50799



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

[PATCH] D50815: Establish the header

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists closed this revision.
mclow.lists added a comment.

Landed as r339943


https://reviews.llvm.org/D50815



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

[PATCH] D50876: Clean up newly created header

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists closed this revision.
mclow.lists added a comment.

Landed as r340049


https://reviews.llvm.org/D50876



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

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[Erich Keane via Phabricator via cfe-commits]

erichkeane added subscribers: gbiv, george.burgess.iv.
erichkeane added a comment.

In https://reviews.llvm.org/D52268#1239557, @riccibruno wrote:

> In https://reviews.llvm.org/D52268#1239538, @erichkeane wrote:
>
> > Does this still work with 32 bit hosts?  Does PointerIntPair have 3 bits in 
> > that case?  Is the alignof static_assert valid in that case?
>
>
> I think it does since Decl is manually over-aligned to 8 bytes. But you are 
> right that the static_assert is probably not needed
>  here since llvm::PointerIntPair checks that the low bits are available.
>
> > I'm not sure how often this ever gets modified, but it is a touch scary to 
> > me that we've already maxed out the size of this struct.
>
> I am not sure either... It was just a quick change I spotted and this is 
> borderline not worth doing. If you think that we ought to keep
>  some space left I will abandon this revision.


I did a little digging on this, and it seems to be to keep track of a 
declarations linkage for caching sake.  Unless the committees change linkage at 
all, I guess I don't see this changing much.  I think my concerns are assuaged.

That said, I'm adding @george.burgess.iv (or @gbiv ?) to the review, since he's 
the original author.


Repository:
  rC Clang

https://reviews.llvm.org/D52268



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

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: test/support/verbose_assert.h:26
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };

> The renaming is to clarify that an `stderr` stream is being selected.
And yet, there is `clog`, right there.



https://reviews.llvm.org/D51868



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

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In https://reviews.llvm.org/D52268#1239538, @erichkeane wrote:

> Does this still work with 32 bit hosts?  Does PointerIntPair have 3 bits in 
> that case?  Is the alignof static_assert valid in that case?


I think it does since Decl is manually over-aligned to 8 bytes. But you are 
right that the static_assert is probably not needed
here since llvm::PointerIntPair checks that the low bits are available.

> I'm not sure how often this ever gets modified, but it is a touch scary to me 
> that we've already maxed out the size of this struct.

I am not sure either... It was just a quick change I spotted and this is 
borderline not worth doing. If you think that we ought to keep
some space left I will abandon this revision.


Repository:
  rC Clang

https://reviews.llvm.org/D52268



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

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: include/clang/AST/DeclarationName.h:243
+  ///   C++ literal operator, or C++ using directive.
   uintptr_t Ptr = 0;
 

erichkeane wrote:
> There is an llvm type for storing something in the lower bits of a pointer.  
> I THINK it is llvm::PointerIntPair.
> 
> I'd MUCH prefer you do that instead of the reimplementation here.
Well it was already like this but point taken.



Comment at: lib/AST/DeclarationName.cpp:42
 
+void DeclarationName::VerifyStaticAsserts() {
+  llvm_unreachable("VerifyStaticAsserts is not meant to be called!");

erichkeane wrote:
> I'm a touch confused by this function.  It shouldn't be necessary when the 
> static asserts can all be put at the global/class level.
Yes, but the enumeration StoredNameKind is private to DeclarationName.
Therefore I cannot just put the static_asserts into a .cpp and be done with it.
An alternative is of course to put them in the class definition in the header 
but
look at this mess... (and this is after clang-format).


Repository:
  rC Clang

https://reviews.llvm.org/D52267



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

r342557 - Basic: correct `__WINT_TYPE__` on Windows

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Sep 19 09:18:55 2018
New Revision: 342557

URL: http://llvm.org/viewvc/llvm-project?rev=342557=rev
Log:
Basic: correct `__WINT_TYPE__` on Windows

Windows uses `unsigned short` for `wint_t`.  Correct the type definition as
vended by the compiler.  This type is defined in corecrt.h and is
unconditionally typedef'ed.  cl does not have an equivalent to `__WINT_TYPE__`
which is why this was never detected.

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/test/Preprocessor/init.c
cfe/trunk/test/Preprocessor/woa-defaults.c
cfe/trunk/test/Sema/format-strings-ms.c

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=342557=342556=342557=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Wed Sep 19 09:18:55 2018
@@ -643,6 +643,7 @@ public:
   WindowsTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
 this->WCharType = TargetInfo::UnsignedShort;
+this->WIntType = TargetInfo::UnsignedShort;
   }
 };
 

Modified: cfe/trunk/test/Preprocessor/init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=342557=342556=342557=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Wed Sep 19 09:18:55 2018
@@ -1553,7 +1553,7 @@
 // AARCH64-MSVC: #define __SIZEOF_SHORT__ 2
 // AARCH64-MSVC: #define __SIZEOF_SIZE_T__ 8
 // AARCH64-MSVC: #define __SIZEOF_WCHAR_T__ 2
-// AARCH64-MSVC: #define __SIZEOF_WINT_T__ 4
+// AARCH64-MSVC: #define __SIZEOF_WINT_T__ 2
 // AARCH64-MSVC: #define __SIZE_MAX__ 18446744073709551615ULL
 // AARCH64-MSVC: #define __SIZE_TYPE__ long long unsigned int
 // AARCH64-MSVC: #define __SIZE_WIDTH__ 64
@@ -1602,8 +1602,8 @@
 // AARCH64-MSVC: #define __WCHAR_TYPE__ unsigned short
 // AARCH64-MSVC: #define __WCHAR_UNSIGNED__ 1
 // AARCH64-MSVC: #define __WCHAR_WIDTH__ 16
-// AARCH64-MSVC: #define __WINT_TYPE__ int
-// AARCH64-MSVC: #define __WINT_WIDTH__ 32
+// AARCH64-MSVC: #define __WINT_TYPE__ unsigned short
+// AARCH64-MSVC: #define __WINT_WIDTH__ 16
 // AARCH64-MSVC: #define __aarch64__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=arm-none-none < /dev/null | 
FileCheck -match-full-lines -check-prefix ARM %s

Modified: cfe/trunk/test/Preprocessor/woa-defaults.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/woa-defaults.c?rev=342557=342556=342557=diff
==
--- cfe/trunk/test/Preprocessor/woa-defaults.c (original)
+++ cfe/trunk/test/Preprocessor/woa-defaults.c Wed Sep 19 09:18:55 2018
@@ -27,7 +27,7 @@
 // CHECK: #define __SIZEOF_SHORT__ 2
 // CHECK: #define __SIZEOF_SIZE_T__ 4
 // CHECK: #define __SIZEOF_WCHAR_T__ 2
-// CHECK: #define __SIZEOF_WINT_T__ 4
+// CHECK: #define __SIZEOF_WINT_T__ 2
 // CHECK: #define __SIZE_TYPE__ unsigned int
 // CHECK: #define __UINTPTR_TYPE__ unsigned int
 

Modified: cfe/trunk/test/Sema/format-strings-ms.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-ms.c?rev=342557=342556=342557=diff
==
--- cfe/trunk/test/Sema/format-strings-ms.c (original)
+++ cfe/trunk/test/Sema/format-strings-ms.c Wed Sep 19 09:18:55 2018
@@ -13,6 +13,7 @@ void non_iso_warning_test(__int32 i32, _
   printf("%I32d", i32); // expected-warning{{'I32' length modifier is not 
supported by ISO C}}
   printf("%I64d", i64); // expected-warning{{'I64' length modifier is not 
supported by ISO C}}
   printf("%wc", c); // expected-warning{{'w' length modifier is not supported 
by ISO C}}
+  // expected-warning@-1{{format specifies type 'wint_t' (aka 'unsigned 
short') but the argument has type 'wchar_t' (aka 'unsigned short')}}
   printf("%Z", p); // expected-warning{{'Z' conversion specifier is not 
supported by ISO C}}
 }
 
@@ -35,7 +36,7 @@ void unsigned_test() {
 }
 
 void w_test(wchar_t c, wchar_t *s) {
-  printf("%wc", c);
+  printf("%wc", c); // expected-warning{{format specifies type 'wint_t' (aka 
'unsigned short') but the argument has type 'wchar_t' (aka 'unsigned short')}}
   printf("%wC", c);
   printf("%C", c);
   printf("%ws", s);
@@ -49,7 +50,7 @@ void w_test(wchar_t c, wchar_t *s) {
   scanf("%S", s);
 
   double bad;
-  printf("%wc", bad); // expected-warning{{format specifies type 'wint_t' (aka 
'int') but the argument has type 'double'}}
+  printf("%wc", bad); // expected-warning{{format specifies type 'wint_t' (aka 
'unsigned short') but the argument has type 'double'}}
   printf("%wC", bad); // expected-warning{{format specifies type 'wchar_t' 
(aka 'unsigned short') but the argument has type 'double'}}
   printf("%C", bad); // 

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Does this still work with 32 bit hosts?  Does PointerIntPair have 3 bits in 
that case?  Is the alignof static_assert valid in that case?




Comment at: lib/AST/Linkage.h:40
 
+  enum { NumLVComputationKindBits = 3 };
+

I'm not sure how often this ever gets modified, but it is a touch scary to me 
that we've already maxed out the size of this struct.





Comment at: lib/AST/Linkage.h:89
   llvm::SmallDenseMap CachedLinkageInfo;
+  static_assert(alignof(NamedDecl) >= 8,
+"LinkageComputer assumes that NamedDecl is aligned to >= 8!");

Is this not guaranteed by the static-assert in pointerintpair?


Repository:
  rC Clang

https://reviews.llvm.org/D52268



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

> clang-cl isn't supposed to do (explicit) registry accesses when you hold it 
> right (pass in -fms-compatibility-version etc). Have you seen registry access 
> costs, or is that speculation?

No speculation, I will share the FileMon logs shortly. It is indirectly 
dependent DLLs that access the registry, not clang-cl itself.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Since it helps existing msbuild configs, adding this seems like a good thing to 
me.

clang-cl isn't supposed to do (explicit) registry accesses when you hold it 
right (pass in -fms-compatibility-version etc). Have you seen registry access 
costs, or is that speculation?

Re fastbuild: We use ninja with 
https://chromium.googlesource.com/infra/goma/client/ as distributed build 
system, which gives use the same build system on all platforms, -j1000 builds, 
and we can build 40K translation units in a few minutes. We're pretty happy 
with our setup :-)


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

A few drive-by comments... This is a patch with interactions I'm not sure I 
feel comfortable approving myself, so I'll count on @rjmccall to do so.




Comment at: include/clang/AST/DeclarationName.h:243
+  ///   C++ literal operator, or C++ using directive.
   uintptr_t Ptr = 0;
 

There is an llvm type for storing something in the lower bits of a pointer.  I 
THINK it is llvm::PointerIntPair.

I'd MUCH prefer you do that instead of the reimplementation here.



Comment at: lib/AST/DeclarationName.cpp:42
 
+void DeclarationName::VerifyStaticAsserts() {
+  llvm_unreachable("VerifyStaticAsserts is not meant to be called!");

I'm a touch confused by this function.  It shouldn't be necessary when the 
static asserts can all be put at the global/class level.


Repository:
  rC Clang

https://reviews.llvm.org/D52267



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

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

[Taewook Oh via Phabricator via cfe-commits]

twoh added a comment.

ping!


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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

[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: erichkeane, rjmccall.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

Since Decls are already aligned explicitly to 8 bytes, stash the
3 bits representing an LVComputationKind into the lower 3 bits
of the NamedDecl * in LinkageComputer.


Repository:
  rC Clang

https://reviews.llvm.org/D52268

Files:
  lib/AST/Linkage.h


Index: lib/AST/Linkage.h
===
--- lib/AST/Linkage.h
+++ lib/AST/Linkage.h
@@ -20,6 +20,7 @@
 #include "clang/AST/Type.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 
 namespace clang {
 /// Kinds of LV computation.  The linkage side of the computation is
@@ -36,6 +37,8 @@
   /// in computing linkage.
   unsigned IgnoreAllVisibility : 1;
 
+  enum { NumLVComputationKindBits = 3 };
+
   explicit LVComputationKind(NamedDecl::ExplicitVisibilityKind EK)
   : ExplicitKind(EK), IgnoreExplicitVisibility(false),
 IgnoreAllVisibility(false) {}
@@ -78,12 +81,16 @@
   // using C = Foo;
   // using D = Foo;
   //
-  // The unsigned represents an LVComputationKind.
-  using QueryType = std::pair;
+  // The integer represents an LVComputationKind.
+  using QueryType =
+  llvm::PointerIntPair;
   llvm::SmallDenseMap CachedLinkageInfo;
+  static_assert(alignof(NamedDecl) >= 8,
+"LinkageComputer assumes that NamedDecl is aligned to >= 8!");
 
   static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
   }
 
   llvm::Optional lookup(const NamedDecl *ND,


Index: lib/AST/Linkage.h
===
--- lib/AST/Linkage.h
+++ lib/AST/Linkage.h
@@ -20,6 +20,7 @@
 #include "clang/AST/Type.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 
 namespace clang {
 /// Kinds of LV computation.  The linkage side of the computation is
@@ -36,6 +37,8 @@
   /// in computing linkage.
   unsigned IgnoreAllVisibility : 1;
 
+  enum { NumLVComputationKindBits = 3 };
+
   explicit LVComputationKind(NamedDecl::ExplicitVisibilityKind EK)
   : ExplicitKind(EK), IgnoreExplicitVisibility(false),
 IgnoreAllVisibility(false) {}
@@ -78,12 +81,16 @@
   // using C = Foo;
   // using D = Foo;
   //
-  // The unsigned represents an LVComputationKind.
-  using QueryType = std::pair;
+  // The integer represents an LVComputationKind.
+  using QueryType =
+  llvm::PointerIntPair;
   llvm::SmallDenseMap CachedLinkageInfo;
+  static_assert(alignof(NamedDecl) >= 8,
+"LinkageComputer assumes that NamedDecl is aligned to >= 8!");
 
   static QueryType makeCacheKey(const NamedDecl *ND, LVComputationKind Kind) {
-return std::make_pair(ND, Kind.toBits());
+return QueryType(ND, Kind.toBits());
   }
 
   llvm::Optional lookup(const NamedDecl *ND,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52267: [AST] Various optimizations + refactoring in DeclarationName(Table)

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: erichkeane, rjmccall.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith, mehdi_amini.

Introduce the following optimizations in DeclarationName(Table)

1. Store common kinds inline in DeclarationName instead of 
DeclarationNameExtra. Currently the kind of C++ constructor, destructor, 
conversion function and overloaded operator names is stored in 
DeclarationNameExtra. Instead store it inline in DeclarationName. To do this 
align IdentifierInfo, CXXSpecialName, DeclarationNameExtra and 
CXXOperatorIdName to 8 bytes so that we can use the lower 3 bits of 
DeclarationName::Ptr. This is already the case on 64 bits archs anyway. This 
also allow us to remove DeclarationNameExtra from CXXSpecialName and 
CXXOperatorIdName, which shave off a pointer from CXXSpecialName. (This might 
be the thing that sink this patch since you might judge that aligning 
IdentifierInfo to 8 bytes even on 32 bits archs is not acceptable. Although if 
we care about 32 bits archs there is probably a lot more that can be done.)

2. Synchronize the enumerations DeclarationName::NameKind, 
DeclarationName::StoredNameKind and Selector::IdentifierInfoFlag. This makes 
DeclarationName::getNameKind much more efficient since we can replace the 
switch table by a single comparison and an addition. static_asserts are 
provided in DeclarationName::VerifyStaticAsserts to ensure that the 
enumerations remain in sync.

3. Put the overloaded operator names inline in DeclarationNameTable to remove 
an indirection. This increase the size of DeclarationNameTable a little bit but 
this is not important since it is only used in ASTContext, and never copied nor 
moved from. This also get rid of the last dynamic allocation in 
DeclarationNameTable.

Altogether these optimizations cut the run time of parsing all of Boost by 
about 0.8%.
While we are at it, do the following NFC modifications:

1. Put the internal classes CXXSpecialName, CXXDeductionGuideNameExtra, 
CXXOperatorIdName, CXXLiteralOperatorIdName and DeclarationNameExtra in a 
namespace detail since these classes are only meant to be used by 
DeclarationName and DeclarationNameTable. Make this more explicit by making the 
members of these classes private and friending DeclarationName(Table).

2. Make DeclarationName::getFETokenInfo a non-template since every users are 
using it to get a void *. It was supposed to be used with a type to avoid a 
subsequent static_cast.

3. Change the internal functions DeclarationName::getAs* to castAs* since when 
we use them we already know the correct kind. This has no external impact since 
all of these are private.


Repository:
  rC Clang

https://reviews.llvm.org/D52267

Files:
  include/clang/AST/DeclarationName.h
  include/clang/Basic/IdentifierTable.h
  lib/AST/DeclarationName.cpp
  lib/Basic/IdentifierTable.cpp
  lib/Sema/IdentifierResolver.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp

Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3574,7 +3574,7 @@
 II->isPoisoned() ||
 (IsModule ? II->hasRevertedBuiltin() : II->getObjCOrBuiltinID()) ||
 II->hasRevertedTokenIDToIdentifier() ||
-(NeedDecls && II->getFETokenInfo()))
+(NeedDecls && II->getFETokenInfo()))
   return true;
 
 return false;
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -908,7 +908,7 @@
  (IsModule ? II.hasRevertedBuiltin() : II.getObjCOrBuiltinID()) ||
  II.hasRevertedTokenIDToIdentifier() ||
  (!(IsModule && Reader.getPreprocessor().getLangOpts().CPlusPlus) &&
-  II.getFETokenInfo());
+  II.getFETokenInfo());
 }
 
 static bool readBit(unsigned ) {
Index: lib/Sema/IdentifierResolver.cpp
===
--- lib/Sema/IdentifierResolver.cpp
+++ lib/Sema/IdentifierResolver.cpp
@@ -147,7 +147,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 updatingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   if (!Ptr) {
 Name.setFETokenInfo(D);
@@ -172,7 +172,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 updatingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   if (!Ptr) {
 AddDecl(D);
@@ -213,7 +213,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 updatingIdentifier(*II);
 
-  void *Ptr = Name.getFETokenInfo();
+  void *Ptr = Name.getFETokenInfo();
 
   assert(Ptr && "Didn't find this decl on its identifier's chain!");
 
@@ -232,7 +232,7 @@
   if (IdentifierInfo *II = Name.getAsIdentifierInfo())
 

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

FWIW the recommendation against /Ox in my version is because of 
https://github.com/ulfjack/ryu/pull/70#issuecomment-412168459


https://reviews.llvm.org/D52266



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

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: include/clang/Driver/CLCompatOptions.td:129
+def _SLASH_Oy : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, 
HelpText<"Enable frame pointer omission (x86 only)">;
+def _SLASH_Oy_ : CLFlag<"Oy-">, Alias<_SLASH_O>, AliasArgs<["y-"]>, 
HelpText<"Disable frame pointer omission (x86 only)">;
+

I took a stab at this myself for a bit, and did a few things differently (but 
most the same way).

Here's how they /? output looks in my local binary:

```
  /O1 Minimize size, equivalent to /Og /Os /Oy /Ob2 /Gs /GF 
/Gy
  /O2 Maximize speed, equivalent to /Og /Oi /Ot /Oy /Ob2 
/Gs /GF /Gy
  /Ob0Disable function inlining
  /Ob1Only inline functions explicitly or implicitly marked 
inline
  /Ob2Allow compiler to inline all functions it deems 
beneficial
  /Od Disable optimization
  /Og No effect
  /Oi-Disable use of builtin functions
  /Oi Enable use of builtin functions
  /Os Optimize for size
  /Ot Optimize for speed
  /Ox Deprecated, use O2 instead; equivalent to /Og /Oi /Ot 
/Oy /Ob2
  /Oy-Don't omit frame pointers (default)
  /Oy Omit frame pointers (x86 only)
  /O   Set several /O flags at once; '/O2y-' is the same as 
'/O2 /y-' for example
```

Since we reference them now, I also added these:

```
  /GF Enable string pooling (default)
...
  /Gs Same as /Gs0
  /Gs  Set stack probe size (default 4096)
...
  /Gy-Don't put each function in its own section (default)
  /Gy Put each function in its own section
```

As far as I can tell we do not make /O1 and /O2 imply /Gs -- we keep it at the 
default value of 4096 (?) Fixing this probably means increasing chrome's size 
(?).

cl.exe doesn't support O0; I wonder why rnk added it. I asked on the revision 
that added it.

The name after def isn't needed; maybe we should omit the names of flags we 
don't reference?

Here's my local diff, mix and match as you see fit:

```
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td (revision 342334)
+++ include/clang/Driver/CLCompatOptions.td (working copy)
@@ -85,16 +85,21 @@
   HelpText<"Assume thread-local variables are defined in the executable">;
 def _SLASH_GR : CLFlag<"GR">, HelpText<"Enable emission of RTTI data">;
 def _SLASH_GR_ : CLFlag<"GR-">, HelpText<"Disable emission of RTTI data">;
+def _SLASH_GF : CLIgnoredFlag<"GF">,
+  HelpText<"Enable string pooling (default)">;
 def _SLASH_GF_ : CLFlag<"GF-">, HelpText<"Disable string pooling">,
   Alias;
-def _SLASH_GS : CLFlag<"GS">, HelpText<"Enable buffer security check">;
+def _SLASH_GS : CLFlag<"GS">,
+  HelpText<"Enable buffer security check (default)">;
 def _SLASH_GS_ : CLFlag<"GS-">, HelpText<"Disable buffer security check">;
-def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size">,
+def: CLFlag<"Gs">, HelpText<"Same as /Gs0">,
+  Alias, AliasArgs<["0"]>;
+def _SLASH_Gs : CLJoined<"Gs">, HelpText<"Set stack probe size (default 
4096)">,
   Alias;
 def _SLASH_Gy : CLFlag<"Gy">, HelpText<"Put each function in its own section">,
   Alias;
 def _SLASH_Gy_ : CLFlag<"Gy-">,
-  HelpText<"Don't put each function in its own section">,
+  HelpText<"Don't put each function in its own section (default)">,
   Alias;
 def _SLASH_Gw : CLFlag<"Gw">, HelpText<"Put each data item in its own 
section">,
   Alias;
@@ -109,18 +114,50 @@
   Alias;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
   Alias;
+
+// FIXME: cl.exe doesn't understand this, not sure why clang-cl does.
 def _SLASH_O0 : CLFlag<"O0">, Alias;
-// /Oy- is handled by the /O option because /Oy- only has an effect on 32-bit.
-def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">;
-def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">, Alias;
-def _SLASH_Oi : CLFlag<"Oi">, HelpText<"Enable use of builtin functions">,
-  Alias;
-def _SLASH_Oi_ : CLFlag<"Oi-">, HelpText<"Disable use of builtin functions">,
-  Alias;
-def _SLASH_Os : CLFlag<"Os">, HelpText<"Optimize for size">, Alias,
-  AliasArgs<["s"]>;
-def _SLASH_Ot : CLFlag<"Ot">, HelpText<"Optimize for speed">, Alias,
-  AliasArgs<["2"]>;
+
+def _SLASH_O : CLJoined<"O">,
+  HelpText<"Set several /O flags at once; "
+   "'/O2y-' is the same as '/O2 /y-' for example">;
+
+// FIXME: Looks like this doesn't actually imply /Gs in clang-cl yet.
+def _SLASH_O1 : CLFlag<"O1">,
+  HelpText<"Minimize size, equivalent to /Og /Os /Oy /Ob2 /Gs /GF /Gy">,
+  Alias<_SLASH_O>, AliasArgs<["1"]>;
+def _SLASH_O2 : CLFlag<"O2">,
+  HelpText<"Maximize speed, equivalent to 

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: test/Driver/Xarch.c:5
+// RUN: not grep ' "-O3" ' %t.log
+// RUN: grep "argument unused during compilation: '-Xarch_i386 -O3'" %t.log
 // RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 
-S %s -S -Xarch_i386 -o 2> %t.log

I know that this isn’t your doing, but while you’re here, would you mind 
replacing the temp files with pipes and grep with FileCheck?


https://reviews.llvm.org/D52266



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

[PATCH] D50949: lambdas in modules: handle lambdas in .pcm [de]serialization

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande updated this revision to Diff 166130.
elsteveogrande added a comment.

rebase atop https://reviews.llvm.org/D50948 (correctly this time) to exclude 
those changes


Repository:
  rC Clang

https://reviews.llvm.org/D50949

Files:
  include/clang/AST/LambdaCapture.h
  lib/Serialization/ASTReaderDecl.cpp
  test/Modules/merge-lambdas.cpp

Index: test/Modules/merge-lambdas.cpp
===
--- test/Modules/merge-lambdas.cpp
+++ test/Modules/merge-lambdas.cpp
@@ -49,3 +49,25 @@
 
 #pragma clang module import A
 void (*p)() = f();
+
+#pragma clang module build PR38531
+module PR38531 {}
+#pragma clang module contents
+#pragma clang module begin PR38531
+template 
+struct S {};
+struct Fun {
+  template ())>
+  Fun(F) {}
+};
+template 
+void foo(Fun=[]{}) {}
+struct T {
+  void func() {
+foo();
+  }
+};
+#pragma clang module end
+#pragma clang module endbuild
+#pragma clang module import PR38531
+S s;
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1761,8 +1761,6 @@
   PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) {
 // We faked up this definition data because we found a class for which we'd
 // not yet loaded the definition. Replace it with the real thing now.
-assert(!DD.hasLambdaData() && !MergeDD.hasLambdaData() &&
-   "faked up lambda definition?");
 PFDI->second = ASTReader::PendingFakeDefinitionKind::FakeLoaded;
 
 // Don't change which declaration is the definition; that is required
@@ -1845,9 +1843,41 @@
   // FIXME: Issue a diagnostic if FirstFriend doesn't match when we come to
   // lazily load it.
 
-  if (DD.hasLambdaData()) {
-// FIXME: ODR-checking for merging lambdas (this happens, for instance,
-// when they occur within the body of a function template specialization).
+  assert (DD.hasLambdaData() == MergeDD.hasLambdaData() &&
+  "Merging definitions, one of which is a lambda, the other not?");
+  if (DD.hasLambdaData() && MergeDD.hasLambdaData()) {
+auto  = *DD.LambdaData;
+auto  = *MergeDD.LambdaData;
+
+#define MATCH_LAM_FIELD(Field) \
+DetectedOdrViolation |= LDD.Field != MergeLDD.Field;
+MATCH_LAM_FIELD(Dependent)
+MATCH_LAM_FIELD(IsGenericLambda)
+MATCH_LAM_FIELD(CaptureDefault)
+MATCH_LAM_FIELD(NumCaptures)
+MATCH_LAM_FIELD(NumExplicitCaptures)
+MATCH_LAM_FIELD(ManglingNumber)
+#undef MATCH_LAM_FIELD
+
+auto *C1 = LDD.Captures;
+auto *C2 = MergeLDD.Captures;
+for (int I = 0; !DetectedOdrViolation && I < LDD.NumCaptures; ++I) {
+  if (C1[I] != C2[I]) {
+DetectedOdrViolation = true;
+  }
+}
+
+if (LDD.MethodTyInfo || MergeLDD.MethodTyInfo) {
+  if (!LDD.MethodTyInfo ||
+  !MergeLDD.MethodTyInfo ||
+  (LDD.MethodTyInfo->getType().getTypePtrOrNull() !=
+  MergeLDD.MethodTyInfo->getType().getTypePtrOrNull())) {
+DetectedOdrViolation = true;
+  }
+}
+
+// FIXME: Issue a diagnostic if ContextDecl doesn't match when we come to
+// lazily load it.
   }
 
   if (D->getODRHash() != MergeDD.ODRHash) {
Index: include/clang/AST/LambdaCapture.h
===
--- include/clang/AST/LambdaCapture.h
+++ include/clang/AST/LambdaCapture.h
@@ -135,6 +135,16 @@
 assert(isPackExpansion() && "No ellipsis location for a non-expansion");
 return EllipsisLoc;
   }
+
+  // [In]Equality operators -- primarily for ODR-compliance checking.
+
+  bool operator==(LambdaCapture const ) const {
+return DeclAndBits == RHS.DeclAndBits;
+  }
+
+  bool operator!=(LambdaCapture const ) const {
+return !operator==(RHS);
+  }
 };
 
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52266: [clang-cl] Provide separate flags for all the /O variants

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added reviewers: thakis, rnk.

This provides better help text in "clang-cl /?".

Also it cleans things up a bit: previously "/Od" could be handled either as a 
separate flag aliased to "-O0", or by the main optimization flag processing in 
TranslateOptArg. With this patch, all the flags get aliased back to /O so 
they're handled by TranslateOptArg.

Thanks to Nico for the idea!


https://reviews.llvm.org/D52266

Files:
  include/clang/Driver/CLCompatOptions.td
  lib/Driver/ToolChains/MSVC.cpp
  test/Driver/Xarch.c


Index: test/Driver/Xarch.c
===
--- test/Driver/Xarch.c
+++ test/Driver/Xarch.c
@@ -1,8 +1,8 @@
-// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O2 %s -S -### 2> 
%t.log
-// RUN: grep ' "-O2" ' %t.log | count 1
-// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O2 %s -S -### 2> 
%t.log
-// RUN: not grep ' "-O2" ' %t.log
-// RUN: grep "argument unused during compilation: '-Xarch_i386 -O2'" %t.log
+// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O3 %s -S -### 2> 
%t.log
+// RUN: grep ' "-O3" ' %t.log | count 1
+// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O3 %s -S -### 2> 
%t.log
+// RUN: not grep ' "-O3" ' %t.log
+// RUN: grep "argument unused during compilation: '-Xarch_i386 -O3'" %t.log
 // RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 
-S %s -S -Xarch_i386 -o 2> %t.log
 // RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -o'" %t.log | count 2
 // RUN: grep "error: invalid Xarch argument: '-Xarch_i386 -S'" %t.log
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1378,6 +1378,7 @@
   }
   break;
 case 'g':
+  A->claim();
   break;
 case 'i':
   if (I + 1 != E && OptStr[I + 1] == '-') {
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -109,18 +109,25 @@
   Alias;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
   Alias;
-def _SLASH_O0 : CLFlag<"O0">, Alias;
-// /Oy- is handled by the /O option because /Oy- only has an effect on 32-bit.
-def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">;
-def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">, Alias;
-def _SLASH_Oi : CLFlag<"Oi">, HelpText<"Enable use of builtin functions">,
-  Alias;
-def _SLASH_Oi_ : CLFlag<"Oi-">, HelpText<"Disable use of builtin functions">,
-  Alias;
-def _SLASH_Os : CLFlag<"Os">, HelpText<"Optimize for size">, Alias,
-  AliasArgs<["s"]>;
-def _SLASH_Ot : CLFlag<"Ot">, HelpText<"Optimize for speed">, Alias,
-  AliasArgs<["2"]>;
+
+// The _SLASH_O option handles all the /O variants, but we also provide 
separate aliased options to provide separate help messages.
+def _SLASH_O : CLJoined<"O">, HelpText<"Set the optimization level">, 
MetaVarName<"[0|1|2|b{0123}|d|g|i[-]|s|t|x|y[-]]">;
+def _SLASH_O0 : CLFlag<"O0">, Alias, HelpText<"Disable optimization">;
+def _SLASH_O1 : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>, 
HelpText<"Optimize for small size (equivalent to /Og /Os /Oy /Ob2 /Gs /GF 
/Gy)">;
+def _SLASH_O2 : CLFlag<"O2">, Alias<_SLASH_O>, AliasArgs<["2"]>, 
HelpText<"Optimize for maximum speed (equivalent to /Og /Oi /Ot /Oy /Ob2 /Gs 
/GF /Gy)">;
+def _SLASH_Ob0 : CLFlag<"Ob0">, Alias<_SLASH_O>, AliasArgs<["b0"]>, 
HelpText<"Disable inlining">;
+def _SLASH_Ob1 : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>, 
HelpText<"Inline functions which are (explicitly or implicitly) marked inline">;
+def _SLASH_Ob2 : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>, 
HelpText<"Inline suitable functions">;
+def _SLASH_Od : CLFlag<"Od">, Alias, HelpText<"Disable optimization">;
+def _SLASH_Og : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>;
+def _SLASH_Oi : CLFlag<"Oi">, Alias<_SLASH_O>, AliasArgs<["i"]>, 
HelpText<"Optimize using builtin knowledge of functions">;
+def _SLASH_Oi_ : CLFlag<"Oi-">, Alias<_SLASH_O>, AliasArgs<["i-"]>, 
HelpText<"Disable optimization based on builtin knowledge of functions">;
+def _SLASH_Os : CLFlag<"Os">, Alias<_SLASH_O>, AliasArgs<["s"]>, 
HelpText<"Optimize for small size over speed">;
+def _SLASH_Ot : CLFlag<"Ot">, Alias<_SLASH_O>, AliasArgs<["t"]>, 
HelpText<"Optimize for speed over small size">;
+def _SLASH_Ox : CLFlag<"Ox">, Alias<_SLASH_O>, AliasArgs<["x"]>, 
HelpText<"Optimize maximum speed (equivalent to /Og /Oi /Ot /Oy /Ob2)">;
+def _SLASH_Oy : CLFlag<"Oy">, Alias<_SLASH_O>, AliasArgs<["y"]>, 
HelpText<"Enable frame pointer omission (x86 only)">;
+def _SLASH_Oy_ : CLFlag<"Oy-">, Alias<_SLASH_O>, AliasArgs<["y-"]>, 
HelpText<"Disable frame pointer omission (x86 only)">;
+
 def _SLASH_QUESTION : CLFlag<"?">, Alias,
   HelpText<"Display available options">;
 def 

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Hamza Sood via Phabricator via cfe-commits]

hamzasood added inline comments.



Comment at: include/filesystem:1396
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);

compnerd wrote:
> This possibly changes the meaning on other targets.  What was the error that 
> this triggered?
I've re-uploaded the patch with full context to make this clearer.

That's a member function on an exported type, which I don't think should have 
any visibility attributes. The specific issue in this case is that both the 
class type and the member function are marked as dllimport, which causes a 
compilation error.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

STL_MSFT wrote:
> compnerd wrote:
> > I think that the condition here is inverted, and should be enabled for 
> > MinGW32.
> What error prompted this? It hasn't caused problems when running libc++'s 
> tests against MSVC's STL (with both C1XX and Clang).
The comment above this says that it's copied from `__config`, but they must've 
gone out of sync at some point because `__config` doesn't have that extra 
section that I deleted.

This causes lots of errors when testing libc++. E.g. libc++ isn't declaring 
`std::timespec` on Windows because `_LIBCPP_HAS_C11_FEATURES` isn't defined, 
but the tests think that it's available so they try to use it (which causes 
compilation failures in lots of tests).

The better solution here might be to update `__config` to match this, but I'm 
not familiar with some of those platforms so this seemed like the safest 
approach for now.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

hamzasood wrote:
> STL_MSFT wrote:
> > compnerd wrote:
> > > I think that the condition here is inverted, and should be enabled for 
> > > MinGW32.
> > What error prompted this? It hasn't caused problems when running libc++'s 
> > tests against MSVC's STL (with both C1XX and Clang).
> The comment above this says that it's copied from `__config`, but they 
> must've gone out of sync at some point because `__config` doesn't have that 
> extra section that I deleted.
> 
> This causes lots of errors when testing libc++. E.g. libc++ isn't declaring 
> `std::timespec` on Windows because `_LIBCPP_HAS_C11_FEATURES` isn't defined, 
> but the tests think that it's available so they try to use it (which causes 
> compilation failures in lots of tests).
> 
> The better solution here might be to update `__config` to match this, but I'm 
> not familiar with some of those platforms so this seemed like the safest 
> approach for now.
This is a workaround for a libc++ issue rather than an MSVC one. See the 
response to @compnerd for the full details.



Comment at: test/support/verbose_assert.h:24
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");

mclow.lists wrote:
> Why the renaming here? 
> 
> What's the deal with sometimes using `clog` and sometimes `cerr`?
> (I know, you didn't do this, but ???)
> 
Some of the casting that goes on in that template triggers an error under 
`-fno-ms-extensions` when given a function pointer (in this case: `std::endl`). 
Selecting between a standard/wide stream isn't needed for `std::endl` as it can 
go to either, so I've bypassed the selection by sending it straight to `cerr`.

The renaming is to clarify that an `stderr` stream (and nothing else) is being 
selected, which sort of justifies going directly to `cerr`.


https://reviews.llvm.org/D51868



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

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 166124.
hamzasood added a comment.
Herald added a subscriber: christof.

I've added a fix for another related issue: the tests would fail to link if 
libc++ is built only as a DLL.

The auto-linking doesn't look for a dynamic library unless `_DLL` is defined 
(which I think is done by passing `/MD` to CL), but the test runner doesn't use 
CL-style commands. This could either be solved by either manually defining that 
macro or just disabling auto-linking. The test runner already configures the 
linker correctly so disabling auto-linking seemed like the simplest option.


https://reviews.llvm.org/D51868

Files:
  CMakeLists.txt
  include/filesystem
  test/std/strings/c.strings/cuchar.pass.cpp
  test/support/test_macros.h
  test/support/verbose_assert.h
  utils/libcxx/test/config.py

Index: utils/libcxx/test/config.py
===
--- utils/libcxx/test/config.py
+++ utils/libcxx/test/config.py
@@ -515,6 +515,10 @@
 # and so that those tests don't have to be changed to tolerate
 # this insanity.
 self.cxx.compile_flags += ['-DNOMINMAX']
+# Disable auto-linking; the library is linked manually when
+# configuring the linker flags.
+if self.cxx_stdlib_under_test == 'libc++':
+self.cxx.compile_flags += ['-D_LIBCPP_NO_AUTO_LINK']
 additional_flags = self.get_lit_conf('test_compiler_flags')
 if additional_flags:
 self.cxx.compile_flags += shlex.split(additional_flags)
Index: test/support/verbose_assert.h
===
--- test/support/verbose_assert.h
+++ test/support/verbose_assert.h
@@ -21,18 +21,18 @@
 
 template ::value ? 1
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");
   static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::cerr << val; }
 };
 
 template 
-struct SelectStream {
+struct SelectErrStream {
   static void Print(Tp const& val) { std::wcerr << val; }
 };
 
@@ -86,14 +86,14 @@
 template 
 friend LogType& operator<<(LogType& log, Tp const& value) {
   if (!log.is_disabled) {
-SelectStream::Print(value);
+SelectErrStream::Print(value);
   }
   return log;
 }
 
 friend LogType& operator<<(LogType& log, EndLType* m) {
   if (!log.is_disabled) {
-SelectStream::Print(m);
+std::cerr << m;
   }
   return log;
 }
Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -143,11 +143,6 @@
 #  define TEST_HAS_C11_FEATURES
 #  define TEST_HAS_TIMESPEC_GET
 #endif
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library
-#  define TEST_HAS_TIMESPEC_GET
-#endif
 #  endif
 #endif
 
Index: test/std/strings/c.strings/cuchar.pass.cpp
===
--- test/std/strings/c.strings/cuchar.pass.cpp
+++ test/std/strings/c.strings/cuchar.pass.cpp
@@ -8,6 +8,9 @@
 //===--===//
 //
 // XFAIL: *
+//
+// The MSVC stdlib has cuchar, which causes this test to pass unexpectedly.
+// UNSUPPORTED: windows
 
 // 
 
Index: include/filesystem
===
--- include/filesystem
+++ include/filesystem
@@ -1390,7 +1390,6 @@
 return __storage_->__what_.c_str();
   }
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);
 
 private:
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -67,7 +67,7 @@
 option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
-option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" ON)
+option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build libc++experimental.a" $)
 set(ENABLE_FILESYSTEM_DEFAULT ${LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY})
 if (WIN32)
   set(ENABLE_FILESYSTEM_DEFAULT OFF)
@@ -542,8 +542,13 @@
 
 # Warning flags ===
 add_definitions(-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+if (MSVC)
+add_compile_flags(/W4)
+else()
+add_compile_flags_if_supported(-Wall)
+endif()
 add_compile_flags_if_supported(
--Wall -Wextra -W -Wwrite-strings
+-Wextra -W -Wwrite-strings
 -Wno-unused-parameter -Wno-long-long
 

[PATCH] D50214: Add inherited attributes before parsed attributes.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D50214



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In https://reviews.llvm.org/D52193#1239171, @hans wrote:

> Thanks for adding the Ninja numbers. It confirms that Ninja is significantly 
> faster than MSBuild + /MP.


It is true for the 6-cores Xeon, but not for the dual-18-cores. I think there 
are two issues there, somehow unrelated to /MP: 1. Invoking the child clang-cl 
-cc1 for **each** cpp file makes things much slower. I’ve clocked the 
invocation at about 60-100ms (which is caused mainly by loading dlls & 
registery accesses). Most likely Reid’s change about delay loading dlls should 
improve that. 2. The other issue is, I think, the process context switching at 
the OS-level. This link:  
https://www.phoronix.com/scan.php?page=article=2990wx-linux-windows=4 
- shows that multi-threading is significantly faster on a Linux machine, as far 
as high cores count goes (when compared with the same test ran on the same 
machine under Windows 10).

> Since that's the case, maybe we should be poking MS about making MSBuild 
> faster instead of adding /MP support to Clang? Or making it easier to use 
> Ninja in VS projects? Your patch says RFC after all :-)

Sad for Microsoft, but at this point it is a _design_ change MSBuild needs. And 
as a PM I would rather invest in bindings to better build systems (Ninja, 
Fastbuild). However I expect there are still users of MSBuild out there, and 
without /MP this means essentially that migrating to clang-cl requires also 
changing their build system and their VS2017 integration with the said build 
system.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52264: Deduplicate replacements from diagnostics.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: bkramer.
Herald added a subscriber: cfe-commits.

After r329813, clang-apply-replacements stopped deduplicating identical
replacements; however, tools like clang-tidy relies on the deduplication of
identical dignostics replacements from different TUs to apply fixes correctly.

This change partially roll back the behavior by deduplicating changes from
diagnostics. Ideally, we should deduplicate on diagnostics level, but we need to
figure out an effecient way.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52264

Files:
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  test/clang-apply-replacements/Inputs/identical/file1.yaml
  test/clang-apply-replacements/Inputs/identical/file2.yaml
  test/clang-apply-replacements/Inputs/identical/identical.cpp
  test/clang-apply-replacements/identical.cpp


Index: test/clang-apply-replacements/identical.cpp
===
--- test/clang-apply-replacements/identical.cpp
+++ test/clang-apply-replacements/identical.cpp
@@ -1,5 +1,6 @@
 // RUN: mkdir -p %T/Inputs/identical
 // RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/identical/identical.cpp > 
%T/Inputs/identical/identical.cpp
 // RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file1.yaml 
> %T/Inputs/identical/file1.yaml
+// RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file2.yaml 
> %T/Inputs/identical/file2.yaml
 // RUN: clang-apply-replacements %T/Inputs/identical
 // RUN: FileCheck -input-file=%T/Inputs/identical/identical.cpp 
%S/Inputs/identical/identical.cpp
Index: test/clang-apply-replacements/Inputs/identical/identical.cpp
===
--- test/clang-apply-replacements/Inputs/identical/identical.cpp
+++ test/clang-apply-replacements/Inputs/identical/identical.cpp
@@ -1,2 +1,2 @@
 class MyType {};
-// CHECK: class MyType00 {};
+// CHECK: class MyType0 {};
Index: test/clang-apply-replacements/Inputs/identical/file2.yaml
===
--- test/clang-apply-replacements/Inputs/identical/file2.yaml
+++ test/clang-apply-replacements/Inputs/identical/file2.yaml
@@ -10,9 +10,5 @@
 Offset:  12
 Length:  0
 ReplacementText: '0'
-  - FilePath:$(path)/identical.cpp
-Offset:  12
-Length:  0
-ReplacementText: '0'
 ...
 
Index: test/clang-apply-replacements/Inputs/identical/file1.yaml
===
--- test/clang-apply-replacements/Inputs/identical/file1.yaml
+++ test/clang-apply-replacements/Inputs/identical/file1.yaml
@@ -10,9 +10,5 @@
 Offset:  12
 Length:  0
 ReplacementText: '0'
-  - FilePath:$(path)/identical.cpp
-Offset:  12
-Length:  0
-ReplacementText: '0'
 ...
 
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -125,7 +125,8 @@
 }
 
 /// \brief Extract replacements from collected TranslationUnitReplacements and
-/// TranslationUnitDiagnostics and group them per file.
+/// TranslationUnitDiagnostics and group them per file. Identical replacements
+/// from diagnostics are deduplicated.
 ///
 /// \param[in] TUs Collection of all found and deserialized
 /// TranslationUnitReplacements.
@@ -142,10 +143,20 @@
   llvm::DenseMap>
   GroupedReplacements;
 
-  auto AddToGroup = [&](const tooling::Replacement ) {
+  // Deduplicate identical replacements in diagnostics.
+  // FIXME: Find an efficient way to deduplicate on diagnostics level.
+  llvm::DenseMap>
+  DiagReplacements;
+
+  auto AddToGroup = [&](const tooling::Replacement , bool FromDiag) {
 // Use the file manager to deduplicate paths. FileEntries are
 // automatically canonicalized.
 if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) 
{
+  if (FromDiag) {
+auto  = DiagReplacements[Entry];
+if (!Replaces.insert(R).second)
+  return;
+  }
   GroupedReplacements[Entry].push_back(R);
 } else if (Warned.insert(R.getFilePath()).second) {
   errs() << "Described file '" << R.getFilePath()
@@ -155,13 +166,13 @@
 
   for (const auto  : TUs)
 for (const tooling::Replacement  : TU.Replacements)
-  AddToGroup(R);
+  AddToGroup(R, false);
 
   for (const auto  : TUDs)
 for (const auto  : TU.Diagnostics)
   for (const auto  : D.Fix)
 for (const tooling::Replacement  : Fix.second)
-  AddToGroup(R);
+  AddToGroup(R, true);
 
   // Sort replacements per file to keep consistent behavior when
   // clang-apply-replacements run on differents 

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 166110.
yvvan added a comment.

Test is added


https://reviews.llvm.org/D52261

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/member-access.c

Index: test/CodeCompletion/member-access.c
===
--- test/CodeCompletion/member-access.c
+++ test/CodeCompletion/member-access.c
@@ -10,3 +10,22 @@
   // CHECK-CC1: x
   // CHECK-CC1: y
   // CHECK-CC1: z
+}
+
+struct Point2 {
+  float x;
+};
+
+void test2(struct Point2 p) {
+  p->
+}
+
+void test3(struct Point2 *p) {
+  p.
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:20:6 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: x (requires fix-it: {20:4-20:6} to ".")
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:24:5 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3: x (requires fix-it: {24:4-24:5} to "->")
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4037,39 +4037,33 @@
   QualType ConvertedBaseType = ConvertedBase.get()->getType();
 
   enum CodeCompletionContext::Kind contextKind;
+  QualType TypeWithoutPointer = ConvertedBaseType;
 
   if (IsArrow) {
-if (const PointerType *Ptr = ConvertedBaseType->getAs())
-  ConvertedBaseType = Ptr->getPointeeType();
+if (const PointerType *Ptr = TypeWithoutPointer->getAs())
+  TypeWithoutPointer = Ptr->getPointeeType();
   }
 
   if (IsArrow) {
 contextKind = CodeCompletionContext::CCC_ArrowMemberAccess;
   } else {
-if (ConvertedBaseType->isObjCObjectPointerType() ||
-ConvertedBaseType->isObjCObjectOrInterfaceType()) {
+if (TypeWithoutPointer->isObjCObjectPointerType() ||
+TypeWithoutPointer->isObjCObjectOrInterfaceType()) {
   contextKind = CodeCompletionContext::CCC_ObjCPropertyAccess;
 } else {
   contextKind = CodeCompletionContext::CCC_DotMemberAccess;
 }
   }
 
-  CodeCompletionContext CCContext(contextKind, ConvertedBaseType);
+  CodeCompletionContext CCContext(contextKind, TypeWithoutPointer);
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(), CCContext,
 ::IsMember);
 
-  auto DoCompletion = [&](Expr *Base, bool IsArrow, Optional AccessOpFixIt) -> bool {
-if (!Base)
+  auto DoCompletion = [&](QualType BaseType, bool IsArrow,
+  Optional AccessOpFixIt) -> bool {
+if (BaseType.isNull())
   return false;
-
-ExprResult ConvertedBase = PerformMemberExprBaseConversion(Base, IsArrow);
-if (ConvertedBase.isInvalid())
-  return false;
-Base = ConvertedBase.get();
-
-QualType BaseType = Base->getType();
-
 if (IsArrow) {
   if (const PointerType *Ptr = BaseType->getAs())
 BaseType = Ptr->getPointeeType();
@@ -4140,13 +4134,28 @@
 
   Results.EnterNewScope();
 
-  bool CompletionSucceded = DoCompletion(Base, IsArrow, None);
+  bool CompletionSucceded = DoCompletion(ConvertedBaseType, IsArrow, None);
   if (CodeCompleter->includeFixIts()) {
-const CharSourceRange OpRange =
-CharSourceRange::getTokenRange(OpLoc, OpLoc);
-CompletionSucceded |= DoCompletion(
-OtherOpBase, !IsArrow,
-FixItHint::CreateReplacement(OpRange, IsArrow ? "." : "->"));
+QualType ConvertedOtherBaseType;
+if (!OtherOpBase) {
+  const Type *BaseTypePtr = ConvertedBaseType.getTypePtrOrNull();
+  // Proceed if the initial type is pointer but IsArrow is false or the
+  // other way around.
+  if (BaseTypePtr && IsArrow != BaseTypePtr->isPointerType())
+ConvertedOtherBaseType = ConvertedBaseType;
+} else {
+  ExprResult ConvertedOtherBase =
+  PerformMemberExprBaseConversion(OtherOpBase, !IsArrow);
+  if (!ConvertedOtherBase.isInvalid())
+ConvertedOtherBaseType = ConvertedOtherBase.get()->getType();
+}
+if (!ConvertedOtherBaseType.isNull()) {
+  const CharSourceRange OpRange =
+  CharSourceRange::getTokenRange(OpLoc, OpLoc);
+  CompletionSucceded |= DoCompletion(
+  ConvertedOtherBaseType, !IsArrow,
+  FixItHint::CreateReplacement(OpRange, IsArrow ? "." : "->"));
+}
   }
 
   Results.ExitScope();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r342540 - [clang-tidy] run-clang-tidy.py - fails using python 3.7

[Andi-Bogdan Postelnicu via cfe-commits]

Author: abpostelnicu
Date: Wed Sep 19 04:52:20 2018
New Revision: 342540

URL: http://llvm.org/viewvc/llvm-project?rev=342540=rev
Log:
[clang-tidy] run-clang-tidy.py - fails using python 3.7
Differential Revision: https://reviews.llvm.org/D51220

Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=342540=342539=342540=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Wed Sep 19 
04:52:20 2018
@@ -167,9 +167,9 @@ def run_tidy(args, tmpdir, build_path, q
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')
-  if err > 0:
-sys.stderr.write(err + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  if len(err) > 0:
+sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
 


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

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.
yvvan added reviewers: nik, ilya-biryukov.

Current completion fix-its approach does not provide OtherOpBase for C code. 
But we can easily proceed in this case taking the original Base type.


https://reviews.llvm.org/D52261

Files:
  lib/Sema/SemaCodeComplete.cpp


Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4037,39 +4037,33 @@
   QualType ConvertedBaseType = ConvertedBase.get()->getType();
 
   enum CodeCompletionContext::Kind contextKind;
+  QualType TypeWithoutPointer = ConvertedBaseType;
 
   if (IsArrow) {
-if (const PointerType *Ptr = ConvertedBaseType->getAs())
-  ConvertedBaseType = Ptr->getPointeeType();
+if (const PointerType *Ptr = TypeWithoutPointer->getAs())
+  TypeWithoutPointer = Ptr->getPointeeType();
   }
 
   if (IsArrow) {
 contextKind = CodeCompletionContext::CCC_ArrowMemberAccess;
   } else {
-if (ConvertedBaseType->isObjCObjectPointerType() ||
-ConvertedBaseType->isObjCObjectOrInterfaceType()) {
+if (TypeWithoutPointer->isObjCObjectPointerType() ||
+TypeWithoutPointer->isObjCObjectOrInterfaceType()) {
   contextKind = CodeCompletionContext::CCC_ObjCPropertyAccess;
 } else {
   contextKind = CodeCompletionContext::CCC_DotMemberAccess;
 }
   }
 
-  CodeCompletionContext CCContext(contextKind, ConvertedBaseType);
+  CodeCompletionContext CCContext(contextKind, TypeWithoutPointer);
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(), CCContext,
 ::IsMember);
 
-  auto DoCompletion = [&](Expr *Base, bool IsArrow, Optional 
AccessOpFixIt) -> bool {
-if (!Base)
+  auto DoCompletion = [&](QualType BaseType, bool IsArrow,
+  Optional AccessOpFixIt) -> bool {
+if (BaseType.isNull())
   return false;
-
-ExprResult ConvertedBase = PerformMemberExprBaseConversion(Base, IsArrow);
-if (ConvertedBase.isInvalid())
-  return false;
-Base = ConvertedBase.get();
-
-QualType BaseType = Base->getType();
-
 if (IsArrow) {
   if (const PointerType *Ptr = BaseType->getAs())
 BaseType = Ptr->getPointeeType();
@@ -4140,13 +4134,28 @@
 
   Results.EnterNewScope();
 
-  bool CompletionSucceded = DoCompletion(Base, IsArrow, None);
+  bool CompletionSucceded = DoCompletion(ConvertedBaseType, IsArrow, None);
   if (CodeCompleter->includeFixIts()) {
-const CharSourceRange OpRange =
-CharSourceRange::getTokenRange(OpLoc, OpLoc);
-CompletionSucceded |= DoCompletion(
-OtherOpBase, !IsArrow,
-FixItHint::CreateReplacement(OpRange, IsArrow ? "." : "->"));
+QualType ConvertedOtherBaseType;
+if (!OtherOpBase) {
+  const Type *BaseTypePtr = ConvertedBaseType.getTypePtrOrNull();
+  // Proceed if the initial type is pointer but IsArrow is false or the
+  // other way around.
+  if (BaseTypePtr && IsArrow != BaseTypePtr->isPointerType())
+ConvertedOtherBaseType = ConvertedBaseType;
+} else {
+  ExprResult ConvertedOtherBase =
+  PerformMemberExprBaseConversion(OtherOpBase, !IsArrow);
+  if (!ConvertedOtherBase.isInvalid())
+ConvertedOtherBaseType = ConvertedOtherBase.get()->getType();
+}
+if (!ConvertedOtherBaseType.isNull()) {
+  const CharSourceRange OpRange =
+  CharSourceRange::getTokenRange(OpLoc, OpLoc);
+  CompletionSucceded |= DoCompletion(
+  ConvertedOtherBaseType, !IsArrow,
+  FixItHint::CreateReplacement(OpRange, IsArrow ? "." : "->"));
+}
   }
 
   Results.ExitScope();


Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4037,39 +4037,33 @@
   QualType ConvertedBaseType = ConvertedBase.get()->getType();
 
   enum CodeCompletionContext::Kind contextKind;
+  QualType TypeWithoutPointer = ConvertedBaseType;
 
   if (IsArrow) {
-if (const PointerType *Ptr = ConvertedBaseType->getAs())
-  ConvertedBaseType = Ptr->getPointeeType();
+if (const PointerType *Ptr = TypeWithoutPointer->getAs())
+  TypeWithoutPointer = Ptr->getPointeeType();
   }
 
   if (IsArrow) {
 contextKind = CodeCompletionContext::CCC_ArrowMemberAccess;
   } else {
-if (ConvertedBaseType->isObjCObjectPointerType() ||
-ConvertedBaseType->isObjCObjectOrInterfaceType()) {
+if (TypeWithoutPointer->isObjCObjectPointerType() ||
+TypeWithoutPointer->isObjCObjectOrInterfaceType()) {
   contextKind = CodeCompletionContext::CCC_ObjCPropertyAccess;
 } else {
   contextKind = CodeCompletionContext::CCC_DotMemberAccess;
 }
   }
 
-  CodeCompletionContext CCContext(contextKind, ConvertedBaseType);
+  CodeCompletionContext CCContext(contextKind, 

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

[Andi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342540: [clang-tidy] run-clang-tidy.py - fails using python 
3.7 (authored by Abpostelnicu, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51220?vs=166095=166096#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51220

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -167,9 +167,9 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')
-  if err > 0:
-sys.stderr.write(err + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  if len(err) > 0:
+sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
 


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -167,9 +167,9 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')
-  if err > 0:
-sys.stderr.write(err + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
+  if len(err) > 0:
+sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

[Andi via Phabricator via cfe-commits]

Abpostelnicu updated this revision to Diff 166095.

https://reviews.llvm.org/D51220

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


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -167,9 +167,9 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')
-  if err > 0:
-sys.stderr.write(err + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
+  if len(err) > 0:
+sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
 


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -167,9 +167,9 @@
 if proc.returncode != 0:
   failed_files.append(name)
 with lock:
-  sys.stdout.write(' '.join(invocation) + '\n' + output + '\n')
-  if err > 0:
-sys.stderr.write(err + '\n')
+  sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
+  if len(err) > 0:
+sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52173: Python bindings TypeError in reparse method

[Axel Tillequin via Phabricator via cfe-commits]

bdcht edited reviewers, added: sylvestre.ledru; removed: clang.
bdcht added a comment.

changed reviewers.


Repository:
  rC Clang

https://reviews.llvm.org/D52173



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added a comment.

PTAL again.

I confirmed that current patch can link chrome and functions with local static 
variable are exported.

But current ToT clang was not improved well by this patch.
I guess there is some change recently making effect of this patch smaller. Or 
chromium has many inline functions with static local variable?


https://reviews.llvm.org/D51340



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta updated this revision to Diff 166087.
takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option 
to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl".
takuto.ikuta edited the summary of this revision.

https://reviews.llvm.org/D51340

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/CLCompatOptions.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+int ImportedFunctionUser() {
+  return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;
+
+  // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+  // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+  void InclassDefFunc() {}
+
+  int f();
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+  }
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+  }
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+  __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+  // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+  void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+  static int static_variable = 0;
+  return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+  NoTemplateExportedClass a;
+  a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+
+  T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void 

[PATCH] D51214: [clangd] Add option to enable/disable function argument snippets.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE342533: [clangd] Add option to enable/disable function 
argument snippets. (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51214?vs=166076=166082#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -169,6 +169,13 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt EnableFunctionArgSnippets(
+"function-arg-placeholders",
+llvm::cl::desc("When disabled, completions contain only parentheses for "
+   "function calls. When enabled, completions also contain "
+   "placeholders for method parameters."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
@@ -296,6 +303,7 @@
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -169,6 +169,13 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt EnableFunctionArgSnippets(
+"function-arg-placeholders",
+llvm::cl::desc("When disabled, completions contain only parentheses for "
+   "function calls. When enabled, completions also contain "
+   "placeholders for method parameters."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
@@ -296,6 +303,7 @@
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r342533 - [clangd] Add option to enable/disable function argument snippets.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Sep 19 03:16:44 2018
New Revision: 342533

URL: http://llvm.org/viewvc/llvm-project?rev=342533=rev
Log:
[clangd] Add option to enable/disable function argument snippets.

Summary:
Currently LSP clients cannot directly change EnableFunctionArgSnippets 
parameter.
This patch is to provide them with a way to enable/disable that functionality.

Reviewers: hokein, ioeric, ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: sammccall, MaskRay, jkorous, arphaman, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=342533=342532=342533=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed Sep 19 03:16:44 2018
@@ -169,6 +169,13 @@ static llvm::cl::opt Co
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt EnableFunctionArgSnippets(
+"function-arg-placeholders",
+llvm::cl::desc("When disabled, completions contain only parentheses for "
+   "function calls. When enabled, completions also contain "
+   "placeholders for method parameters."),
+llvm::cl::init(clangd::CodeCompleteOptions().EnableFunctionArgSnippets));
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
@@ -296,6 +303,7 @@ int main(int argc, char *argv[]) {
 CCOpts.IncludeIndicator.NoInsert.clear();
   }
   CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
+  CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
 
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(


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

[clang-tools-extra] r342529 - [clangd] Store preamble macros in dynamic index.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Sep 19 02:35:04 2018
New Revision: 342529

URL: http://llvm.org/viewvc/llvm-project?rev=342529=rev
Log:
[clangd] Store preamble macros in dynamic index.

Summary:
Pros:
o Loading macros from preamble for every completion is slow (see profile).
o Calculating macro USR is also slow (see profile).
o Sema can provide a lot of macro completion results (e.g. when filter is empty,
60k for some large TUs!).

Cons:
o Slight memory increase in dynamic index (~1%).
o Some extra work during preamble build (should be fine as preamble build and
indexAST is way slower).

Before:
{F7195645}

After:
{F7195646}

Reviewers: ilya-biryukov, sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=342529=342528=342529=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Wed Sep 19 02:35:04 2018
@@ -14,6 +14,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include 
 
@@ -41,10 +42,13 @@ indexSymbols(ASTContext , std::share
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
-
-  if (IsIndexMainAST)
+  if (IsIndexMainAST) {
 // We only collect refs when indexing main AST.
 CollectorOpts.RefFilter = RefKind::All;
+  }else {
+IndexOpts.IndexMacrosInPreprocessor = true;
+CollectorOpts.CollectMacro = true;
+  }
 
   SymbolCollector Collector(std::move(CollectorOpts));
   Collector.setPreprocessor(PP);

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=342529=342528=342529=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Sep 19 
02:35:04 2018
@@ -223,12 +223,13 @@ TEST(CompletionTest, Filter) {
 void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
 
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -276,11 +277,12 @@ void TestAfterDotCompletion(clangd::Code
 void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -430,10 +432,11 @@ TEST(CompletionTest, Snippets) {
 TEST(CompletionTest, Kinds) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
   int variable;
   struct Struct {};
   int function();
+  // make sure MACRO is not included in preamble.
+  #define MACRO 10
   int X = ^
   )cpp",
   {func("indexFunction"), var("indexVariable"), cls("indexClass")});
@@ -1921,6 +1924,21 @@ TEST(CompletionTest, MergeMacrosFromInde
   UnorderedElementsAre(Named("Clangd_Macro_Test")));
 }
 
+TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+  auto Results = completions(
+  R"cpp(#define CLANGD_PREAMBLE x
+
+  int x = 0;
+  #define CLANGD_MAIN x
+  void f() { CLANGD_^ }
+  )cpp",
+  {func("CLANGD_INDEX")});
+  // Index is overriden in code completion options, so the preamble symbol is
+  // not seen.
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
+
Named("CLANGD_INDEX")));
+}
+
 TEST(CompletionTest, DeprecatedResults) {
   std::string Body = R"cpp(
 void TestClangd();

Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=342529=342528=342529=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Wed 

[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE342529: [clangd] Store preamble macros in dynamic index. 
(authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52078?vs=165959=166078#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078

Files:
  clangd/index/FileIndex.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -223,12 +223,13 @@
 void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
 
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -276,11 +277,12 @@
 void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -430,10 +432,11 @@
 TEST(CompletionTest, Kinds) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
   int variable;
   struct Struct {};
   int function();
+  // make sure MACRO is not included in preamble.
+  #define MACRO 10
   int X = ^
   )cpp",
   {func("indexFunction"), var("indexVariable"), cls("indexClass")});
@@ -1921,6 +1924,21 @@
   UnorderedElementsAre(Named("Clangd_Macro_Test")));
 }
 
+TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+  auto Results = completions(
+  R"cpp(#define CLANGD_PREAMBLE x
+
+  int x = 0;
+  #define CLANGD_MAIN x
+  void f() { CLANGD_^ }
+  )cpp",
+  {func("CLANGD_INDEX")});
+  // Index is overriden in code completion options, so the preamble symbol is
+  // not seen.
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
+Named("CLANGD_INDEX")));
+}
+
 TEST(CompletionTest, DeprecatedResults) {
   std::string Body = R"cpp(
 void TestClangd();
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -15,6 +15,7 @@
 #include "index/FileIndex.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "gtest/gtest.h"
@@ -330,6 +331,21 @@
  FileURI("unittest:///test2.cc"))}));
 }
 
+TEST(FileIndexTest, CollectMacros) {
+  FileIndex M;
+  update(M, "f", "#define CLANGD 1");
+
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  bool SeenSymbol = false;
+  M.index().fuzzyFind(Req, [&](const Symbol ) {
+EXPECT_EQ(Sym.Name, "CLANGD");
+EXPECT_EQ(Sym.SymInfo.Kind, index::SymbolKind::Macro);
+SeenSymbol = true;
+  });
+  EXPECT_TRUE(SeenSymbol);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -14,6 +14,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include 
 
@@ -41,10 +42,13 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
-
-  if (IsIndexMainAST)
+  if (IsIndexMainAST) {
 // We only collect refs when indexing main AST.
 CollectorOpts.RefFilter = RefKind::All;
+  }else {
+IndexOpts.IndexMacrosInPreprocessor = true;
+CollectorOpts.CollectMacro = true;
+  }
 
   SymbolCollector Collector(std::move(CollectorOpts));
   Collector.setPreprocessor(PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52079: [Sema] Do not load macros from preamble when LoadExternal is false.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342528: [Sema] Do not load macros from preamble when 
LoadExternal is false. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52079?vs=165969=166077#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52079

Files:
  include/clang/Sema/CodeCompleteOptions.h
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-pch-skip.cpp

Index: include/clang/Sema/CodeCompleteOptions.h
===
--- include/clang/Sema/CodeCompleteOptions.h
+++ include/clang/Sema/CodeCompleteOptions.h
@@ -36,7 +36,8 @@
   unsigned IncludeBriefComments : 1;
 
   /// Hint whether to load data from the external AST to provide full results.
-  /// If false, namespace-level declarations from the preamble may be omitted.
+  /// If false, namespace-level declarations and macros from the preamble may be
+  /// omitted.
   unsigned LoadExternal : 1;
 
   /// Include results after corrections (small fix-its), e.g. change '.' to '->'
Index: test/Index/complete-pch-skip.cpp
===
--- test/Index/complete-pch-skip.cpp
+++ test/Index/complete-pch-skip.cpp
@@ -4,19 +4,26 @@
 
 int main() { return ns:: }
 int main2() { return ns::foo(). }
+int main3() { PREAMBLE_ }
 
-// RUN: echo "namespace ns { struct foo { int baz }; }" > %t.h
+// RUN: printf "namespace ns { struct foo { int baz }; }\n#define PREAMBLE_MAC" > %t.h
 // RUN: c-index-test -write-pch %t.h.pch -x c++-header %t.h
 //
 // RUN: c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=WITH-PCH %s
 // WITH-PCH: {TypedText bar}
 // WITH-PCH: {TypedText foo}
 
+// RUN: c-index-test -code-completion-at=%s:7:24 -include %t.h %s | FileCheck -check-prefix=WITH-PCH-MACRO %s
+// WITH-PCH-MACRO: {TypedText PREAMBLE_MAC}
+
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH %s
 // SKIP-PCH-NOT: foo
 // SKIP-PCH: {TypedText bar}
 // SKIP-PCH-NOT: foo
 
+// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:7:24 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH-MACRO %s
+// SKIP-PCH-MACRO-NOT: {TypedText PREAMBLE_MAC}
+
 // Verify that with *no* preamble (no -include flag) we still get local results.
 // SkipPreamble used to break this, by making lookup *too* lazy.
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 %s | FileCheck -check-prefix=NO-PCH %s
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3304,14 +3304,14 @@
 }
 
 static void AddMacroResults(Preprocessor , ResultBuilder ,
-bool IncludeUndefined,
+bool LoadExternal, bool IncludeUndefined,
 bool TargetTypeIsPointer = false) {
   typedef CodeCompletionResult Result;
 
   Results.EnterNewScope();
 
-  for (Preprocessor::macro_iterator M = PP.macro_begin(),
- MEnd = PP.macro_end();
+  for (Preprocessor::macro_iterator M = PP.macro_begin(LoadExternal),
+MEnd = PP.macro_end(LoadExternal);
M != MEnd; ++M) {
 auto MD = PP.getMacroDefinition(M->first);
 if (IncludeUndefined || MD) {
@@ -3327,7 +3327,6 @@
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions ,
@@ -3611,7 +3610,7 @@
   }
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -3749,7 +3748,8 @@
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false,
+PreferredTypeIsPointer);
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
 }
@@ -4372,7 +4372,7 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -4688,7 +4688,7 @@
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, 

r342528 - [Sema] Do not load macros from preamble when LoadExternal is false.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Sep 19 02:34:55 2018
New Revision: 342528

URL: http://llvm.org/viewvc/llvm-project?rev=342528=rev
Log:
[Sema] Do not load macros from preamble when LoadExternal is false.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/Index/complete-pch-skip.cpp

Modified: cfe/trunk/include/clang/Sema/CodeCompleteOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteOptions.h?rev=342528=342527=342528=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteOptions.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteOptions.h Wed Sep 19 02:34:55 2018
@@ -36,7 +36,8 @@ public:
   unsigned IncludeBriefComments : 1;
 
   /// Hint whether to load data from the external AST to provide full results.
-  /// If false, namespace-level declarations from the preamble may be omitted.
+  /// If false, namespace-level declarations and macros from the preamble may 
be
+  /// omitted.
   unsigned LoadExternal : 1;
 
   /// Include results after corrections (small fix-its), e.g. change '.' to 
'->'

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=342528=342527=342528=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Sep 19 02:34:55 2018
@@ -3304,14 +3304,14 @@ CXCursorKind clang::getCursorKindForDecl
 }
 
 static void AddMacroResults(Preprocessor , ResultBuilder ,
-bool IncludeUndefined,
+bool LoadExternal, bool IncludeUndefined,
 bool TargetTypeIsPointer = false) {
   typedef CodeCompletionResult Result;
 
   Results.EnterNewScope();
 
-  for (Preprocessor::macro_iterator M = PP.macro_begin(),
- MEnd = PP.macro_end();
+  for (Preprocessor::macro_iterator M = PP.macro_begin(LoadExternal),
+MEnd = PP.macro_end(LoadExternal);
M != MEnd; ++M) {
 auto MD = PP.getMacroDefinition(M->first);
 if (IncludeUndefined || MD) {
@@ -3327,7 +3327,6 @@ static void AddMacroResults(Preprocessor
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions ,
@@ -3611,7 +3610,7 @@ void Sema::CodeCompleteOrdinaryName(Scop
   }
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -3749,7 +3748,8 @@ void Sema::CodeCompleteExpression(Scope
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false,
+PreferredTypeIsPointer);
   HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
 Results.data(), Results.size());
 }
@@ -4372,7 +4372,7 @@ void Sema::CodeCompleteCase(Scope *S) {
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -4688,7 +4688,7 @@ void Sema::CodeCompleteAfterIf(Scope *S)
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -5722,7 +5722,7 @@ void Sema::CodeCompleteObjCPassingType(S
  CodeCompleter->loadExternal());
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, 
Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -5951,10 +5951,9 @@ void Sema::CodeCompleteObjCMessageReceiv
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   HandleCodeCompleteResults(this,