[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

This change is in conflict with our similar patch to introduce error handling 
in `ASTImporter`. That patch would be a much bigger one, because (almost) every 
import can fail, return of every `Import` (not only Decl) is changed from `X` 
to `Expected`. After every import (inside the import functions too) the 
result needs to be checked always for error.


Repository:
  rC Clang

https://reviews.llvm.org/D50672



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

[PATCH] D50645: [clangd] Show non-instantiated decls in signatureHelp

[Haojian Wu via Phabricator via cfe-commits]

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

looks good.




Comment at: clangd/CodeComplete.cpp:721
+  // would get 'std::basic_string').
+  if (auto Func = Candidate.getFunction()) {
+if (auto Pattern = Func->getTemplateInstantiationPattern())

nit: auto*, the same below.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50645



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

r339651 - [X86] Lowering addus/subus intrinsics to native IR

[Tomasz Krupa via cfe-commits]

Author: tkrupa
Date: Tue Aug 14 01:01:38 2018
New Revision: 339651

URL: http://llvm.org/viewvc/llvm-project?rev=339651&view=rev
Log:
[X86] Lowering addus/subus intrinsics to native IR

Summary: This is the patch that lowers x86 intrinsics to native IR in order to 
enable optimizations.

Reviewers: craig.topper, spatel, RKSimon

Reviewed By: craig.topper

Subscribers: cfe-commits

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


Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/avx2-builtins.c
cfe/trunk/test/CodeGen/avx512bw-builtins.c
cfe/trunk/test/CodeGen/avx512vlbw-builtins.c
cfe/trunk/test/CodeGen/sse2-builtins.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=339651&r1=339650&r2=339651&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Aug 14 01:01:38 2018
@@ -8907,6 +8907,39 @@ static Value *EmitX86SExtMask(CodeGenFun
   return CGF.Builder.CreateSExt(Mask, DstTy, "vpmovm2");
 }
 
+// Emit addition or subtraction with saturation.
+// Handles both signed and unsigned intrinsics.
+static Value *EmitX86AddSubSatExpr(CodeGenFunction &CGF, const CallExpr *E,
+   SmallVectorImpl &Ops,
+   bool IsAddition) {
+
+  // Collect vector elements and type data.
+  llvm::Type *ResultType = CGF.ConvertType(E->getType());
+
+  Value *Res;
+  if (IsAddition) {
+// ADDUS: a > (a+b) ? ~0 : (a+b)
+// If Ops[0] > Add, overflow occured.
+Value *Add = CGF.Builder.CreateAdd(Ops[0], Ops[1]);
+Value *ICmp = CGF.Builder.CreateICmp(ICmpInst::ICMP_UGT, Ops[0], Add);
+Value *Max = llvm::Constant::getAllOnesValue(ResultType);
+Res = CGF.Builder.CreateSelect(ICmp, Max, Add);
+  } else {
+// SUBUS: max(a, b) - b
+Value *ICmp = CGF.Builder.CreateICmp(ICmpInst::ICMP_UGT, Ops[0], Ops[1]);
+Value *Select = CGF.Builder.CreateSelect(ICmp, Ops[0], Ops[1]);
+Res = CGF.Builder.CreateSub(Select, Ops[1]);
+  }
+
+  if (E->getNumArgs() == 4) { // For masked intrinsics.
+Value *VecSRC = Ops[2];
+Value *Mask = Ops[3];
+return EmitX86Select(CGF, Mask, Res, VecSRC);
+  }
+
+  return Res;
+}
+
 Value *CodeGenFunction::EmitX86CpuIs(const CallExpr *E) {
   const Expr *CPUExpr = E->getArg(0)->IgnoreParenCasts();
   StringRef CPUStr = cast(CPUExpr)->getString();
@@ -10530,10 +10563,23 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 Load->setVolatile(true);
 return Load;
   }
+  case X86::BI__builtin_ia32_paddusb512_mask:
+  case X86::BI__builtin_ia32_paddusw512_mask:
+  case X86::BI__builtin_ia32_paddusb256:
+  case X86::BI__builtin_ia32_paddusw256:
+  case X86::BI__builtin_ia32_paddusb128:
+  case X86::BI__builtin_ia32_paddusw128:
+return EmitX86AddSubSatExpr(*this, E, Ops, true /* IsAddition */);
+  case X86::BI__builtin_ia32_psubusb512_mask:
+  case X86::BI__builtin_ia32_psubusw512_mask:
+  case X86::BI__builtin_ia32_psubusb256:
+  case X86::BI__builtin_ia32_psubusw256:
+  case X86::BI__builtin_ia32_psubusb128:
+  case X86::BI__builtin_ia32_psubusw128:
+return EmitX86AddSubSatExpr(*this, E, Ops, false /* IsAddition */);
   }
 }
 
-
 Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned BuiltinID,
const CallExpr *E) {
   SmallVector Ops;

Modified: cfe/trunk/test/CodeGen/avx2-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx2-builtins.c?rev=339651&r1=339650&r2=339651&view=diff
==
--- cfe/trunk/test/CodeGen/avx2-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx2-builtins.c Tue Aug 14 01:01:38 2018
@@ -68,13 +68,19 @@ __m256i test_mm256_adds_epi16(__m256i a,
 
 __m256i test_mm256_adds_epu8(__m256i a, __m256i b) {
   // CHECK-LABEL: test_mm256_adds_epu8
-  // CHECK: call <32 x i8> @llvm.x86.avx2.paddus.b(<32 x i8> %{{.*}}, <32 x 
i8> %{{.*}})
+  // CHECK-NOT: call <32 x i8> @llvm.x86.avx2.paddus.b(<32 x i8> %{{.*}}, <32 
x i8> %{{.*}})
+  // CHECK: add <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: icmp ugt <32 x i8> %{{.*}}, %{{.*}}
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i8> , <32 x i8> {{.*}}
   return _mm256_adds_epu8(a, b);
 }
 
 __m256i test_mm256_adds_epu16(__m256i a, __m256i b) {
   // CHECK-LABEL: test_mm256_adds_epu16
-  // CHECK: call <16 x i16> @llvm.x86.avx2.paddus.w(<16 x i16> %{{.*}}, <16 x 
i16> %{{.*}})
+  // CHECK-NOT: call <16 x i16> @llvm.x86.avx2.paddus.w(<16 x i16> %{{.*}}, 
<16 x i16> %{{.*}})
+  // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp ugt <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> {{.*}}
   return _mm256_adds_epu16(a, b);
 }
 
@@ -1181,13 +1187,19 @@ __m256i test_mm256_subs_epi16(__m256i a,
 
 __m256i test_mm256_subs_epu8(__m256i a, __m

[PATCH] D46892: [X86] Lowering addus/subus intrinsics to native IR (Clang part)

[Tomasz Krupa via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339651: [X86] Lowering addus/subus intrinsics to native IR 
(authored by tkrupa, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46892?vs=159461&id=160523#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46892

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/avx2-builtins.c
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512vlbw-builtins.c
  test/CodeGen/sse2-builtins.c

Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -59,13 +59,19 @@
 
 __m128i test_mm_adds_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epu8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: add <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: icmp ugt <16 x i8> %{{.*}}, %{{.*}}
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i8> , <16 x i8> {{.*}}
   return _mm_adds_epu8(A, B);
 }
 
 __m128i test_mm_adds_epu16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epu16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: add <8 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp ugt <8 x i16> %{{.*}}, %{{.*}}
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> , <8 x i16> {{.*}}
   return _mm_adds_epu16(A, B);
 }
 
@@ -1422,13 +1428,19 @@
 
 __m128i test_mm_subs_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epu8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: icmp ugt <16 x i8> {{.*}}, {{.*}}
+  // CHECK: select <16 x i1> {{.*}}, <16 x i8> {{.*}}, <16 x i8> {{.*}}
+  // CHECK: sub <16 x i8> {{.*}}, {{.*}}
   return _mm_subs_epu8(A, B);
 }
 
 __m128i test_mm_subs_epu16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epu16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.psubus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.psubus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: icmp ugt <8 x i16> {{.*}}, {{.*}}
+  // CHECK: select <8 x i1> {{.*}}, <8 x i16> {{.*}}, <8 x i16> {{.*}}
+  // CHECK: sub <8 x i16> {{.*}}, {{.*}}
   return _mm_subs_epu16(A, B);
 }
 
Index: test/CodeGen/avx512bw-builtins.c
===
--- test/CodeGen/avx512bw-builtins.c
+++ test/CodeGen/avx512bw-builtins.c
@@ -600,7 +600,7 @@
 __m512i test_mm512_mask_adds_epi8(__m512i __W, __mmask64 __U, __m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_mask_adds_epi8
   // CHECK: @llvm.x86.avx512.mask.padds.b.512
-  return _mm512_mask_adds_epi8(__W,__U,__A,__B); 
+ return _mm512_mask_adds_epi8(__W,__U,__A,__B); 
 }
 __m512i test_mm512_maskz_adds_epi8(__mmask64 __U, __m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_maskz_adds_epi8
@@ -610,7 +610,7 @@
 __m512i test_mm512_adds_epi16(__m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_adds_epi16
   // CHECK: @llvm.x86.avx512.mask.padds.w.512
-  return _mm512_adds_epi16(__A,__B); 
+ return _mm512_adds_epi16(__A,__B); 
 }
 __m512i test_mm512_mask_adds_epi16(__m512i __W, __mmask32 __U, __m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_mask_adds_epi16
@@ -620,36 +620,58 @@
 __m512i test_mm512_maskz_adds_epi16(__mmask32 __U, __m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_maskz_adds_epi16
   // CHECK: @llvm.x86.avx512.mask.padds.w.512
-  return _mm512_maskz_adds_epi16(__U,__A,__B); 
+return _mm512_maskz_adds_epi16(__U,__A,__B); 
 }
 __m512i test_mm512_adds_epu8(__m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_adds_epu8
-  // CHECK: @llvm.x86.avx512.mask.paddus.b.512
+  // CHECK-NOT: @llvm.x86.avx512.mask.paddus.b.512
+  // CHECK: add <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: icmp ugt <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: select <64 x i1> %{{.*}}, <64 x i8> , <64 x i8> {{.*}}
   return _mm512_adds_epu8(__A,__B); 
 }
 __m512i test_mm512_mask_adds_epu8(__m512i __W, __mmask64 __U, __m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_mask_adds_epu8
-  // CHECK: @llvm.x86.avx512.mask.paddus.b.512
+  // CHECK-NOT: @llvm.x86.avx512.mask.paddus.b.512
+  // CHECK: add <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: icmp ugt <64 x i8> %{{.*}}, %{{.*}}
+  // CHECK: select <64 x i1> %{{.*}}, <64 x i8> , <64 x i8> {{.*}}
+  // CHECK: select <64 x i1> %{{.*}}, <64 x i8> %{{.*}}, <64 x i8> %{{.*}}
   return _mm512_mask_adds_epu8(__W,__U,__A,__B); 
 }
 __m512i test_mm512_maskz_adds_epu8(__mmask64 __U, __m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_maskz_adds_epu8
-  // CHECK: @llvm.x86.avx512.mask.paddus.b

[PATCH] D50594: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check-clang-analyzer and check-clang-analyzer-z3

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added a comment.

Hi George,

This commit seems to be causing some testsuite regression on Armv8 (both 
Aarch64 and ARM) architectures:
http://lab.llvm.org:8011/builders/clang-cmake-armv8-full/builds/5450
http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/5625

Best regards,

Thomas


Repository:
  rC Clang

https://reviews.llvm.org/D50594



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:5
+
+void DirectAcess(){
+  absl::strings_internal::InternalFunction();

Please run clang-format over the test code as well. The braces here in 
`FriendUsage` miss a space.


https://reviews.llvm.org/D50542



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

[PATCH] D50689: [clangd] NFC: Improve Dex Iterators debugging traits

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay.

https://reviews.llvm.org/D50689

Files:
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -231,13 +231,14 @@
   const PostingList L4 = {0, 1, 5};
   const PostingList L5;
 
-  EXPECT_EQ(llvm::to_string(*(create(L0))), "[4, 7, 8, 20, 42, 100]");
+  EXPECT_EQ(llvm::to_string(*(create(L0))), "[{4}, 7, 8, 20, 42, 100, END]");
 
   auto Nested = createAnd(createAnd(create(L1), create(L2)),
   createOr(create(L3), create(L4), create(L5)));
 
   EXPECT_EQ(llvm::to_string(*Nested),
-"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
+"(& (& [{1}, 3, 5, 8, 9, END] [{1}, 5, 7, 9, END]) (| [0, {5}, "
+"END] [0, {1}, 5, END] [{END}]))");
 }
 
 TEST(DexIndexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -99,7 +99,9 @@
   ///
   /// Where Type is the iterator type representation: "&" for And, "|" for Or,
   /// ChildN is N-th iterator child. Raw iterators over PostingList are
-  /// represented as "[ID1, ID2, ...]" where IDN is N-th PostingList entry.
+  /// represented as "[ID1, ID2, ..., {IDX}, ... END]" where IDN is N-th
+  /// PostingList entry and IDX is the one currently being pointed to by the
+  /// corresponding iterator.
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
const Iterator &Iterator) {
 return Iterator.dump(OS);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -49,10 +49,19 @@
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << '[';
 auto Separator = "";
-for (const auto &ID : Documents) {
-  OS << Separator << ID;
+for (auto It = std::begin(Documents); It != std::end(Documents); ++It) {
+  OS << Separator;
+  if (It == Index)
+OS << '{' << *It << '}';
+  else
+OS << *It;
   Separator = ", ";
 }
+OS << Separator;
+if (Index == std::end(Documents))
+  OS << "{END}";
+else
+  OS << "END";
 OS << ']';
 return OS;
   }


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -231,13 +231,14 @@
   const PostingList L4 = {0, 1, 5};
   const PostingList L5;
 
-  EXPECT_EQ(llvm::to_string(*(create(L0))), "[4, 7, 8, 20, 42, 100]");
+  EXPECT_EQ(llvm::to_string(*(create(L0))), "[{4}, 7, 8, 20, 42, 100, END]");
 
   auto Nested = createAnd(createAnd(create(L1), create(L2)),
   createOr(create(L3), create(L4), create(L5)));
 
   EXPECT_EQ(llvm::to_string(*Nested),
-"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
+"(& (& [{1}, 3, 5, 8, 9, END] [{1}, 5, 7, 9, END]) (| [0, {5}, "
+"END] [0, {1}, 5, END] [{END}]))");
 }
 
 TEST(DexIndexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -99,7 +99,9 @@
   ///
   /// Where Type is the iterator type representation: "&" for And, "|" for Or,
   /// ChildN is N-th iterator child. Raw iterators over PostingList are
-  /// represented as "[ID1, ID2, ...]" where IDN is N-th PostingList entry.
+  /// represented as "[ID1, ID2, ..., {IDX}, ... END]" where IDN is N-th
+  /// PostingList entry and IDX is the one currently being pointed to by the
+  /// corresponding iterator.
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
const Iterator &Iterator) {
 return Iterator.dump(OS);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -49,10 +49,19 @@
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << '[';
 auto Separator = "";
-for (const aut

r339653 - [analyzer][UninitializedObjectChecker] Void pointers are casted back to their dynamic type in note message

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 14 01:20:51 2018
New Revision: 339653

URL: http://llvm.org/viewvc/llvm-project?rev=339653&view=rev
Log:
[analyzer][UninitializedObjectChecker] Void pointers are casted back to their 
dynamic type in note message

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=339653&r1=339652&r2=339653&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 Tue Aug 14 01:20:51 2018
@@ -60,6 +60,32 @@ public:
   }
 };
 
+/// Represents a void* field that needs to be casted back to its dynamic type
+/// for a correct note message.
+class NeedsCastLocField final : public FieldNode {
+  QualType CastBackType;
+
+public:
+  NeedsCastLocField(const FieldRegion *FR, const QualType &T)
+  : FieldNode(FR), CastBackType(T) {}
+
+  virtual void printNoteMsg(llvm::raw_ostream &Out) const override {
+Out << "uninitialized pointee ";
+  }
+
+  virtual void printPrefix(llvm::raw_ostream &Out) const override {
+Out << "static_cast" << '<' << CastBackType.getAsString() << ">(";
+  }
+
+  virtual void printNode(llvm::raw_ostream &Out) const override {
+Out << getVariableName(getDecl()) << ')';
+  }
+
+  virtual void printSeparator(llvm::raw_ostream &Out) const override {
+Out << "->";
+  }
+};
+
 } // end of anonymous namespace
 
 // Utility function declarations.
@@ -122,6 +148,10 @@ bool FindUninitializedFields::isPointerO
 
   QualType DynT = DynTInfo.getType();
 
+  // If the static type of the field is a void pointer, we need to cast it back
+  // to the dynamic type before dereferencing.
+  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
+
   if (isVoidPointer(DynT)) {
 IsAnyFieldInitialized = true;
 return false;
@@ -160,11 +190,16 @@ bool FindUninitializedFields::isPointerO
 
 const TypedValueRegion *R = RecordV->getRegion();
 
-if (DynT->getPointeeType()->isStructureOrClassType())
+if (DynT->getPointeeType()->isStructureOrClassType()) {
+  if (NeedsCastBack)
+return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, 
DynT)));
   return isNonUnionUninit(R, LocalChain.add(LocField(FR)));
+}
 
 if (DynT->getPointeeType()->isUnionType()) {
   if (isUnionUninit(R)) {
+if (NeedsCastBack)
+  return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, 
DynT)));
 return addFieldToUninits(LocalChain.add(LocField(FR)));
   } else {
 IsAnyFieldInitialized = true;
@@ -185,8 +220,11 @@ bool FindUninitializedFields::isPointerO
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
 
-  if (isPrimitiveUninit(DerefdV))
+  if (isPrimitiveUninit(DerefdV)) {
+if (NeedsCastBack)
+  return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
 return addFieldToUninits(LocalChain.add(LocField(FR)));
+  }
 
   IsAnyFieldInitialized = true;
   return false;

Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=339653&r1=339652&r2=339653&view=diff
==
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Tue Aug 14 
01:20:51 2018
@@ -292,7 +292,7 @@ void fCyclicVoidPointerTest() {
 }
 
 struct IntDynTypedVoidPointerTest1 {
-  void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}}
+  void *vptr; // expected-note{{uninitialized pointee 'static_cast(this->vptr)'}}
   int dontGetFilteredByNonPedanticMode = 0;
 
   IntDynTypedVoidPointerTest1(void *vptr) : vptr(vptr) {} // 
expected-warning{{1 uninitialized field}}
@@ -305,8 +305,8 @@ void fIntDynTypedVoidPointerTest1() {
 
 struct RecordDynTypedVoidPointerTest {
   struct RecordType {
-int x; // expected-note{{uninitialized field 'this->vptr->x'}}
-int y; // expected-note{{uninitialized field 'this->vptr->y'}}
+int x; // expected-note{{uninitialized field 'static_cast(this->vptr)->x'}}
+int y; // expected-note{{uninitialized field 'static_cast(this->vptr)->y'}}
   };
 
   void *vptr;
@@ -322,9 +322,9 @@ void fRecordDynTypedVoidPointerTest() {
 
 struct NestedNonVoidDynTypedVoidPointerTest {
   struct RecordType {
-int x;  

[PATCH] D49228: [analyzer][UninitializedObjectChecker] Void pointer objects are casted back to their dynmic type in note message

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339653: [analyzer][UninitializedObjectChecker] Void pointers 
are casted back to their… (authored by Szelethus, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D49228?vs=159918&id=160526#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49228

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -292,7 +292,7 @@
 }
 
 struct IntDynTypedVoidPointerTest1 {
-  void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}}
+  void *vptr; // expected-note{{uninitialized pointee 'static_cast(this->vptr)'}}
   int dontGetFilteredByNonPedanticMode = 0;
 
   IntDynTypedVoidPointerTest1(void *vptr) : vptr(vptr) {} // expected-warning{{1 uninitialized field}}
@@ -305,8 +305,8 @@
 
 struct RecordDynTypedVoidPointerTest {
   struct RecordType {
-int x; // expected-note{{uninitialized field 'this->vptr->x'}}
-int y; // expected-note{{uninitialized field 'this->vptr->y'}}
+int x; // expected-note{{uninitialized field 'static_cast(this->vptr)->x'}}
+int y; // expected-note{{uninitialized field 'static_cast(this->vptr)->y'}}
   };
 
   void *vptr;
@@ -322,9 +322,9 @@
 
 struct NestedNonVoidDynTypedVoidPointerTest {
   struct RecordType {
-int x;  // expected-note{{uninitialized field 'this->vptr->x'}}
-int y;  // expected-note{{uninitialized field 'this->vptr->y'}}
-void *vptr; // expected-note{{uninitialized pointee 'this->vptr->vptr'}}
+int x;  // expected-note{{uninitialized field 'static_cast(this->vptr)->x'}}
+int y;  // expected-note{{uninitialized field 'static_cast(this->vptr)->y'}}
+void *vptr; // expected-note{{uninitialized pointee 'static_cast(static_cast(this->vptr)->vptr)'}}
   };
 
   void *vptr;
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -60,6 +60,32 @@
   }
 };
 
+/// Represents a void* field that needs to be casted back to its dynamic type
+/// for a correct note message.
+class NeedsCastLocField final : public FieldNode {
+  QualType CastBackType;
+
+public:
+  NeedsCastLocField(const FieldRegion *FR, const QualType &T)
+  : FieldNode(FR), CastBackType(T) {}
+
+  virtual void printNoteMsg(llvm::raw_ostream &Out) const override {
+Out << "uninitialized pointee ";
+  }
+
+  virtual void printPrefix(llvm::raw_ostream &Out) const override {
+Out << "static_cast" << '<' << CastBackType.getAsString() << ">(";
+  }
+
+  virtual void printNode(llvm::raw_ostream &Out) const override {
+Out << getVariableName(getDecl()) << ')';
+  }
+
+  virtual void printSeparator(llvm::raw_ostream &Out) const override {
+Out << "->";
+  }
+};
+
 } // end of anonymous namespace
 
 // Utility function declarations.
@@ -122,6 +148,10 @@
 
   QualType DynT = DynTInfo.getType();
 
+  // If the static type of the field is a void pointer, we need to cast it back
+  // to the dynamic type before dereferencing.
+  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
+
   if (isVoidPointer(DynT)) {
 IsAnyFieldInitialized = true;
 return false;
@@ -160,11 +190,16 @@
 
 const TypedValueRegion *R = RecordV->getRegion();
 
-if (DynT->getPointeeType()->isStructureOrClassType())
+if (DynT->getPointeeType()->isStructureOrClassType()) {
+  if (NeedsCastBack)
+return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, DynT)));
   return isNonUnionUninit(R, LocalChain.add(LocField(FR)));
+}
 
 if (DynT->getPointeeType()->isUnionType()) {
   if (isUnionUninit(R)) {
+if (NeedsCastBack)
+  return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
 return addFieldToUninits(LocalChain.add(LocField(FR)));
   } else {
 IsAnyFieldInitialized = true;
@@ -185,8 +220,11 @@
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
 
-  if (isPrimitiveUninit(DerefdV))
+  if (isPrimitiveUninit(DerefdV)) {
+if (NeedsCastBack)
+  return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
 return addFieldToUninits(LocalChain.add(LocField(FR)));
+  }
 
   IsAnyFieldInitialized = true;
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50691: [CMake] Fix the LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY option

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: phosek, rnk, compnerd, smeenai.
Herald added subscribers: ldionne, mgorny.

This option should be available if LIBCXX_ENABLE_SHARED is enabled, not 
LIBCXX_ENABLE_STATIC, since the option is about linking shard libraries.

This fixes a typo from SVN r337814.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50691

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -175,7 +175,7 @@
 
 cmake_dependent_option(LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY
   "Statically link the ABI library to shared library" ON
-  "LIBCXX_ENABLE_STATIC_ABI_LIBRARY;LIBCXX_ENABLE_STATIC" OFF)
+  "LIBCXX_ENABLE_STATIC_ABI_LIBRARY;LIBCXX_ENABLE_SHARED" OFF)
 
 # Generate and install a linker script inplace of libc++.so. The linker script
 # will link libc++ to the correct ABI library. This option is on by default


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -175,7 +175,7 @@
 
 cmake_dependent_option(LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY
   "Statically link the ABI library to shared library" ON
-  "LIBCXX_ENABLE_STATIC_ABI_LIBRARY;LIBCXX_ENABLE_STATIC" OFF)
+  "LIBCXX_ENABLE_STATIC_ABI_LIBRARY;LIBCXX_ENABLE_SHARED" OFF)
 
 # Generate and install a linker script inplace of libc++.so. The linker script
 # will link libc++ to the correct ABI library. This option is on by default
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50691: [CMake] Fix the LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY option

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

@hans I think this should be merged to 7.0, if reviewers agree with the change.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50691



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

[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 160531.
kadircet marked 7 inline comments as done.
kadircet added a comment.

- Polished API.
- Resolved discussions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50502

Files:
  clangd/CMakeLists.txt
  clangd/Cancellation.cpp
  clangd/Cancellation.h
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/CancellationTests.cpp

Index: unittests/clangd/CancellationTests.cpp
===
--- /dev/null
+++ unittests/clangd/CancellationTests.cpp
@@ -0,0 +1,76 @@
+#include "Cancellation.h"
+#include "Context.h"
+#include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(CancellationTest, CancellationTest) {
+  {
+TaskHandle TH = TaskHandle::createCancellableTaskHandle();
+WithContext ContextWithCancellation(
+CancellationHandler::setCurrentCancellationToken(TH));
+EXPECT_FALSE(CancellationHandler::isCancelled());
+TH.cancel();
+EXPECT_TRUE(CancellationHandler::isCancelled());
+  }
+  EXPECT_FALSE(CancellationHandler::isCancelled());
+}
+
+TEST(CancellationTest, CheckForError) {
+  llvm::Error e = handleErrors(CancellationHandler::getCancellationError(),
+   [](const TaskCancelledError &) {});
+  EXPECT_FALSE(e);
+}
+
+TEST(CancellationTest, TaskHandleTestHandleDiesContextLives) {
+  llvm::Optional ContextWithCancellation;
+  {
+auto CancellableTaskHandle = TaskHandle::createCancellableTaskHandle();
+ContextWithCancellation.emplace(
+CancellationHandler::setCurrentCancellationToken(
+CancellableTaskHandle));
+EXPECT_FALSE(CancellationHandler::isCancelled());
+CancellableTaskHandle.cancel();
+EXPECT_TRUE(CancellationHandler::isCancelled());
+  }
+  EXPECT_TRUE(CancellationHandler::isCancelled());
+  ContextWithCancellation.reset();
+  EXPECT_FALSE(CancellationHandler::isCancelled());
+}
+
+TEST(CancellationTest, TaskHandleContextDiesHandleLives) {
+  {
+auto CancellableTaskHandle = TaskHandle::createCancellableTaskHandle();
+{
+  WithContext ContextWithCancellation(
+  CancellationHandler::setCurrentCancellationToken(
+  CancellableTaskHandle));
+  EXPECT_FALSE(CancellationHandler::isCancelled());
+  CancellableTaskHandle.cancel();
+  EXPECT_TRUE(CancellationHandler::isCancelled());
+}
+  }
+  EXPECT_FALSE(CancellationHandler::isCancelled());
+}
+
+TEST(CancellationTest, CancellationToken) {
+  auto CancellableTaskHandle = TaskHandle::createCancellableTaskHandle();
+  WithContext ContextWithCancellation(
+  CancellationHandler::setCurrentCancellationToken(CancellableTaskHandle));
+  auto CT = CancellationHandler::isCancelled();
+  EXPECT_FALSE(CT);
+  CancellableTaskHandle.cancel();
+  EXPECT_TRUE(CT);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
 
 add_extra_unittest(ClangdTests
   Annotations.cpp
+  CancellationTests.cpp
   ClangdTests.cpp
   ClangdUnitTests.cpp
   CodeCompleteTests.cpp
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -55,6 +55,7 @@
   virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
   virtual void onHover(TextDocumentPositionParams &Params) = 0;
   virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
+  virtual void onCancelRequest(CancelParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -75,4 +75,5 @@
   Register("workspace/didChangeConfiguration",
&ProtocolCallbacks::onChangeConfiguration);
   Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol);
+  Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -861,6 +861,13 @@
 llvm::json::Value toJSON(const DocumentHighlight &DH);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const DocumentHighlight &);
 
+struct CancelParams {
+  std::string ID;
+};
+llvm::json::Value toJSON(const CancelParams &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream 

r339655 - [analyzer] Made a buildbot happy.

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 14 01:38:35 2018
New Revision: 339655

URL: http://llvm.org/viewvc/llvm-project?rev=339655&view=rev
Log:
[analyzer] Made a buildbot happy.

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=339655&r1=339654&r2=339655&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
 Tue Aug 14 01:38:35 2018
@@ -215,10 +215,12 @@ bool FindUninitializedFields::isPointerO
 llvm_unreachable("All cases are handled!");
   }
 
-  assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isPointerType() ||
-  DynT->isReferenceType()) &&
+  // Temporary variable to avoid warning from -Wunused-function.
+  bool IsPrimitive = isPrimitiveType(DynT->getPointeeType());
+  assert((IsPrimitive || DynT->isAnyPointerType() || DynT->isReferenceType()) 
&&
  "At this point FR must either have a primitive dynamic type, or it "
  "must be a null, undefined, unknown or concrete pointer!");
+  (void)IsPrimitive;
 
   if (isPrimitiveUninit(DerefdV)) {
 if (NeedsCastBack)


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

[PATCH] D50691: [CMake] Fix the LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY option

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D50691#1198514, @mstorsjo wrote:

> @hans I think this should be merged to 7.0, if reviewers agree with the 
> change.


Okay, I'll keep an eye on it.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50691



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

Re: r338934 - [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Hans Wennborg via cfe-commits]

Merged to 7.0 in r339660.

On Sat, Aug 4, 2018 at 1:12 AM, Volodymyr Sapsai via cfe-commits
 wrote:
> Author: vsapsai
> Date: Fri Aug  3 16:12:37 2018
> New Revision: 338934
>
> URL: http://llvm.org/viewvc/llvm-project?rev=338934&view=rev
> Log:
> [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.
>
> Libc++ needs to know when aligned allocation is supported by clang, but is
> otherwise unavailable at link time. Otherwise, libc++ will incorrectly end up
> generating calls to `__builtin_operator_new`/`__builtin_operator_delete` with
> alignment arguments.
>
> This patch implements the following changes:
>
> * The `__cpp_aligned_new` feature test macro to no longer be defined when
>   aligned allocation is otherwise enabled but unavailable.
>
> * The Darwin driver no longer passes `-faligned-alloc-unavailable` when the
>   user manually specifies `-faligned-allocation` or `-fno-aligned-allocation`.
>
> * Instead of a warning Clang now generates a hard error when an aligned
>   allocation or deallocation function is referenced but unavailable.
>
> Patch by Eric Fiselier.
>
> Reviewers: rsmith, vsapsai, erik.pilkington, ahatanak, dexonsmith
>
> Reviewed By: rsmith
>
> Subscribers: Quuxplusone, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D45015
>
>
> Added:
> cfe/trunk/test/Lexer/aligned-allocation.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/test/Driver/unavailable_aligned_allocation.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=338934&r1=338933&r2=338934&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Aug  3 16:12:37 2018
> @@ -364,7 +364,6 @@ def NonVirtualDtor : DiagGroup<"non-virt
>  def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">;
>  def : DiagGroup<"effc++", [NonVirtualDtor]>;
>  def OveralignedType : DiagGroup<"over-aligned">;
> -def AlignedAllocationUnavailable : 
> DiagGroup<"aligned-allocation-unavailable">;
>  def OldStyleCast : DiagGroup<"old-style-cast">;
>  def : DiagGroup<"old-style-definition">;
>  def OutOfLineDeclaration : DiagGroup<"out-of-line-declaration">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=338934&r1=338933&r2=338934&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug  3 16:12:37 
> 2018
> @@ -6465,12 +6465,12 @@ def warn_overaligned_type : Warning<
>"type %0 requires %1 bytes of alignment and the default allocator only "
>"guarantees %2 bytes">,
>InGroup, DefaultIgnore;
> -def warn_aligned_allocation_unavailable :Warning<
> +def err_aligned_allocation_unavailable : Error<
>"aligned %select{allocation|deallocation}0 function of type '%1' is only "
> -  "available on %2 %3 or newer">, InGroup, 
> DefaultError;
> +  "available on %2 %3 or newer">;
>  def note_silence_unligned_allocation_unavailable : Note<
>"if you supply your own aligned allocation functions, use "
> -  "-Wno-aligned-allocation-unavailable to silence this diagnostic">;
> +  "-faligned-allocation to silence this diagnostic">;
>
>  def err_conditional_void_nonvoid : Error<
>"%select{left|right}1 operand to ? is void, but %select{right|left}1 
> operand "
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=338934&r1=338933&r2=338934&view=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Fri Aug  3 16:12:37 2018
> @@ -2035,7 +2035,11 @@ bool Darwin::isAlignedAllocationUnavaila
>  void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
> llvm::opt::ArgStringList &CC1Args,
> Action::OffloadKind DeviceOffloadKind) 
> const {
> -  if (isAlignedAllocationUnavailable())
> +  // Pass "-faligned-alloc-unavailable" only when the user hasn't manually
> +  // enabled or disabled aligned allocations.
> +  if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
> +options::OPT_fno_aligned_allocation) &&
> +  isAlignedAllocationUnavailable())
>  CC1

[PATCH] D50645: [clangd] Show non-instantiated decls in signatureHelp

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 160536.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Use 'auto*' instead of 'auto'


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50645

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1537,6 +1537,59 @@
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(10, ^);
+})cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T...);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+  // It is debatable whether we should substitute the outer template parameter
+  // ('T') in that case. Currently we don't substitute it in signature help, 
but
+  // do substitute in code complete.
+  // FIXME: make code complete and signature help consistent, figure out which
+  // way is better.
+  EXPECT_THAT(signatures(R"cpp(
+template 
+struct X {
+  template 
+  void foo(T, U);
+};
+
+int main() {
+  X().foo(^)
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -714,7 +714,15 @@
"too many arguments");
 SigHelp.activeParameter = static_cast(CurrentArg);
 for (unsigned I = 0; I < NumCandidates; ++I) {
-  const auto &Candidate = Candidates[I];
+  OverloadCandidate Candidate = Candidates[I];
+  // We want to avoid showing instantiated signatures, because they may be
+  // long in some cases (e.g. when 'T' is substituted with 'std::string', 
we
+  // would get 'std::basic_string').
+  if (auto *Func = Candidate.getFunction()) {
+if (auto *Pattern = Func->getTemplateInstantiationPattern())
+  Candidate = OverloadCandidate(Pattern);
+  }
+
   const auto *CCS = Candidate.CreateSignatureString(
   CurrentArg, S, *Allocator, CCTUInfo, true);
   assert(CCS && "Expected the CodeCompletionString to be non-null");


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1537,6 +1537,59 @@
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(10, ^);
+})cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T...);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+  // It is debatable whether we should substitute the outer template parameter
+  // ('T') in that case. Currently we don't substitute it in signature help, but
+  // do substitute in code complete.
+  // FIXME: make code complete and signature help consistent, figure out which
+  // way is better.
+  EXPECT_THAT(signatures(R"cpp(
+template 
+struct X {
+  template 
+  void foo(T, U);
+};
+
+int main() {
+  X().foo(^)
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -714,7 +714,15 @@
"too many arguments");
 SigHelp.activeParameter = static_cast(CurrentArg);
 for (unsigned I = 0; I < NumCandidates; ++I) {
-  const auto &Candidate = Candidates[I];
+  OverloadCandidate Candidate = Candidate

[PATCH] D50645: [clangd] Show non-instantiated decls in signatureHelp

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/CodeComplete.cpp:721
+  // would get 'std::basic_string').
+  if (auto Func = Candidate.getFunction()) {
+if (auto Pattern = Func->getTemplateInstantiationPattern())

hokein wrote:
> nit: auto*, the same below.
Good catch, thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50645



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

[libcxx] r339661 - Merging r339431:

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Aug 14 02:13:39 2018
New Revision: 339661

URL: http://llvm.org/viewvc/llvm-project?rev=339661&view=rev
Log:
Merging r339431:

r339431 | ldionne | 2018-08-10 15:24:56 +0200 (Fri, 10 Aug 2018) | 16 lines

[libc++] Enable aligned allocation based on feature test macro, irrespective of 
standard

Summary:
The current code enables aligned allocation functions when compiling in C++17
and later. This is a problem because aligned allocation functions might not
be supported on the target platform, which leads to an error at link time.

Since r338934, Clang knows not to define __cpp_aligned_new when it's not
available on the target platform -- this commit takes advantage of that to
only use aligned allocation functions when they are available.

Reviewers: vsapsai, EricWF

Subscribers: christof, dexonsmith, cfe-commits, EricWF, mclow.lists

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


Added:

libcxx/branches/release_70/test/libcxx/memory/aligned_allocation_macro.pass.cpp
  - copied unchanged from r339431, 
libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp
Modified:
libcxx/branches/release_70/   (props changed)
libcxx/branches/release_70/include/__config
libcxx/branches/release_70/include/new

Propchange: libcxx/branches/release_70/
--
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Aug 14 02:13:39 2018
@@ -1 +1,2 @@
 /libcxx/branches/apple:136569-137939
+/libcxx/trunk:339431

Modified: libcxx/branches/release_70/include/__config
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_70/include/__config?rev=339661&r1=339660&r2=339661&view=diff
==
--- libcxx/branches/release_70/include/__config (original)
+++ libcxx/branches/release_70/include/__config Tue Aug 14 02:13:39 2018
@@ -991,6 +991,11 @@ template  struct __static_asse
 #  endif
 #endif // defined(__APPLE__)
 
+#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
+!defined(_LIBCPP_BUILDING_LIBRARY) && \
+(!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606)
+#  define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+#endif
 
 #if defined(__APPLE__) || defined(__FreeBSD__)
 #define _LIBCPP_HAS_DEFAULTRUNELOCALE

Modified: libcxx/branches/release_70/include/new
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_70/include/new?rev=339661&r1=339660&r2=339661&view=diff
==
--- libcxx/branches/release_70/include/new (original)
+++ libcxx/branches/release_70/include/new Tue Aug 14 02:13:39 2018
@@ -108,13 +108,6 @@ void  operator delete[](void* ptr, void*
 # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION
 #endif
 
-#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
-(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \
-(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
-# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
-#endif
-
-
 #if !__has_builtin(__builtin_operator_new) || \
__has_builtin(__builtin_operator_new) < 201802L || \
defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \


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

Re: [libcxx] r339431 - [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

[Hans Wennborg via cfe-commits]

Merged to 7.0 in r339661.

On Fri, Aug 10, 2018 at 3:24 PM, Louis Dionne via cfe-commits
 wrote:
> Author: ldionne
> Date: Fri Aug 10 06:24:56 2018
> New Revision: 339431
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339431&view=rev
> Log:
> [libc++] Enable aligned allocation based on feature test macro, irrespective 
> of standard
>
> Summary:
> The current code enables aligned allocation functions when compiling in C++17
> and later. This is a problem because aligned allocation functions might not
> be supported on the target platform, which leads to an error at link time.
>
> Since r338934, Clang knows not to define __cpp_aligned_new when it's not
> available on the target platform -- this commit takes advantage of that to
> only use aligned allocation functions when they are available.
>
> Reviewers: vsapsai, EricWF
>
> Subscribers: christof, dexonsmith, cfe-commits, EricWF, mclow.lists
>
> Differential Revision: https://reviews.llvm.org/D50344
>
> Added:
> libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp
> Modified:
> libcxx/trunk/include/__config
> libcxx/trunk/include/new
>
> Modified: libcxx/trunk/include/__config
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__config?rev=339431&r1=339430&r2=339431&view=diff
> ==
> --- libcxx/trunk/include/__config (original)
> +++ libcxx/trunk/include/__config Fri Aug 10 06:24:56 2018
> @@ -988,6 +988,11 @@ template  struct __static_asse
>  #  endif
>  #endif // defined(__APPLE__)
>
> +#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
> +!defined(_LIBCPP_BUILDING_LIBRARY) && \
> +(!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606)
> +#  define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
> +#endif
>
>  #if defined(__APPLE__) || defined(__FreeBSD__)
>  #define _LIBCPP_HAS_DEFAULTRUNELOCALE
>
> Modified: libcxx/trunk/include/new
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/new?rev=339431&r1=339430&r2=339431&view=diff
> ==
> --- libcxx/trunk/include/new (original)
> +++ libcxx/trunk/include/new Fri Aug 10 06:24:56 2018
> @@ -108,13 +108,6 @@ void  operator delete[](void* ptr, void*
>  # define _LIBCPP_HAS_NO_SIZED_DEALLOCATION
>  #endif
>
> -#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
> -(!(defined(_LIBCPP_BUILDING_LIBRARY) || _LIBCPP_STD_VER > 14 || \
> -(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
> -# define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
> -#endif
> -
> -
>  #if !__has_builtin(__builtin_operator_new) || \
> __has_builtin(__builtin_operator_new) < 201802L || \
> defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \
>
> Added: libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp?rev=339431&view=auto
> ==
> --- libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp (added)
> +++ libcxx/trunk/test/libcxx/memory/aligned_allocation_macro.pass.cpp Fri Aug 
> 10 06:24:56 2018
> @@ -0,0 +1,25 @@
> +//===--===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is dual licensed under the MIT and the University of Illinois 
> Open
> +// Source Licenses. See LICENSE.TXT for details.
> +//
> +//===--===//
> +
> +// UNSUPPORTED: c++98, c++03, c++11, c++14
> +// XFAIL: with_system_cxx_lib=macosx10.12
> +// XFAIL: with_system_cxx_lib=macosx10.11
> +// XFAIL: with_system_cxx_lib=macosx10.10
> +// XFAIL: with_system_cxx_lib=macosx10.9
> +// XFAIL: with_system_cxx_lib=macosx10.8
> +// XFAIL: with_system_cxx_lib=macosx10.7
> +
> +#include 
> +
> +
> +#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
> +#   error "libc++ should have aligned allocation in C++17 and up when 
> targeting a platform that supports it"
> +#endif
> +
> +int main() { }
>
>
> ___
> 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

r339662 - [XRay][clang] Add more test cases of -fxray-modes= (NFC)

[Dean Michael Berris via cfe-commits]

Author: dberris
Date: Tue Aug 14 02:16:37 2018
New Revision: 339662

URL: http://llvm.org/viewvc/llvm-project?rev=339662&view=rev
Log:
[XRay][clang] Add more test cases of -fxray-modes= (NFC)

This confirms expectations for multiple values provided through the
driver when selecting specific modes and the order of appearance of
individual values for the `-fxray-modes=` flag.

This change just adds more test cases to an existing test file.

Modified:
cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp

Modified: cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp?rev=339662&r1=339661&r2=339662&view=diff
==
--- cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp (original)
+++ cfe/trunk/test/Driver/XRay/xray-mode-flags.cpp Tue Aug 14 02:16:37 2018
@@ -26,6 +26,20 @@
 // RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none,all \
 // RUN: -### %s \
 // RUN: 2>&1 | FileCheck --check-prefixes FDR,BASIC %s
+//
+// We also should support having the individual modes be concatenated.
+//
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=none \
+// RUN: -fxray-modes=xray-fdr \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes FDR %s
+//
+// Order also matters.
+//
+// RUN: %clang -v -o /dev/null -fxray-instrument -fxray-modes=xray-fdr \
+// RUN: -fxray-modes=none \
+// RUN: -### %s \
+// RUN: 2>&1 | FileCheck --check-prefixes NONE %s
 
 // BASIC: libclang_rt.xray-basic
 // FDR: libclang_rt.xray-fdr


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

Re: r339597 - Enforce instantiation of template multiversion functions

[Hans Wennborg via cfe-commits]

Merged to 7.0 in r339663.

On Mon, Aug 13, 2018 at 8:33 PM, Erich Keane via cfe-commits
 wrote:
> Author: erichkeane
> Date: Mon Aug 13 11:33:20 2018
> New Revision: 339597
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339597&view=rev
> Log:
> Enforce instantiation of template multiversion functions
>
> Multiversioned member functions inside of a template type were
> not properly being emitted.  The solution to this is to simply
> ensure that their bodies are correctly evaluated/assigned during
> template instantiation.
>
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> cfe/trunk/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=339597&r1=339596&r2=339597&view=diff
> ==
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Mon Aug 13 11:33:20 2018
> @@ -2743,7 +2743,7 @@ public:
>/// predicate.
>void forEachMultiversionedFunctionVersion(
>const FunctionDecl *FD,
> -  llvm::function_ref Pred) const;
> +  llvm::function_ref Pred) const;
>
>const CXXConstructorDecl *
>getCopyConstructorForExceptionObject(CXXRecordDecl *RD);
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=339597&r1=339596&r2=339597&view=diff
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Aug 13 11:33:20 2018
> @@ -9819,7 +9819,7 @@ bool ASTContext::DeclMustBeEmitted(const
>
>  void ASTContext::forEachMultiversionedFunctionVersion(
>  const FunctionDecl *FD,
> -llvm::function_ref Pred) const {
> +llvm::function_ref Pred) const {
>assert(FD->isMultiVersion() && "Only valid for multiversioned functions");
>llvm::SmallDenseSet SeenDecls;
>FD = FD->getCanonicalDecl();
>
> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=339597&r1=339596&r2=339597&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Mon Aug 13 11:33:20 
> 2018
> @@ -5175,10 +5175,20 @@ void Sema::PerformPendingInstantiations(
>  if (FunctionDecl *Function = dyn_cast(Inst.first)) {
>bool DefinitionRequired = Function->getTemplateSpecializationKind() ==
>  TSK_ExplicitInstantiationDefinition;
> -  InstantiateFunctionDefinition(/*FIXME:*/Inst.second, Function, true,
> -DefinitionRequired, true);
> -  if (Function->isDefined())
> -Function->setInstantiationIsPending(false);
> +  if (Function->isMultiVersion()) {
> +getASTContext().forEachMultiversionedFunctionVersion(
> +Function, [this, Inst, DefinitionRequired](FunctionDecl *CurFD) {
> +  InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, CurFD, 
> true,
> +DefinitionRequired, true);
> +  if (CurFD->isDefined())
> +CurFD->setInstantiationIsPending(false);
> +});
> +  } else {
> +InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, Function, true,
> +  DefinitionRequired, true);
> +if (Function->isDefined())
> +  Function->setInstantiationIsPending(false);
> +  }
>continue;
>  }
>
>
> Modified: cfe/trunk/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-target-mv-member-funcs.cpp?rev=339597&r1=339596&r2=339597&view=diff
> ==
> --- cfe/trunk/test/CodeGenCXX/attr-target-mv-member-funcs.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/attr-target-mv-member-funcs.cpp Mon Aug 13 
> 11:33:20 2018
> @@ -111,13 +111,12 @@ int templ_use() {
>  // CHECK:  call i32 @_ZN5templIiE3fooEi.ifunc
>  // CHECK:  call i32 @_ZN5templIdE3fooEi.ifunc
>
> -
>  // CHECK: define i32 (%struct.templ*, i32)* @_ZN5templIiE3fooEi.resolver() 
> comdat
>  // CHECK: ret i32 (%struct.templ*, i32)* @_ZN5templIiE3fooEi.arch_sandybridge
>  // CHECK: ret i32 (%struct.templ*, i32)* @_ZN5templIiE3fooEi.arch_ivybridge
>  // CHECK: ret i32 (%struct.templ*, i32)* @_ZN5templIiE3fooEi.sse4.2
>  // CHECK: ret i32 (%struct.templ*, i32)* @_ZN5templIiE3fooEi
> -//
> +
>  // CHECK: define i32 (%struct.templ.0*, i32)* @_ZN5templIdE3fooEi

[PATCH] D50627: [clangd] Add a testcase for empty preamble.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In https://reviews.llvm.org/D50627#1196988, @ilya-biryukov wrote:

> Maybe also add a test for find-definition that was broken before? (non-empty 
> preamble -> empty preamble -> bad gotodef that goes to included file instead 
> of the local variable)
>  To have a regression test against similar failures.


I think this patch is in a good scope (for empty preamble). I'd leave it as it 
is. The failure of GoTodefinition test is caused by an inconsistent behavior of 
using `lastBuiltPreamble`/`NewPreamble` in TUScheduler, I will send out a 
separate patch fixing it (based on our discussion).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50627



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

[PATCH] D50695: [clangd] Always use the latest preamble

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, javed.absar.

Fix an inconsistent behavior of using `LastBuiltPreamble`/`NewPreamble`
in TUScheduler (see the test for details), AST should always use
NewPreamble. This patch makes LastBuiltPreamble always point to
NewPreamble.

Preamble rarely fails to build, even there are errors in headers, so we
assume it would not cause performace issue for code completion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50695

Files:
  clangd/TUScheduler.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1021,6 +1021,65 @@
   EXPECT_THAT(*Locations, IsEmpty());
 }
 
+TEST(GoToDefinitoin, WithPreamble) {
+  // Test stragety: AST should always use the latest preamble instead of last
+  // good preamble.
+  MockFSProvider FS;
+  IgnoreDiagnostics DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  auto FooCppUri = URIForFile{FooCpp};
+  // The trigger locations must be the same.
+  Annotations FooWithHeader(R"cpp(#include "fo^o.h")cpp");
+  Annotations FooWithoutHeader(R"cpp(double[[fo^o]]();)cpp");
+
+  FS.Files[FooCpp] = FooWithHeader.code();
+
+  auto FooH = testPath("foo.h");
+  auto FooHUri = URIForFile{FooH};
+  Annotations FooHeader(R"cpp([[]])cpp");
+  FS.Files[FooH] = FooHeader.code();
+
+  runAddDocument(Server, FooCpp, FooWithHeader.code());
+  // GoToDefinition goes to a #include file: the result comes from the 
preamble.
+  Server.findDefinitions(
+  FooCpp, FooWithHeader.point(),
+  [&](llvm::Expected> Loc) {
+ASSERT_TRUE(bool(Loc));
+EXPECT_THAT(*Loc, ElementsAre(Location{FooHUri, FooHeader.range()}));
+  });
+
+  // Only preamble is built, and no AST is built in this request.
+  Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::No);
+  // We build AST here, and it should use the latest preamble rather than the
+  // stale one.
+  Server.findDefinitions(
+  FooCpp, FooWithoutHeader.point(),
+  [&](llvm::Expected> Loc) {
+ASSERT_TRUE(bool(Loc));
+EXPECT_THAT(*Loc,
+ElementsAre(Location{FooCppUri, 
FooWithoutHeader.range()}));
+  });
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Wait for GoToDefinition";
+
+
+  // Reset test environment.
+  runAddDocument(Server, FooCpp, FooWithHeader.code());
+  // Both preamble and AST are built in this request.
+  Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::Yes);
+  // Use the AST being built in above request.
+  Server.findDefinitions(
+  FooCpp, FooWithoutHeader.point(),
+  [&](llvm::Expected> Loc) {
+ASSERT_TRUE(bool(Loc));
+EXPECT_THAT(*Loc,
+ElementsAre(Location{FooCppUri, 
FooWithoutHeader.range()}));
+  });
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Wait for GoToDefinition";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -370,8 +370,8 @@
 bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
 {
   std::lock_guard Lock(Mutex);
-  if (NewPreamble)
-LastBuiltPreamble = NewPreamble;
+  // Always use the NewPreamble.
+  LastBuiltPreamble = NewPreamble;
 }
 // Before doing the expensive AST reparse, we want to release our reference
 // to the old preamble, so it can be freed if there are no other references


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1021,6 +1021,65 @@
   EXPECT_THAT(*Locations, IsEmpty());
 }
 
+TEST(GoToDefinitoin, WithPreamble) {
+  // Test stragety: AST should always use the latest preamble instead of last
+  // good preamble.
+  MockFSProvider FS;
+  IgnoreDiagnostics DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  auto FooCppUri = URIForFile{FooCpp};
+  // The trigger locations must be the same.
+  Annotations FooWithHeader(R"cpp(#include "fo^o.h")cpp");
+  Annotations FooWithoutHeader(R"cpp(double[[fo^o]]();)cpp");
+
+  FS.Files[FooCpp] = FooWithHeader.code();
+
+  auto FooH = testPath("foo.h");
+  auto FooHUri = URIForFile{FooH};
+  Annotations FooHeader(R"cpp([[]])cpp");
+  FS.Files[FooH] = FooHeader.code();
+
+  runAddDocument(Server, FooCpp, FooWithHeader.code());
+  // GoToDefinition goes to a #include file: the result comes from the preamble.
+  S

[clang-tools-extra] r339665 - [clangd] Show non-instantiated decls in signatureHelp

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Tue Aug 14 02:36:32 2018
New Revision: 339665

URL: http://llvm.org/viewvc/llvm-project?rev=339665&view=rev
Log:
[clangd] Show non-instantiated decls in signatureHelp

Summary:
To avoid producing very verbose output in substitutions involving
typedefs, e.g.
  T -> std::vector::iterator
gets turned into an unreadable mess when printed out for libstdc++,
result contains internal types (std::__Vector_iterator<...>) and
expanded well-defined typedefs (std::basic_string).

Until we improve the presentation code in clang, going with
non-instantiated decls looks like a better UX trade-off.

Reviewers: hokein, ioeric, kadircet

Reviewed By: hokein

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=339665&r1=339664&r2=339665&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Aug 14 02:36:32 2018
@@ -714,7 +714,15 @@ public:
"too many arguments");
 SigHelp.activeParameter = static_cast(CurrentArg);
 for (unsigned I = 0; I < NumCandidates; ++I) {
-  const auto &Candidate = Candidates[I];
+  OverloadCandidate Candidate = Candidates[I];
+  // We want to avoid showing instantiated signatures, because they may be
+  // long in some cases (e.g. when 'T' is substituted with 'std::string', 
we
+  // would get 'std::basic_string').
+  if (auto *Func = Candidate.getFunction()) {
+if (auto *Pattern = Func->getTemplateInstantiationPattern())
+  Candidate = OverloadCandidate(Pattern);
+  }
+
   const auto *CCS = Candidate.CreateSignatureString(
   CurrentArg, S, *Allocator, CCTUInfo, true);
   assert(CCS && "Expected the CodeCompletionString to be non-null");

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=339665&r1=339664&r2=339665&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue Aug 14 
02:36:32 2018
@@ -1537,6 +1537,59 @@ TEST(SignatureHelpTest, OverloadsOrderin
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(10, ^);
+})cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T...);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+  // It is debatable whether we should substitute the outer template parameter
+  // ('T') in that case. Currently we don't substitute it in signature help, 
but
+  // do substitute in code complete.
+  // FIXME: make code complete and signature help consistent, figure out which
+  // way is better.
+  EXPECT_THAT(signatures(R"cpp(
+template 
+struct X {
+  template 
+  void foo(T, U);
+};
+
+int main() {
+  X().foo(^)
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D50645: [clangd] Show non-instantiated decls in signatureHelp

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339665: [clangd] Show non-instantiated decls in 
signatureHelp (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D50645

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1537,6 +1537,59 @@
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(10, ^);
+})cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T...);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+  // It is debatable whether we should substitute the outer template parameter
+  // ('T') in that case. Currently we don't substitute it in signature help, 
but
+  // do substitute in code complete.
+  // FIXME: make code complete and signature help consistent, figure out which
+  // way is better.
+  EXPECT_THAT(signatures(R"cpp(
+template 
+struct X {
+  template 
+  void foo(T, U);
+};
+
+int main() {
+  X().foo(^)
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -714,7 +714,15 @@
"too many arguments");
 SigHelp.activeParameter = static_cast(CurrentArg);
 for (unsigned I = 0; I < NumCandidates; ++I) {
-  const auto &Candidate = Candidates[I];
+  OverloadCandidate Candidate = Candidates[I];
+  // We want to avoid showing instantiated signatures, because they may be
+  // long in some cases (e.g. when 'T' is substituted with 'std::string', 
we
+  // would get 'std::basic_string').
+  if (auto *Func = Candidate.getFunction()) {
+if (auto *Pattern = Func->getTemplateInstantiationPattern())
+  Candidate = OverloadCandidate(Pattern);
+  }
+
   const auto *CCS = Candidate.CreateSignatureString(
   CurrentArg, S, *Allocator, CCTUInfo, true);
   assert(CCS && "Expected the CodeCompletionString to be non-null");


Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1537,6 +1537,59 @@
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T, T, T);
+
+int main() {
+  foo(10, ^);
+})cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+  EXPECT_THAT(signatures(R"cpp(
+template 
+void foo(T...);
+
+int main() {
+  foo(^);
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+  // It is debatable whether we should substitute the outer template parameter
+  // ('T') in that case. Currently we don't substitute it in signature help, but
+  // do substitute in code complete.
+  // FIXME: make code complete and signature help consistent, figure out which
+  // way is better.
+  EXPECT_THAT(signatures(R"cpp(
+template 
+struct X {
+  template 
+  void foo(T, U);
+};
+
+int main() {
+  X().foo(^)
+}
+  )cpp")
+  .signatures,
+  ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

[Simon Cook via Phabricator via cfe-commits]

simoncook requested changes to this revision.
simoncook added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: PkmX.

I've tested this, regression tests involving linking now mostly pass as crt0 
can now be found, but it seems that system headers are still being pulled in 
causing a couple of tests to fail. In particular using `limits.h` is causing 
build failures for me.

Compiling with `-v`, I see for `riscv32-unknown-elf-clang`

  #include "..." search starts here:
  #include <...> search starts here:
   
/data/jenkins/workspace/riscv32-llvm-gcc-branches/install/bin/../lib/gcc/riscv32-unknown-elf/8.2.0/../../../../riscv32-unknown-elf/include
   /usr/local/include
   
/data/jenkins/workspace/riscv32-llvm-gcc-branches/install/lib/clang/8.0.0/include
   /usr/include
  End of search list.

whereas for `riscv32-unknown-elf-gcc`:

  #include "..." search starts here:
  #include <...> search starts here:
   
/data/jenkins/workspace/riscv32-llvm-gcc-branches/install/lib/gcc/riscv32-unknown-elf/8.2.0/include
   
/data/jenkins/workspace/riscv32-llvm-gcc-branches/install/lib/gcc/riscv32-unknown-elf/8.2.0/include-fixed
   
/data/jenkins/workspace/riscv32-llvm-gcc-branches/install/lib/gcc/riscv32-unknown-elf/8.2.0/../../../../riscv32-unknown-elf/include
  End of search list.

Can you see why system headers are still being pulled in unless `--sysroot` is 
specified?


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[PATCH] D50438: [clangd] Sort GoToDefinition results.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 160544.
hokein marked 3 inline comments as done.
hokein added a comment.

Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -311,6 +311,50 @@
   }
 }
 
+TEST(GoToDefinition, Rank) {
+  auto T = Annotations(R"cpp(
+struct $foo1[[Foo]] {
+  $foo2[[Foo]]();
+  $foo3[[Foo]](Foo&&);
+  $foo4[[Foo]](const char*);
+};
+
+Foo $f[[f]]();
+
+void $g[[g]](Foo foo);
+
+void call() {
+  const char* $str[[str]] = "123";
+  Foo a = $1^str;
+  Foo b = Foo($2^str);
+  Foo c = $3^f();
+  $4^g($5^f());
+  g($6^str);
+}
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  EXPECT_THAT(findDefinitions(AST, T.point("1")),
+  ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4";
+  EXPECT_THAT(findDefinitions(AST, T.point("2")),
+  ElementsAre(RangeIs(T.range("str";
+  EXPECT_THAT(findDefinitions(AST, T.point("3")),
+  ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3";
+  EXPECT_THAT(findDefinitions(AST, T.point("4")),
+  ElementsAre(RangeIs(T.range("g";
+  EXPECT_THAT(findDefinitions(AST, T.point("5")),
+  ElementsAre(RangeIs(T.range("f")),
+  RangeIs(T.range("foo3";
+
+  auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6"));
+  EXPECT_EQ(3ul, DefinitionAtPoint6.size());
+  EXPECT_THAT(
+  DefinitionAtPoint6,
+  HasSubsequence(RangeIs(T.range("str")), RangeIs(T.range("foo4";
+  EXPECT_THAT(
+  DefinitionAtPoint6,
+  HasSubsequence(RangeIs(T.range("str")), RangeIs(T.range("foo3";
+}
+
 TEST(GoToDefinition, RelPathsInCompileCommand) {
   // The source is in "/clangd-test/src".
   // We build in "/clangd-test/build".
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -68,10 +68,24 @@
   const MacroInfo *Info;
 };
 
+struct DeclInfo {
+  const Decl *D;
+  // Indicates the declaration is referenced by an explicit AST node.
+  bool IsReferencedExplicitly;
+  bool operator==(const DeclInfo &L) {
+return std::tie(D, IsReferencedExplicitly) ==
+   std::tie(L.D, L.IsReferencedExplicitly);
+  }
+};
+
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
-  std::vector Decls;
   std::vector MacroInfos;
+  // The value of the map indicates whether the declaration has been referenced
+  // explicitly in the code.
+  // True means the declaration is explicitly referenced at least once; false
+  // otherwise
+  llvm::DenseMap Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -82,13 +96,19 @@
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  std::vector takeDecls() {
-// Don't keep the same declaration multiple times.
-// This can happen when nodes in the AST are visited twice.
-std::sort(Decls.begin(), Decls.end());
-auto Last = std::unique(Decls.begin(), Decls.end());
-Decls.erase(Last, Decls.end());
-return std::move(Decls);
+  std::vector getDecls() const {
+std::vector Result;
+for (auto It : Decls)
+  Result.push_back({It.first, It.second});
+
+// Sort results. Declarations being referenced explicitly come first.
+std::sort(Result.begin(), Result.end(),
+  [](const DeclInfo &L, const DeclInfo &R) {
+if (L.IsReferencedExplicitly != R.IsReferencedExplicitly)
+  return L.IsReferencedExplicitly > R.IsReferencedExplicitly;
+return L.D < R.D;
+  });
+return Result;
   }
 
   std::vector takeMacroInfos() {
@@ -112,15 +132,30 @@
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 if (Loc == SearchedLocation) {
+  // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr).
+  auto hasImplicitExpr = [](const Expr *E) {
+if (!E || E->child_begin() == E->child_end())
+  return false;
+// Use the first child is good enough for most cases -- normally the
+// expression returned by handleDeclOccurence contains exactly one
+// child expression.
+const auto *FirstChild = *E->child_begin();
+return llvm::isa(FirstChild) ||
+   llvm::isa(FirstChild) ||
+   llvm::isa(FirstChild) ||
+   llvm::isa(FirstChild);
+  };
+
+  bool IsExplicit = !hasImplicitExpr(ASTNode.OrigE);
   // Find and add 

r339667 - Add a stub mangling for ObjC selectors in the Microsoft ABI.

[David Chisnall via cfe-commits]

Author: theraven
Date: Tue Aug 14 03:04:36 2018
New Revision: 339667

URL: http://llvm.org/viewvc/llvm-project?rev=339667&view=rev
Log:
Add a stub mangling for ObjC selectors in the Microsoft ABI.

This mangling is used only for outlined SEH finally blocks, which have
internal linkage.

This fixes the failure of CodeGenObjC/2007-04-03-ObjcEH.m on builds with
expensive checks enabled, on Windows.  This test should probably be
specifying a triple: it currently picks up whatever the host environment
is using.  Unfortunately, I have no idea what it is trying to test,
because it contains no comments and predates Clang having working
Objective-C IR generation.

Modified:
cfe/trunk/lib/AST/MicrosoftMangle.cpp

Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=339667&r1=339666&r2=339667&view=diff
==
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Tue Aug 14 03:04:36 2018
@@ -905,8 +905,14 @@ void MicrosoftCXXNameMangler::mangleUnqu
 
 case DeclarationName::ObjCZeroArgSelector:
 case DeclarationName::ObjCOneArgSelector:
-case DeclarationName::ObjCMultiArgSelector:
-  llvm_unreachable("Can't mangle Objective-C selector names here!");
+case DeclarationName::ObjCMultiArgSelector: {
+  // This is reachable only when constructing an outlined SEH finally
+  // block.  Nothing depends on this mangling and it's used only with
+  // functinos with internal linkage.
+  llvm::SmallString<64> Name;
+  mangleSourceName(Name.str());
+  break;
+}
 
 case DeclarationName::CXXConstructorName:
   if (isStructorDecl(ND)) {


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

r339668 - [gnu-objc] Make selector order deterministic.

[David Chisnall via cfe-commits]

Author: theraven
Date: Tue Aug 14 03:05:25 2018
New Revision: 339668

URL: http://llvm.org/viewvc/llvm-project?rev=339668&view=rev
Log:
[gnu-objc] Make selector order deterministic.

Summary:
This probably fixes PR35277, though there may be other sources of
nondeterminism (this was the only case of iterating over a DenseMap).

It's difficult to provide a test case for this, because it shows up only
on systems with ASLR enabled.

Reviewers: rjmccall

Reviewed By: rjmccall

Subscribers: bmwiedemann, mgrang, cfe-commits

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

Added:
cfe/trunk/test/CodeGenObjC/gnu-deterministic-selectors.m
Modified:
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp

Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=339668&r1=339667&r2=339668&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Tue Aug 14 03:05:25 2018
@@ -3541,12 +3541,16 @@ llvm::Function *CGObjCGNU::ModuleInitFun
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, 
".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelector &sel : table[untypedSel]) {
 llvm::Constant *selectorTypeEncoding = NULLPtr;
 if (!sel.first.empty())
   selectorTypeEncoding =

Added: cfe/trunk/test/CodeGenObjC/gnu-deterministic-selectors.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/gnu-deterministic-selectors.m?rev=339668&view=auto
==
--- cfe/trunk/test/CodeGenObjC/gnu-deterministic-selectors.m (added)
+++ cfe/trunk/test/CodeGenObjC/gnu-deterministic-selectors.m Tue Aug 14 
03:05:25 2018
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 
%s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s 
-emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, 
i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 
0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 
x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* 
getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), 
i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
@.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr 
inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { 
i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+   SEL a = @selector(z);
+   SEL b = @selector(a);
+   SEL c = @selector(g);
+   SEL d = @selector(l);
+   SEL e = @selector(j);
+}


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

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339668: [gnu-objc] Make selector order deterministic. 
(authored by theraven, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50559?vs=160312&id=160545#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50559

Files:
  lib/CodeGen/CGObjCGNU.cpp
  test/CodeGenObjC/gnu-deterministic-selectors.m


Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- test/CodeGenObjC/gnu-deterministic-selectors.m
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 
%s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s 
-emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, 
i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 
0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 
x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* 
getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), 
i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
@.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr 
inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { 
i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+   SEL a = @selector(z);
+   SEL b = @selector(a);
+   SEL c = @selector(g);
+   SEL d = @selector(l);
+   SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, 
".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelector &sel : table[untypedSel]) {
 llvm::Constant *selectorTypeEncoding = NULLPtr;
 if (!sel.first.empty())
   selectorTypeEncoding =


Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- test/CodeGenObjC/gnu-deterministic-selectors.m
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s -emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+	SEL a = @selector(z);
+	SEL b = @selector(a);
+	SEL c = @selector(g);
+	SEL d = @selector(l);
+	SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for

[PATCH] D50594: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check-clang-analyzer and check-clang-analyzer-z3

[Yvan Roux via Phabricator via cfe-commits]

yroux added a comment.

Notice that the affected bots run two times the test cases where one execution 
passes and one fails, as you can see in (search Analysis/plist-macros.cpp for 
instance):

http://lab.llvm.org:8011/builders/clang-cmake-armv8-full/builds/5450/steps/ninja%20check%201/logs/stdio

and I think it is due to llvm-lit being invoked with "--param USE_Z3_SOLVER=0 
--param USE_Z3_SOLVER=0"


Repository:
  rC Clang

https://reviews.llvm.org/D50594



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

[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Looks mostly good.




Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+  // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration 
objects
+  // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+  // absl::FDivDuration(t1, t2);}

deannagarcia wrote:
> hokein wrote:
> > I think we should ignore templates. The template function could be used by 
> > other types, fixing the template is not correct. 
> I removed this template function, but left the template function TakesGeneric 
> below to show that the automatic type does not require/give a warning. Is 
> that alright?
The check will still give warnings in the template instantiation,  I think we 
need `unless(isInTemplateInstantiation()` matcher to filter them out.


https://reviews.llvm.org/D50389



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

[PATCH] D50630: [AST] Update/correct the static_asserts for the bit-fields in Type

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

I actually have patches for TemplateSpecializationType, PackExpansionType,
DependentTemplateSpecializationType and SubstTemplateTypeParmPackType which move
the unsigned to the bit-fields in Type. The same could be done for the others I 
suppose.
This assume that the bit-fields of Type have 64 bits available though.

An other one is in ElaboratedType: the member "TagDecl *OwnedTagDecl" is very
commonly null (IIRC only non-null for about 600 of the 66k ElaboratedTypes when
parsing all of Boost). Therefore we can simply put it in a trailing object
and stash a bit in the bit-fields of Type indicating when this pointer is null.

Bruno


Repository:
  rC Clang

https://reviews.llvm.org/D50630



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

Fwd: patch file for bug 38557

[Owen Pan via cfe-commits]

Hi,

Attached is the patch file for Bug 38557 (
https://bugs.llvm.org/show_bug.cgi?id=38557).

Thanks,
Owen
Index: UnwrappedLineParser.cpp
===
--- UnwrappedLineParser.cpp	(revision 339102)
+++ UnwrappedLineParser.cpp	(working copy)
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50641: [clangd][test] Fix exit messages in tests

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

I think that by introducing different codepath for testing purposes we might 
end up with fewer bugs in tests but the actual production code could become 
less tested. Actually, even the -lit-test itself might be not the theoretically 
most correct approach but I do see the practical reason for it. In general I'd 
rather keep the testing specific code to a minimum though.

What we might be able to do specifically for this case is to return false iff 
we hit unexpected EOF in clangd::runLanguageServerLoop() (currently void) and 
|| it with ShutdownRequestReceived in ClangdLSPServer::run().

I still see some value in monitoring clangd's stderr in tests in general 
though. I can imagine something simple like redirection of clangd's stderr to a 
file and just grepping the log for lines starting with "E[" and in case of some 
set of errors being expected/allowed for specific test case then grepping for 
negation (grep -v expected_error). There might be some work necessary to either 
recalibrate log levels or add allowed errors to large part of tests. For 
example clangd currently logs this error for most tests:

  E[11:24:49.910] Failed to get status for RootPath clangd: No such file or 
directory


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50641



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

Re: Fwd: patch file for bug 38557

[Jonas Toth via cfe-commits]

Do you have a phabricator account? Otherwise I will create a revision
for you (ofc mentioning your name in the commit message) for review.


Am 14.08.2018 um 12:30 schrieb Owen Pan via cfe-commits:
> Hi,
>
> Attached is the patch file for Bug 38557
> (https://bugs.llvm.org/show_bug.cgi?id=38557).
>
> Thanks,
> Owen
>
>
> ___
> 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

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

[Jonas Toth via Phabricator via cfe-commits]

JonasToth created this revision.
JonasToth added reviewers: hokein, krasimir, djasper, klimek.

The Bug was reported and fixed by Owen Pan.


Repository:
  rC Clang

https://reviews.llvm.org/D50697

Files:
  UnwrappedLineParser.cpp


Index: UnwrappedLineParser.cpp
===
--- UnwrappedLineParser.cpp
+++ UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like


Index: UnwrappedLineParser.cpp
===
--- UnwrappedLineParser.cpp
+++ UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49798: [ASTImporter] Adding some friend function related unittests.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 160548.
balazske added a comment.

- Corrected the code in tests.


Repository:
  rC Clang

https://reviews.llvm.org/D49798

Files:
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2275,6 +2275,205 @@
   EXPECT_EQ(ImportedD1->getPreviousDecl(), ImportedD);
 }
 
+TEST_P(ImportFriendFunctions, Lookup) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  Decl *FromTU =
+  getTuDecl("struct X { friend void f(); };", Lang_CXX, "input0.cc");
+  auto *FromD = FirstDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  {
+auto FromName = FromD->getDeclName();
+auto *Class = FirstDeclMatcher().match(FromTU, ClassPattern);
+auto LookupRes = Class->noload_lookup(FromName);
+ASSERT_EQ(LookupRes.size(), 0u);
+LookupRes = cast(FromTU)->noload_lookup(FromName);
+ASSERT_EQ(LookupRes.size(), 1u);
+  }
+
+  auto *ToD = cast(Import(FromD, Lang_CXX));
+  auto ToName = ToD->getDeclName();
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *Class = FirstDeclMatcher().match(ToTU, ClassPattern);
+  auto LookupRes = Class->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 0u);
+  LookupRes = ToTU->noload_lookup(ToName);
+  EXPECT_EQ(LookupRes.size(), 1u);
+
+  EXPECT_EQ(DeclCounter().match(ToTU, FunctionPattern), 1u);
+  auto *To0 = FirstDeclMatcher().match(ToTU, FunctionPattern);
+  EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+}
+
+TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU =
+  getTuDecl("struct X { friend void f(); };"
+// This proto decl makes f available to normal
+// lookup, otherwise it is hidden.
+// Normal C++ lookup (implemented in
+// `clang::Sema::CppLookupName()` and in `LookupDirect()`)
+// returns the found `NamedDecl` only if the set IDNS is matched
+"void f();",
+Lang_CXX, "input0.cc");
+  auto *From0 = FirstDeclMatcher().match(FromTU, FunctionPattern);
+  auto *From1 = LastDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_TRUE(From0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_FALSE(From0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_FALSE(From1->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_TRUE(From1->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  {
+auto Name = From0->getDeclName();
+auto *Class = FirstDeclMatcher().match(FromTU, ClassPattern);
+auto LookupRes = Class->noload_lookup(Name);
+ASSERT_EQ(LookupRes.size(), 0u);
+LookupRes = FromTU->noload_lookup(Name);
+ASSERT_EQ(LookupRes.size(), 1u);
+  }
+
+  auto *To0 = cast(Import(From0, Lang_CXX));
+  auto Name = To0->getDeclName();
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  auto *Class = FirstDeclMatcher().match(ToTU, ClassPattern);
+  auto LookupRes = Class->noload_lookup(Name);
+  EXPECT_EQ(LookupRes.size(), 0u);
+  LookupRes = ToTU->noload_lookup(Name);
+  // Test is disabled because this result is 2.
+  EXPECT_EQ(LookupRes.size(), 1u);
+
+  ASSERT_EQ(DeclCounter().match(ToTU, FunctionPattern), 2u);
+  To0 = FirstDeclMatcher().match(ToTU, FunctionPattern);
+  auto *To1 = LastDeclMatcher().match(ToTU, FunctionPattern);
+  EXPECT_TRUE(To0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  EXPECT_FALSE(To1->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  EXPECT_TRUE(To1->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+}
+
+TEST_P(ImportFriendFunctions, LookupWithProtoBefore) {
+  auto FunctionPattern = functionDecl(hasName("f"));
+  auto ClassPattern = cxxRecordDecl(hasName("X"));
+
+  TranslationUnitDecl *FromTU =
+  getTuDecl("void f();"
+"struct X { friend void f(); };",
+Lang_CXX, "input0.cc");
+  auto *From0 = FirstDeclMatcher().match(FromTU, FunctionPattern);
+  auto *From1 = LastDeclMatcher().match(FromTU, FunctionPattern);
+  ASSERT_FALSE(From0->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_TRUE(From0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_TRUE(From1->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
+  ASSERT_TRUE(From1->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  {
+auto Name = From0->getDeclName();
+auto *Class = FirstDe

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 160550.
JonasToth added a comment.

Using git for the diff, add testcase


Repository:
  rC Clang

https://reviews.llvm.org/D50697

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1112,6 +1112,20 @@
"case 0: return; /* long long long long long long long long 
long long long long comment line */\n"
"}",
Style));
+
+  EXPECT_EQ("switch (n) {\n"
+"  default /*comments*/:\n"
+"return true;\n"
+"  case 0:\n"
+"return false;\n"
+"}",
+format("switch (n) {\n"
+   "  default /*comments*/:\n"
+   "return true;\n"
+   "case 0:\n"
+   "  return false;\n"
+   "}",
+   Style));
   verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1112,6 +1112,20 @@
"case 0: return; /* long long long long long long long long long long long long comment line */\n"
"}",
Style));
+
+  EXPECT_EQ("switch (n) {\n"
+"  default /*comments*/:\n"
+"return true;\n"
+"  case 0:\n"
+"return false;\n"
+"}",
+format("switch (n) {\n"
+   "  default /*comments*/:\n"
+   "return true;\n"
+   "case 0:\n"
+   "  return false;\n"
+   "}",
+   Style));
   verifyFormat("switch (a) {\n"
"#if FOO\n"
"case 0: return 0;\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Fwd: patch file for bug 38557

[Jonas Toth via cfe-commits]

Hi Owen,

thank you for your patch. I created a revision for it to pass review:
https://reviews.llvm.org/D50697

If you do have an phabricator account you can take over, otherwise we
can get it into trunk for you :)

All the best, Jonas


Am 14.08.2018 um 12:30 schrieb Owen Pan via cfe-commits:
> Hi,
>
> Attached is the patch file for Bug 38557
> (https://bugs.llvm.org/show_bug.cgi?id=38557).
>
> Thanks,
> Owen
>
>
> ___
> 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

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: lib/AST/ASTImporter.cpp:2872
 Importer.MapImported(D, FoundField);
+// In case of a FieldDecl of a ClassTemplateSpecializationDecl, the
+// initializer of a FieldDecl might not had been instantiated in the

a_sidorin wrote:
> Honestly speaking, I wonder about this behaviour because it doesn't look 
> similar to instantiation of only methods that are used. Is it a common rule?
Yes, this is a common rule. The instantiation of an initializer is similar to 
the instantiation of default arguments in a sense that both are instantated 
only if they are being used.

To be more precise, quoting from Vandevoorde - C++ Templates The Complete Guide 
/ 14.2.2 Instantiated Components:

... Default functioncallarguments   are considered  
separately  wheninstantiating
templates.  Specifically,   theyare not instantiatedunless  
there   is  a   callto  that
function(or member  function)   thatactuallymakes   
use of  the default argument.
If, on  the other   hand,   the functionis  called  
withexplicitarguments   thatoverride
the default,thenthe default arguments   are not 
instantiated.
Similarly,  exception   specifications  and **default   member  
initializers**  are not
instantiatedunless  theyare needed.





Comment at: lib/AST/ASTImporter.cpp:4550
+  // in the "From" context, but not in the "To" context.
+  for (auto *FromField : D->fields())
+Importer.Import(FromField);

a_sidorin wrote:
> Importing additional fields can change the layout of the specialization. For 
> CSA, this usually results in strange assertions hard to debug. Could you 
> please share the results of testing of this change?
> This change also doesn't seem to have related tests in this patch.
TLDR; We will not create additional fields.

By the time when we import the field, we already know that the existing 
specialization is structurally equivalent with the new one. 
Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, the 
structural equivalence check ensures that they have the exact same fields.
When we import the field of the new spec and if there is an existing FieldDecl 
in the "To" context, then no new FieldDecl will be created (this is handled in 
`VisitFieldDecl` by first doing a lookup of existing field with the same name 
and type).
This patch extends `VisitFieldDecl` in a way that we add new initializer 
expressions to the existing FieldDecl, if it didn't have and in the "From" 
context it has.

For the record, I  added a new test to make sure that a new FieldDecl will not 
be created during the merge.



Comment at: lib/AST/ASTImporter.cpp:4551
+  for (auto *FromField : D->fields())
+Importer.Import(FromField);
+

a_sidorin wrote:
> The result of import is unchecked here and below. Is it intentional?
Yes, that is intentional. We plan to refactor all ASTImporter functions to 
provide a proper error handling mechanism, and in that change we would like to 
enforce the check of all import functions.
Unfortunately, currently we already have many places where we do not check the 
return value of import.


Repository:
  rC Clang

https://reviews.llvm.org/D50451



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

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

[Gabor Marton via Phabricator via cfe-commits]

martong marked 3 inline comments as done.
martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:4551
+  for (auto *FromField : D->fields())
+Importer.Import(FromField);
+

martong wrote:
> a_sidorin wrote:
> > The result of import is unchecked here and below. Is it intentional?
> Yes, that is intentional. We plan to refactor all ASTImporter functions to 
> provide a proper error handling mechanism, and in that change we would like 
> to enforce the check of all import functions.
> Unfortunately, currently we already have many places where we do not check 
> the return value of import.
Actually, having proper error handling is just one thing, the other more 
important reason why we don't check the result of the import here, because we 
don't want to stop merging other fields/functions if one merge failed. If one 
merge failed, other merge operations still could be successful.


Repository:
  rC Clang

https://reviews.llvm.org/D50451



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

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 160552.
martong added a comment.

- Add new test
- Fix indentation


Repository:
  rC Clang

https://reviews.llvm.org/D50451

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2683,6 +2683,263 @@
   EXPECT_EQ(FromIndex, 3u);
 }
 
+TEST_P(ASTImporterTestBase, MergeFieldDeclsOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+  int a{0}; // FieldDecl with InitListExpr
+  X(char) : a(3) {} // (1)
+  X(int) {} // (2)
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(
+  void foo() {
+  // ClassTemplateSpec with ctor (1): FieldDecl without InitlistExpr
+  X xc('c');
+  }
+  )", Lang_CXX11);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  // FieldDecl without InitlistExpr:
+  auto *ToField = *ToSpec->field_begin();
+  ASSERT_TRUE(ToField);
+  ASSERT_FALSE(ToField->getInClassInitializer());
+  Decl *FromTU = getTuDecl(ClassTemplate +
+  R"(
+  void bar() {
+  // ClassTemplateSpec with ctor (2): FieldDecl WITH InitlistExpr
+  X xc(1);
+  }
+  )", Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  // FieldDecl with InitlistExpr:
+  auto *FromField = *FromSpec->field_begin();
+  ASSERT_TRUE(FromField);
+  ASSERT_TRUE(FromField->getInClassInitializer());
+
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+  ASSERT_TRUE(ImportedSpec);
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  // After the import, the FieldDecl has to be merged, thus it should have the
+  // InitListExpr.
+  EXPECT_TRUE(ToField->getInClassInitializer());
+}
+
+TEST_P(ASTImporterTestBase, MergeFunctionOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+void f() {}
+void g() {}
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(
+  void foo() {
+  X x;
+  x.f();
+  }
+  )", Lang_CXX11);
+  Decl *FromTU = getTuDecl(ClassTemplate +
+  R"(
+  void bar() {
+  X x;
+  x.g();
+  }
+  )", Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  auto FunPattern = functionDecl(hasName("g"),
+ hasParent(classTemplateSpecializationDecl()));
+  auto *FromFun =
+  FirstDeclMatcher().match(FromTU, FunPattern);
+  auto *ToFun =
+  FirstDeclMatcher().match(ToTU, FunPattern);
+  ASSERT_TRUE(FromFun->hasBody());
+  ASSERT_FALSE(ToFun->hasBody());
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+  ASSERT_TRUE(ImportedSpec);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  EXPECT_TRUE(ToFun->hasBody());
+}
+
+TEST_P(ASTImporterTestBase,
+   ODRViolationOfClassTemplateSpecializationsShouldBeReported) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {};
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+   R"(
+  template <>
+  struct X {
+  int a;
+  };
+  void foo() {
+  X x;
+  }
+  )",
+   Lang_CXX11);
+  Decl *FromTU = getTuDecl(ClassTemplate +
+   R"(
+  template <>
+  struct X {
+  int b;
+  };
+  void foo() {
+  X x;
+  }
+  )",
+   Lang_CXX11);
+  auto *FromSpec = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl(hasName("X")));
+  auto *ImportedSpec = Import(FromSpec, Lang_CXX11);
+
+  // We expect one (ODR) warning during the import.
+  EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+
+  // The second specialization is different from the first, thus it violates
+  // ODR, consequently we expect to keep the first specialization only, which is
+  // already in the "To" context.
+  EXPECT_TRUE(ImportedSpec);
+  auto *ToSpec = FirstDeclMatcher().match(
+  ToTU, classTemplateSpecializationDecl(hasName("X")));
+  EXPECT_EQ(ImportedSpec, ToSpec);
+  EXPECT_EQ(1u, DeclCounter().match(
+ToTU, classTemplateSpecializationDecl()));
+}
+
+TEST_P(ASTImporterTestBase, MergeCtorOfClassTemplateSpecialization) {
+  std::string ClassTemplate =
+  R"(
+  template 
+  struct X {
+  X(char) {}
+  X(int) {}
+  };
+  )";
+  Decl *ToTU = getToTuDecl(ClassTemplate +
+  R"(

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:4550
+  // in the "From" context, but not in the "To" context.
+  for (auto *FromField : D->fields())
+Importer.Import(FromField);

martong wrote:
> a_sidorin wrote:
> > Importing additional fields can change the layout of the specialization. 
> > For CSA, this usually results in strange assertions hard to debug. Could 
> > you please share the results of testing of this change?
> > This change also doesn't seem to have related tests in this patch.
> TLDR; We will not create additional fields.
> 
> By the time when we import the field, we already know that the existing 
> specialization is structurally equivalent with the new one. 
> Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, the 
> structural equivalence check ensures that they have the exact same fields.
> When we import the field of the new spec and if there is an existing 
> FieldDecl in the "To" context, then no new FieldDecl will be created (this is 
> handled in `VisitFieldDecl` by first doing a lookup of existing field with 
> the same name and type).
> This patch extends `VisitFieldDecl` in a way that we add new initializer 
> expressions to the existing FieldDecl, if it didn't have and in the "From" 
> context it has.
> 
> For the record, I  added a new test to make sure that a new FieldDecl will 
> not be created during the merge.
This is the new test: 
`ODRViolationOfClassTemplateSpecializationsShouldBeReported`. It checks that it 
is not possible to add new fields to a specialization, rather an ODR violation 
is diagnosed.


Repository:
  rC Clang

https://reviews.llvm.org/D50451



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

[PATCH] D50699: Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a project: clang.

See bug report https://bugs.llvm.org/show_bug.cgi?id=38525 for more details.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

Files:
  lib/Format/ContinuationIndenter.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50700: [clangd] Generate better incomplete bigrams for the Dex index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay.

Currently, the query trigram generator would simply yield `u_p` trigram for the 
`u_p` query. This is not optimal, since the user is likely to try matching two 
heads with this query and this patch addresses the issue.


https://reviews.llvm.org/D50700

Files:
  clang-tools-extra/clangd/index/dex/Trigram.cpp
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -300,6 +300,8 @@
   EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
   EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
 
+  EXPECT_THAT(generateQueryTrigrams("u_p"), trigramsAre({"up$"}));
+
   EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
   EXPECT_THAT(generateQueryTrigrams("clangd"),
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -68,7 +68,10 @@
 ///
 /// For short queries (less than 3 characters with Head or Tail roles in Fuzzy
 /// Matching segmentation) this returns a single trigram with the first
-/// characters (up to 3) to perfrom prefix match.
+/// characters (up to 3) to perfrom prefix match. However, if the query is 
short
+/// but it contains two HEAD symbols then the returned trigram would be an
+/// incomplete bigram with those two headsd. This would help to match
+/// "unique_ptr" and similar symbols with "u_p" query.
 std::vector generateQueryTrigrams(llvm::StringRef Query);
 
 } // namespace dex
Index: clang-tools-extra/clangd/index/dex/Trigram.cpp
===
--- clang-tools-extra/clangd/index/dex/Trigram.cpp
+++ clang-tools-extra/clangd/index/dex/Trigram.cpp
@@ -128,9 +128,15 @@
   // Additional pass is necessary to count valid identifier characters.
   // Depending on that, this function might return incomplete trigram.
   unsigned ValidSymbolsCount = 0;
-  for (size_t I = 0; I < Roles.size(); ++I)
-if (Roles[I] == Head || Roles[I] == Tail)
+  unsigned Heads = 0;
+  for (size_t I = 0; I < Roles.size(); ++I) {
+if (Roles[I] == Head) {
+  ++ValidSymbolsCount;
+  ++Heads;
+} else if (Roles[I] == Tail) {
   ++ValidSymbolsCount;
+}
+  }
 
   std::string LowercaseQuery = Query.lower();
 
@@ -140,13 +146,26 @@
   // If the number of symbols which can form fuzzy matching trigram is not
   // sufficient, generate a single incomplete trigram for query.
   if (ValidSymbolsCount < 3) {
-std::string Symbols = {{END_MARKER, END_MARKER, END_MARKER}};
+std::string Symbols;
+// If the query is not long enough to form a trigram but contains two heads
+// the returned trigram should be "xy$" where "x" and "y" are the heads.
+// This might be particulary important for cases like "u_p" to match
+// "unique_ptr" and similar symbols from the C++ Standard Library.
+if (Heads == 2) {
+  for (size_t I = 0; I < LowercaseQuery.size(); ++I)
+if (Roles[I] == Head)
+  Symbols += LowercaseQuery[I];
+
+  Symbols += END_MARKER;
+} else {
+  Symbols = {{END_MARKER, END_MARKER, END_MARKER}};
   if (LowercaseQuery.size() > 0)
 Symbols[0] = LowercaseQuery[0];
   if (LowercaseQuery.size() > 1)
 Symbols[1] = LowercaseQuery[1];
   if (LowercaseQuery.size() > 2)
 Symbols[2] = LowercaseQuery[2];
+}
 const auto Trigram = Token(Token::Kind::Trigram, Symbols);
 UniqueTrigrams.insert(Trigram);
   } else {


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -300,6 +300,8 @@
   EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
   EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
 
+  EXPECT_THAT(generateQueryTrigrams("u_p"), trigramsAre({"up$"}));
+
   EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
   EXPECT_THAT(generateQueryTrigrams("clangd"),
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -68,7 +68,10 @@
 ///
 /// For short queries (less than 3 characters with Head or Tail roles in Fuzzy
 /// Matching segmentation) this returns a single trigram with the first
-/// characters (up to 3) 

[PATCH] D50700: [clangd] Generate better incomplete bigrams for the Dex index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 160555.
kbobyrev added a comment.

Treat leading underscores as additional signals and don't extract two heads in 
that case.


https://reviews.llvm.org/D50700

Files:
  clang-tools-extra/clangd/index/dex/Trigram.cpp
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -321,6 +321,9 @@
   EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
   EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
 
+  EXPECT_THAT(generateQueryTrigrams("u_p"), trigramsAre({"up$"}));
+  EXPECT_THAT(generateQueryTrigrams("_u_p"), trigramsAre({"_u_"}));
+
   EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
   EXPECT_THAT(generateQueryTrigrams("clangd"),
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -62,7 +62,11 @@
 ///
 /// For short queries (less than 3 characters with Head or Tail roles in Fuzzy
 /// Matching segmentation) this returns a single trigram with the first
-/// characters (up to 3) to perfrom prefix match.
+/// characters (up to 3) to perfrom prefix match. However, if the query is 
short
+/// but it contains two HEAD symbols then the returned trigram would be an
+/// incomplete bigram with those two HEADs (unless query starts with '_' which
+/// is treated as an additional information). This would help to match
+/// "unique_ptr" and similar symbols with "u_p" query
 std::vector generateQueryTrigrams(llvm::StringRef Query);
 
 } // namespace dex
Index: clang-tools-extra/clangd/index/dex/Trigram.cpp
===
--- clang-tools-extra/clangd/index/dex/Trigram.cpp
+++ clang-tools-extra/clangd/index/dex/Trigram.cpp
@@ -116,21 +116,39 @@
 
   // Additional pass is necessary to count valid identifier characters.
   // Depending on that, this function might return incomplete trigram.
+  unsigned Heads = 0;
   unsigned ValidSymbolsCount = 0;
-  for (size_t I = 0; I < Roles.size(); ++I)
-if (Roles[I] == Head || Roles[I] == Tail)
+  for (size_t I = 0; I < Roles.size(); ++I) {
+if (Roles[I] == Head) {
+  ++ValidSymbolsCount;
+  ++Heads;
+} else if (Roles[I] == Tail) {
   ++ValidSymbolsCount;
+}
+  }
 
   std::string LowercaseQuery = Query.lower();
 
   DenseSet UniqueTrigrams;
 
   // If the number of symbols which can form fuzzy matching trigram is not
   // sufficient, generate a single incomplete trigram for query.
   if (ValidSymbolsCount < 3) {
-std::string Chars =
-LowercaseQuery.substr(0, std::min(3UL, Query.size()));
-Chars.append(3 - Chars.size(), END_MARKER);
+std::string Chars;
+// If the query is not long enough to form a trigram but contains two heads
+// the returned trigram should be "xy$" where "x" and "y" are the heads.
+// This might be particulary important for cases like "u_p" to match
+// "unique_ptr" and similar symbols from the C++ Standard Library.
+if (Heads == 2 && !Query.startswith("_")) {
+  for (size_t I = 0; I < LowercaseQuery.size(); ++I)
+if (Roles[I] == Head)
+  Chars += LowercaseQuery[I];
+
+  Chars += END_MARKER;
+} else {
+  Chars = LowercaseQuery.substr(0, std::min(3UL, Query.size()));
+  Chars.append(3 - Chars.size(), END_MARKER);
+}
 UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
   } else {
 std::deque Chars;


Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -321,6 +321,9 @@
   EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
   EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
 
+  EXPECT_THAT(generateQueryTrigrams("u_p"), trigramsAre({"up$"}));
+  EXPECT_THAT(generateQueryTrigrams("_u_p"), trigramsAre({"_u_"}));
+
   EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
   EXPECT_THAT(generateQueryTrigrams("clangd"),
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -62,7 +62,11 @@
 ///
 /// For short queries (less than 3 characters with Head or Tail roles in Fuzzy
 /// Matching segmentation) this returns a single trigram with the first
-/// characters (up to 3) to perfrom prefix match.
+/// characters (up to 3) to perfrom prefix match. However, if the query is s

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:1317
+  for (auto *D : FromRD->decls()) {
+Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD));

Is it sure that `ToD` will never be a nullptr?
I think, `removeDecl` or `addDeclInternal` below may crash if we call it with a 
nullptr.
Also in the assert, `ToD->getDeclContext()` seems achy if `ToD` is a nullptr.


Repository:
  rC Clang

https://reviews.llvm.org/D44100



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

[PATCH] D50699: Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Test case:
F6937162: BreakBeforeBinaryOperators.cpp 


Repository:
  rC Clang

https://reviews.llvm.org/D50699



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

[PATCH] D50702: [clangd] NFC: Cleanup clangd help message

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay.

Add missed space, fix a typo.


https://reviews.llvm.org/D50702

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -147,7 +147,7 @@
 static llvm::cl::opt EnableIndex(
 "index",
 llvm::cl::desc("Enable index-based features such as global code completion 
"
-   "and searching for symbols."
+   "and searching for symbols. "
"Clang uses an index built from symbols in opened files"),
 llvm::cl::init(true));
 
@@ -160,7 +160,7 @@
 static llvm::cl::opt HeaderInsertionDecorators(
 "header-insertion-decorators",
 llvm::cl::desc("Prepend a circular dot or space before the completion "
-   "label, depending on wether "
+   "label, depending on whether "
"an include line will be inserted or not."),
 llvm::cl::init(true));
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -147,7 +147,7 @@
 static llvm::cl::opt EnableIndex(
 "index",
 llvm::cl::desc("Enable index-based features such as global code completion "
-   "and searching for symbols."
+   "and searching for symbols. "
"Clang uses an index built from symbols in opened files"),
 llvm::cl::init(true));
 
@@ -160,7 +160,7 @@
 static llvm::cl::opt HeaderInsertionDecorators(
 "header-insertion-decorators",
 llvm::cl::desc("Prepend a circular dot or space before the completion "
-   "label, depending on wether "
+   "label, depending on whether "
"an include line will be inserted or not."),
 llvm::cl::init(true));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50702: [clangd] NFC: Cleanup clangd help message

[Eric Liu via Phabricator via cfe-commits]

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

lg


https://reviews.llvm.org/D50702



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

[PATCH] D47987: Provide only one declaration of __throw_runtime_error

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.
Herald added a subscriber: ldionne.

Any updates on this one? I like @mclow.lists idea of removing it from __locale.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47987



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

[clang-tools-extra] r339673 - [clangd] NFC: Cleanup clangd help message

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Tue Aug 14 05:00:39 2018
New Revision: 339673

URL: http://llvm.org/viewvc/llvm-project?rev=339673&view=rev
Log:
[clangd] NFC: Cleanup clangd help message

Add missed space, fix a typo.

Reviewed by: ioeric

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

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=339673&r1=339672&r2=339673&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Aug 14 05:00:39 2018
@@ -147,7 +147,7 @@ static llvm::cl::opt InputMirrorFi
 static llvm::cl::opt EnableIndex(
 "index",
 llvm::cl::desc("Enable index-based features such as global code completion 
"
-   "and searching for symbols."
+   "and searching for symbols. "
"Clang uses an index built from symbols in opened files"),
 llvm::cl::init(true));
 
@@ -160,7 +160,7 @@ static llvm::cl::opt
 static llvm::cl::opt HeaderInsertionDecorators(
 "header-insertion-decorators",
 llvm::cl::desc("Prepend a circular dot or space before the completion "
-   "label, depending on wether "
+   "label, depending on whether "
"an include line will be inserted or not."),
 llvm::cl::init(true));
 


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

[PATCH] D50702: [clangd] NFC: Cleanup clangd help message

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE339673: [clangd] NFC: Cleanup clangd help message 
(authored by omtcyfz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50702?vs=160557&id=160558#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50702

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -147,7 +147,7 @@
 static llvm::cl::opt EnableIndex(
 "index",
 llvm::cl::desc("Enable index-based features such as global code completion 
"
-   "and searching for symbols."
+   "and searching for symbols. "
"Clang uses an index built from symbols in opened files"),
 llvm::cl::init(true));
 
@@ -160,7 +160,7 @@
 static llvm::cl::opt HeaderInsertionDecorators(
 "header-insertion-decorators",
 llvm::cl::desc("Prepend a circular dot or space before the completion "
-   "label, depending on wether "
+   "label, depending on whether "
"an include line will be inserted or not."),
 llvm::cl::init(true));
 


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -147,7 +147,7 @@
 static llvm::cl::opt EnableIndex(
 "index",
 llvm::cl::desc("Enable index-based features such as global code completion "
-   "and searching for symbols."
+   "and searching for symbols. "
"Clang uses an index built from symbols in opened files"),
 llvm::cl::init(true));
 
@@ -160,7 +160,7 @@
 static llvm::cl::opt HeaderInsertionDecorators(
 "header-insertion-decorators",
 llvm::cl::desc("Prepend a circular dot or space before the completion "
-   "label, depending on wether "
+   "label, depending on whether "
"an include line will be inserted or not."),
 llvm::cl::init(true));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Fwd: patch file for bug 38557

[Owen Pan via cfe-commits]

Hi Jonas,

Thank you very much! I just created a phabricator account and created a
revision for another bug (https://reviews.llvm.org/D50699). I don't know
exactly how to add my test case like you did for me, so I just uploaded it.
I also added the reviewers listed by bugs.llvm.org.

How do I "take over" the revision you created for me?

Thanks,
Owen

On Tue, Aug 14, 2018 at 4:16 AM Jonas Toth 
wrote:

> Hi Owen,
>
> thank you for your patch. I created a revision for it to pass review:
> https://reviews.llvm.org/D50697
>
> If you do have an phabricator account you can take over, otherwise we can
> get it into trunk for you :)
>
> All the best, Jonas
>
> Am 14.08.2018 um 12:30 schrieb Owen Pan via cfe-commits:
>
> Hi,
>
> Attached is the patch file for Bug 38557 (
> https://bugs.llvm.org/show_bug.cgi?id=38557).
>
> Thanks,
> Owen
>
>
> ___
> cfe-commits mailing 
> listcfe-comm...@lists.llvm.orghttp://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

[PATCH] D50699: Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Please create the diffs with full context (git diff -U 999) and add an 
test-case.


Repository:
  rC Clang

https://reviews.llvm.org/D50699



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

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

In https://reviews.llvm.org/D50699#1198770, @owenpan wrote:

> Test case:
>  F6937162: BreakBeforeBinaryOperators.cpp 


Please add a testcase in the source and add checks to verify the changes.


Repository:
  rC Clang

https://reviews.llvm.org/D50699



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

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Aleksei,

Thank you for this patch.
With Balazs, we are working on something similar, but with a different, fine 
grained error value mechanism. Unfortunately we were not aware of that you have 
been working on error handling, and we didn't say that we are working on error 
handling recently, I am terribly sorry about this.

From the CSA perspective, we realized that there may be several different error 
cases which has to be handled differently in `CrossTranslationUnit.cpp`.
For example, there are unsupported constructs which we do not support to import 
(like a struct definition as a parameter of a function).
Another example is when there is a name conflict between the decl in the "From" 
context and the decl in the "To" context, this usually means an ODR error.
We have to handle these errors in a different way after we imported one 
function during CTU analysis.
The fact that there may be more than one kind of errors yields for the use of 
the designated LLVM types: `Error` and `Expected`. A simple `Optional` is 
probably not generic enough.

I find the `importNonNull` and generally the new family of `import` functions 
useful, but I am not sure how they could cooperate with `Expected`. 
Especially, I have some concerns with output parameters.
If we have an Expected as a result type, then there is no way to acquire the 
T if there was an error. However, when we have output parameters, then even if 
there was an error some output params could have been set ... and those can be 
reused even after the return. If error handling is not proper on the callee 
site then we may continue with stale values, which is not possible if we use 
Expected as a return value.

Do you think we can hold back this patch for a few days until Balazs prepares 
the `Expected` based version? Then I think we could compare the patches and 
we could merge the best from the two of them.


Repository:
  rC Clang

https://reviews.llvm.org/D50672



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

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Updated patch created by "svn diff --diff-cmd=diff -x -U99"


Repository:
  rC Clang

https://reviews.llvm.org/D50699

Files:
  ContinuationIndenter.cpp


Index: ContinuationIndenter.cpp
===
--- ContinuationIndenter.cpp
+++ ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: ContinuationIndenter.cpp
===
--- ContinuationIndenter.cpp
+++ ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Do you think we can hold back this patch for a few days until Balazs prepares 
> the Expected based version? Then I think we could compare the patches and 
> we could merge the best from the two of them.

I mean, once Balazs is also ready, then we can create a combined patch which 
holds the good things from both of yours and his patches.


Repository:
  rC Clang

https://reviews.llvm.org/D50672



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

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D50699#1198813, @owenpan wrote:

> Updated patch created by "svn diff --diff-cmd=diff -x -U99"


The patch seems not to be in the correct path (the `lib/Format` thing is 
missing). Could you check that again? If you plan to add more patches you could 
also have a look at `arc` which handles all of that for you :)


Repository:
  rC Clang

https://reviews.llvm.org/D50699



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

[PATCH] D50674: [libc++] Add missing #include in C11 features tests

[Aditya Kumar via Phabricator via cfe-commits]

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

Good catch!


Repository:
  rCXX libc++

https://reviews.llvm.org/D50674



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160562.
hugoeg marked an inline comment as done.

https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-fake-declarations.h
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "abseil-fake-declarations.h"
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+  void OpeningNamespace() {
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  }
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/abseil-fake-declarations.h
===
--- test/clang-tidy/abseil-fake-declarations.h
+++ test/clang-tidy/abseil-fake-declarations.h
@@ -0,0 +1,24 @@
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1) {
+  return s1;
+}
+class SomeContainer {
+
+};
+namespace strings_internal {
+  void InternalFunction(){}
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+  struct InternalStruct{};
+} // namespace container_internal
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to 
+depend upon it beaucse it’s an implementation detail. They cannot friend it, 
+include it, you mention it or refer to it in any way. Doing so violates 
+Abseil's compatibility guidelines and may result in breakage. See 
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+  absl::strings_internal::foo();
+  class foo{
+friend struct absl::container_internal::faa;
+  };
+  absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Updated the patch to include lib/Format path prefix.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

Files:
  lib/Format/ContinuationIndenter.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D50652#1197775, @rnk wrote:

> I'd prefer not to do this, since internal_linkage generates smaller, more 
> debuggable code by default.


IIUC, this is what we had before though, so it's a conservative "revert to 
safety" approach until a better solution can be found.

> I think the symbol table size increase may be specific to MachO, and it may 
> be possible to work around this by changing ld64 to pool strings for symbols 
> by default. I don't know enough about MachO to say if this is possible.

I looked some more at my local build. The sum of object file sizes didn't 
increase very much, the big growth is for the executable, and there it's mostly 
the string table as your analysis showed.

As far as I understand, string pooling should work fine for Mach-O; LLVM does 
it for object files already. And yes, ld64 doesn't do it. I looked at this 
symbol:

  
__ZNSt3__1eqERKNS_21__tree_const_iteratorIN7testing11ExpectationEPNS_11__tree_nodeIS2_PvEElEES9_

If I understand correctly that function would previously have been inlined 
everywhere, but now it occurs 20+ times in the symbol table of base_unittests, 
and yes it's repeated 20+ times in the string table too :-/

> I also think the symbol table size problem may be limited to "large" users of 
> C++: people with 500+ object files using libc++ in a DSO. I'm more 
> comfortable imposing a cost on these users to ask them to opt in to some 
> setting that disables internal_linkage and always_inline than I am leaving 
> always_inline on by default, which adversely affects all users.

Sure, if there's a macro we can use to opt in to get the old inlining 
behaviour, or maybe even better getting linkonce_odr linkage, I'd be a happy 
camper. But the cost of just growing the executables this much seems like a 
real problem for our build infrastructure.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50652



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

[PATCH] D50341: [libcxx] Fix XFAILs for aligned allocation tests on older OSX versions

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D50341#1198339, @vsapsai wrote:

> What about defining a feature for unsupported configurations? I've tried
>
>   if '__cpp_aligned_new' not in macros or \
>   intMacroValue(macros['__cpp_aligned_new']) < 201606:
>   self.config.available_features.add('libcpp-no-aligned-new')
>   
>
> and `// UNSUPPORTED: libcpp-no-aligned-new`. Seems to be working but it's not 
> sufficient. For example, clang-6 for old macOS versions would define 
> `__cpp_aligned_new` but a test would fail. Still, we can use another feature, 
> not necessarily macro-based. Though probably something like 
> `libcpp-no-aligned-new` can replace `// UNSUPPORTED: c++98, c++03, c++11, 
> c++14`.


I thought about something like this, but it would only be useful in a couple of 
places, and I still don't understand why the tests are marked as unsupported on 
some compilers at all. Also, there's already a feature called 
`no-aligned-alloc`, which tests whether `-faligned-alloc` is supported, so we 
should avoid confusing both.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50341



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

[libcxx] r339675 - [libc++] Add missing #include in C11 features tests

[Louis Dionne via cfe-commits]

Author: ldionne
Date: Tue Aug 14 06:29:17 2018
New Revision: 339675

URL: http://llvm.org/viewvc/llvm-project?rev=339675&view=rev
Log:
[libc++] Add missing #include in C11 features tests

Summary:
These #includes are quite important, since otherwise any

#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)

checks are always false, and so we don't actually test for C11 support
in the standard library.

Reviewers: mclow.lists, EricWF

Subscribers: christof, dexonsmith, cfe-commits

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

Modified:
libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp

libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp

Modified: libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp?rev=339675&r1=339674&r2=339675&view=diff
==
--- libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp 
(original)
+++ libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp Tue Aug 
14 06:29:17 2018
@@ -14,6 +14,9 @@
 // _LIBCPP_HAS_C11_FEATURES - which is defined in <__config>
 // They should always be the same
 
+#include <__config>
+#include "test_macros.h"
+
 #ifdef TEST_HAS_C11_FEATURES
 # ifndef _LIBCPP_HAS_C11_FEATURES
 #  error "TEST_HAS_C11_FEATURES is defined, but _LIBCPP_HAS_C11_FEATURES is 
not"

Modified: libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp?rev=339675&r1=339674&r2=339675&view=diff
==
--- libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp (original)
+++ libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp Tue Aug 14 
06:29:17 2018
@@ -11,6 +11,8 @@
 
 #include 
 
+#include "test_macros.h"
+
 #ifndef FLT_ROUNDS
 #error FLT_ROUNDS not defined
 #endif

Modified: 
libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp?rev=339675&r1=339674&r2=339675&view=diff
==
--- 
libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp 
Tue Aug 14 06:29:17 2018
@@ -11,6 +11,8 @@
 
 #include 
 
+#include "test_macros.h"
+
 #ifndef FLT_ROUNDS
 #error FLT_ROUNDS not defined
 #endif


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

[PATCH] D50674: [libc++] Add missing #include in C11 features tests

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339675: [libc++] Add missing #include in C11 features tests 
(authored by ldionne, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50674?vs=160475&id=160567#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50674

Files:
  libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
  libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
  libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp


Index: 
libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
===
--- 
libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
+++ 
libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
@@ -11,6 +11,8 @@
 
 #include 
 
+#include "test_macros.h"
+
 #ifndef FLT_ROUNDS
 #error FLT_ROUNDS not defined
 #endif
Index: libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
===
--- libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
+++ libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
@@ -11,6 +11,8 @@
 
 #include 
 
+#include "test_macros.h"
+
 #ifndef FLT_ROUNDS
 #error FLT_ROUNDS not defined
 #endif
Index: libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
===
--- libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
+++ libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
@@ -14,6 +14,9 @@
 // _LIBCPP_HAS_C11_FEATURES - which is defined in <__config>
 // They should always be the same
 
+#include <__config>
+#include "test_macros.h"
+
 #ifdef TEST_HAS_C11_FEATURES
 # ifndef _LIBCPP_HAS_C11_FEATURES
 #  error "TEST_HAS_C11_FEATURES is defined, but _LIBCPP_HAS_C11_FEATURES is 
not"


Index: libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
===
--- libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
+++ libcxx/trunk/test/std/language.support/support.limits/c.limits/cfloat.pass.cpp
@@ -11,6 +11,8 @@
 
 #include 
 
+#include "test_macros.h"
+
 #ifndef FLT_ROUNDS
 #error FLT_ROUNDS not defined
 #endif
Index: libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
===
--- libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
+++ libcxx/trunk/test/std/depr/depr.c.headers/float_h.pass.cpp
@@ -11,6 +11,8 @@
 
 #include 
 
+#include "test_macros.h"
+
 #ifndef FLT_ROUNDS
 #error FLT_ROUNDS not defined
 #endif
Index: libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
===
--- libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
+++ libcxx/trunk/test/libcxx/language.support/has_c11_features.pass.cpp
@@ -14,6 +14,9 @@
 //		_LIBCPP_HAS_C11_FEATURES - which is defined in <__config>
 //	They should always be the same
 
+#include <__config>
+#include "test_macros.h"
+
 #ifdef TEST_HAS_C11_FEATURES
 # ifndef _LIBCPP_HAS_C11_FEATURES
 #  error "TEST_HAS_C11_FEATURES is defined, but _LIBCPP_HAS_C11_FEATURES is not"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50703: [clangd] NFC: Mark Workspace Symbol feature complete in the documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay.

Workspace Symbol implementation was introduced in 
https://reviews.llvm.org/D44882 and should be complete now.


https://reviews.llvm.org/D50703

Files:
  clang-tools-extra/docs/clangd.rst


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -64,7 +64,7 @@
 | Completion  | Yes|   Yes|
 +-++--+
 | Diagnostics | Yes|   Yes|
-+-++--+ 
++-++--+
 | Fix-its | Yes|   Yes|
 +-++--+
 | Go to Definition| Yes|   Yes|
@@ -83,7 +83,7 @@
 +-++--+
 | Document Symbols| Yes|   Yes|
 +-++--+
-| Workspace Symbols   | Yes|   No |
+| Workspace Symbols   | Yes|   Yes|
 +-++--+
 | Syntax and Semantic Coloring| No |   No |
 +-++--+


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -64,7 +64,7 @@
 | Completion  | Yes|   Yes|
 +-++--+
 | Diagnostics | Yes|   Yes|
-+-++--+ 
++-++--+
 | Fix-its | Yes|   Yes|
 +-++--+
 | Go to Definition| Yes|   Yes|
@@ -83,7 +83,7 @@
 +-++--+
 | Document Symbols| Yes|   Yes|
 +-++--+
-| Workspace Symbols   | Yes|   No |
+| Workspace Symbols   | Yes|   Yes|
 +-++--+
 | Syntax and Semantic Coloring| No |   No |
 +-++--+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

If we think the symbol/string table size increase is acceptable for most 
user's, I suppose Chromium could just do

  -D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_ALWAYS_INLINE

to get back the old behaviour and unblock us? And this could also be suggested 
in the release notes as a work-around for large projects that don't want the 
size increase?


Repository:
  rCXX libc++

https://reviews.llvm.org/D50652



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

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D50652#1198885, @hans wrote:

> Oh, or could we do
>
>   -D_LIBCPP_HIDE_FROM_ABI=
>
>
> and just get regular odr linkage for these functions?


No, you do need to use `_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` because of the 
issue described in 
http://lists.llvm.org/pipermail/llvm-dev/2018-July/124549.html. But yeah, 
Chromium could use this workaround.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50652



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

r339678 - [OpenCL] Add test for constant sampler argument

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Tue Aug 14 06:56:52 2018
New Revision: 339678

URL: http://llvm.org/viewvc/llvm-project?rev=339678&view=rev
Log:
[OpenCL] Add test for constant sampler argument

Modified:
cfe/trunk/test/SemaOpenCL/sampler_t.cl

Modified: cfe/trunk/test/SemaOpenCL/sampler_t.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/sampler_t.cl?rev=339678&r1=339677&r2=339678&view=diff
==
--- cfe/trunk/test/SemaOpenCL/sampler_t.cl (original)
+++ cfe/trunk/test/SemaOpenCL/sampler_t.cl Tue Aug 14 06:56:52 2018
@@ -33,6 +33,8 @@ constant sampler_t glb_smp9 = 0x1000
 
 void foo(sampler_t); // expected-note{{passing argument to parameter here}}
 
+void constant_sampler(constant sampler_t s); // expected-error{{parameter may 
not be qualified with an address space}}
+
 constant struct sampler_s {
   sampler_t smp; // expected-error{{the 'sampler_t' type cannot be used to 
declare a structure or union field}}
 } sampler_str = {0};


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

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D50652#1198893, @ldionne wrote:

> In https://reviews.llvm.org/D50652#1198885, @hans wrote:
>
> > Oh, or could we do
> >
> >   -D_LIBCPP_HIDE_FROM_ABI=
> >
> >
> > and just get regular odr linkage for these functions?
>
>
> No, you do need to use `_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` because of the 
> issue described in 
> http://lists.llvm.org/pipermail/llvm-dev/2018-July/124549.html. But yeah, 
> Chromium could use this workaround.


Actually, scratch that, it does work. One can either use 
`-D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` to restore the 
old behavior, or `-D_LIBCPP_HIDE_FROM_ABI=` to get odr linkage.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50652



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

[PATCH] D50337: [clangd] DexIndex implementation prototype

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 160576.
kbobyrev added a comment.

Don't separate the logic for "long" and "short" queries: 
https://reviews.llvm.org/D50517 (https://reviews.llvm.org/rCTE339548) 
introduced incomplete trigrams which can be used on for "short" queries, too.


https://reviews.llvm.org/D50337

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp
  clang-tools-extra/unittests/clangd/TestIndexOperations.cpp
  clang-tools-extra/unittests/clangd/TestIndexOperations.h

Index: clang-tools-extra/unittests/clangd/TestIndexOperations.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndexOperations.cpp
@@ -0,0 +1,89 @@
+//===-- IndexHelpers.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestIndexOperations.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName) {
+  Symbol Sym;
+  Sym.ID = SymbolID(QName.str());
+  size_t Pos = QName.rfind("::");
+  if (Pos == llvm::StringRef::npos) {
+Sym.Name = QName;
+Sym.Scope = "";
+  } else {
+Sym.Name = QName.substr(Pos + 2);
+Sym.Scope = QName.substr(0, Pos + 2);
+  }
+  return Sym;
+}
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the returned shared pointer.
+// If \p WeakSymbols is provided, it will be pointed to the managed object in
+// the returned shared pointer.
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols) {
+  SymbolSlab::Builder Slab;
+  for (llvm::StringRef QName : QualifiedNames)
+Slab.insert(symbol(QName));
+
+  auto Storage = std::make_shared();
+  Storage->Slab = std::move(Slab).build();
+  for (const auto &Sym : Storage->Slab)
+Storage->Pointers.push_back(&Sym);
+  if (WeakSymbols)
+*WeakSymbols = Storage;
+  auto *Pointers = &Storage->Pointers;
+  return {std::move(Storage), Pointers};
+}
+
+// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
+// to the `generateSymbols` above.
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols) {
+  std::vector Names;
+  for (int i = Begin; i <= End; i++)
+Names.push_back(std::to_string(i));
+  return generateSymbols(Names, WeakSymbols);
+}
+
+std::string getQualifiedName(const Symbol &Sym) {
+  return (Sym.Scope + Sym.Name).str();
+}
+
+std::vector match(const SymbolIndex &I,
+   const FuzzyFindRequest &Req, bool *Incomplete) {
+  std::vector Matches;
+  bool IsIncomplete = I.fuzzyFind(Req, [&](const Symbol &Sym) {
+Matches.push_back(clang::clangd::getQualifiedName(Sym));
+  });
+  if (Incomplete)
+*Incomplete = IsIncomplete;
+  return Matches;
+}
+
+// Returns qualified names of symbols with any of IDs in the index.
+std::vector lookup(const SymbolIndex &I,
+llvm::ArrayRef IDs) {
+  LookupRequest Req;
+  Req.IDs.insert(IDs.begin(), IDs.end());
+  std::vector Results;
+  I.lookup(Req, [&](const Symbol &Sym) {
+Results.push_back(getQualifiedName(Sym));
+  });
+  return Results;
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/unittests/clangd/TestIndexOperations.h
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndexOperations.h
@@ -0,0 +1,57 @@
+//===-- IndexHelpers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/dex/DexIndex.h"
+#include "index/dex/Iterator.h"
+#include "index/dex/Token.h"
+#include "index/dex/Trigram.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName);
+
+struct SlabAndPointers {
+  SymbolSlab Slab;
+  std::vector Pointers;
+};
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the retur

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Added a test case.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3375,6 +3375,22 @@
"= b\n"
"  >> (aa);",
Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("{\n"
+"\treturn someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+"\t\t&& (someOtherLongishConditionPart1\n"
+"\t\t\t|| someOtherEvenLongerNestedConditionPart2);\n"
+"}",
+format("{\n"
+   "\treturn someVeryVeryLongConditionThatBarelyFitsOnALine && 
(someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3375,6 +3375,22 @@
"= b\n"
"  >> (aa);",
Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("{\n"
+"\treturn someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+"\t\t&& (someOtherLongishConditionPart1\n"
+"\t\t\t|| someOtherEvenLongerNestedConditionPart2);\n"
+"}",
+format("{\n"
+   "\treturn someVeryVeryLongConditionThatBarelyFitsOnALine && (someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

Thanks @Hahnfeld for your suggestions.

Unfortunately doing the lowering in the backend one would need to replace the 
math function calls with calls to libdevice function calls. I have not been 
able to do that in an elegant way. Encoding the interface to libdevice is just 
not a clean process not to mention that any changes to libdevice will have to 
be tracked manually with every new CUDA version. It does not make the code more 
maintainable, on the contrary I think it makes it harder to track libdevice 
changes.

On the same note, clang-cuda doesn't do the pow(a,2) -> a*a optimization, I 
checked. It is something that needs to be fixed for Clang-CUDA first before 
OpenMP can make use of it. OpenMP-NVPTX toolchain is designed to exist on top 
of the CUDA toolchain. It therefore inherits all the clang-cuda benefits and in 
this particular case, limitations.

As for the Sema check error you report (the one related to the x restriction), 
I think the fix you proposed is good and should be pushed in a separate patch.


Repository:
  rC Clang

https://reviews.llvm.org/D47849



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

[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia updated this revision to Diff 160582.

https://reviews.llvm.org/D50389

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationDivisionCheck.cpp
  clang-tidy/abseil/DurationDivisionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-division.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-division.cpp

Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+}  // namespace absl
+
+void TakesDouble(double);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+  absl::Duration d;
+
+  const double num_double = d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+  // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+  const float num_float = d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+  const auto SomeVal = 1.0 + d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+  if (MACRO_EQ(d/d, 0.0)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+  if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+  // This one generates a message, but no fix.
+  if (MACRO_DIVEQ(d, d, 0.0)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+ 
+  TakesDouble(d/d);
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+  // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesInt(int);
+template 
+void TakesGeneric(T);
+
+void Negatives() {
+  absl::Duration d;
+  const int num_int = d/d;
+  const long num_long = d/d;
+  const short num_short = d/d;
+  const char num_char = d/d;
+  const auto num_auto = d/d;
+  const auto SomeVal = 1 + d/d;
+
+  TakesInt(d/d);
+  TakesGeneric(d/d);
+  // Explicit cast should disable the warning.
+  const double num_cast1 = static_cast(d/d);
+  const double num_cast2 = (double)(d/d);
+}
+
+template 
+double DoubleDivision(T t1, T t2) {return t1/t2;}
+
+//This also won't trigger a warning
+void TemplateDivision() {
+  absl::Duration d;
+  DoubleDivision(d, d);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+  
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d);  // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1);  // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1));  // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2

[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia added inline comments.



Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+  // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration 
objects
+  // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+  // absl::FDivDuration(t1, t2);}

hokein wrote:
> deannagarcia wrote:
> > hokein wrote:
> > > I think we should ignore templates. The template function could be used 
> > > by other types, fixing the template is not correct. 
> > I removed this template function, but left the template function 
> > TakesGeneric below to show that the automatic type does not require/give a 
> > warning. Is that alright?
> The check will still give warnings in the template instantiation,  I think we 
> need `unless(isInTemplateInstantiation()` matcher to filter them out.
I added that and then put the test back to show that it now won't trigger a 
warning. Is that what you wanted?


https://reviews.llvm.org/D50389



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

[PATCH] D50502: [clangd] Initial cancellation mechanism for LSP requests.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for missing this one. The biggest concern that I have is about the 
thread-safety of the API, it's too easy to misuse from multiple threads at this 
point.

We should define thread-safety guarantees for `TaskHandle` and 
`CancellationToken` and make sure they're not easily broken. The suggested way 
is to make both move-only types.
For `CancellationToken` we might need a non-thread-safe copy operation, though. 
But maybe make it a separate method (`clone`?) with a clear statement that it's 
not thread-safe?

Other than that just NITs.




Comment at: clangd/Cancellation.cpp:17
+namespace {
+static Key>> CancellationTokenKey;
+} // namespace

Having a `shared_ptr` key in the Context can cause data races (e.g. if we copy 
it concurrently from multiple threads).
I suggest we make `CancellationToken` move-only (i.e. disallow copies) and 
return `const CancellationToken&` when getting it from the context.



Comment at: clangd/Cancellation.cpp:34
+llvm::Error CancellationHandler::getCancellationError() {
+  return llvm::make_error();
+}

I'm not sure if this function buys us much in terms of clarity, maybe remove it 
and inline everywhere?



Comment at: clangd/Cancellation.h:9
+//===--===//
+// CancellationToken mechanism for async threads. The caller can generate a
+// TaskHandle for cancellable tasks, then bind that handle to current context

The comments seems to mention all the important bits, but maybe we could 
separate different concerns more clearly based on how the code should be used?
E.g. here's a rough draft to illustrate the idea:

```
Cancellation mechanism for async tasks. Roughly all the clients of this code 
can be classified into three categories:
1. The code that creates and schedules async tasks, e.g. TUScheduler.
2. The callers of the async method that can cancel some of the running tasks, 
e.g. `ClangdLSPServer`
3. The code running inside the async task itself, i.e. code completion or find 
definition implementation that run clang, etc.

For (1), the guideline is to accept a callback for the result of async 
operation and return a `TaskHandle` to allow cancelling the request.
TaskHandle someAsyncMethod(function)> 
Callback);

The callers of async methods (2) can issue cancellations and should be prepared 
to handle `TaskCancelledError` result:
TaskHandle H = someAsyncMethod([](llvm::Expected R) {
if (/* code to check for TaskCancelledError */)
   llvm::errs() << "Task got cancelled";
 else
   llvm::errs() << "The result is: " << *R;
} 
});

sleep(5);
H.cancel();

The worker code itself (3) should check for cancellations using 
`CancellationToken` that can be retrieved via 
`CancellationToken::isCancelled()`.
void codeComplete() {
  const CancellationToken& CT = CancellationToken::isCancelled();
  if (CT.isCancelled()) 
return llvm::makeError();

  runSema();
  if (CT.isCancelled()) 
return llvm::makeError();

  queryIndex();  
  return results;
}
If the operation was cancelled before it could run to completion, it should 
propagate the TaskCancelledError as a result.
```



Comment at: clangd/Cancellation.h:60
+
+class CancellationToken {
+private:

We're missing docs on this and the other classes. I suggest a small doc 
describing what they're used for, e.g. `used for checking if the operation was 
cancelled` and `used to signal the operation should be cancelled`, etc.

Also noting the threading-safety guarantees is a probably a good idea.



Comment at: clangd/Cancellation.h:61
+class CancellationToken {
+private:
+  std::shared_ptr> Token;

NIT: move members to the end of the class to be consistent with the rest of 
clangd.



Comment at: clangd/Cancellation.h:67
+  operator bool() const { return isCancelled(); }
+  CancellationToken(const std::shared_ptr> Token)
+  : Token(Token) {}

Can we make this ctor private? 



Comment at: clangd/Cancellation.h:71
+
+class TaskHandle {
+public:

I wonder if we should make `TaskHandle` move-only?

The reasoning is that is can be easily used from multiple threads (since it's 
used by code dealing with async tasks anyway) and copying it concurrently is a 
data race.
On the other hand, calling `cancel()` is perfectly thread-safe and shouldn't 
cause any problems.



Comment at: clangd/Cancellation.h:74
+  void cancel();
+  static TaskHandle createCancellableTaskHandle();
+  friend class CancellationHandler;

NIT: maybe use a shorter name, e.g. `TaskHandle::create`?



Comment at: clangd/Cancellation.h:82
+
+class CancellationHandler {
+public:

[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks, looks good from my side. I will commit for you if other reviewers have 
no further comments.


https://reviews.llvm.org/D50389



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

[PATCH] D50707: NFC: Enforce good formatting across multiple clang-tools-extra files

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous.

This patch improves readability of multiple files in clang-tools-extra and 
enforces LLVM Coding Guidelines.


https://reviews.llvm.org/D50707

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/CodeCompletionStrings.h
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/Context.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Merge.h
  clang-tools-extra/clangd/index/SymbolYAML.h
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/unittests/clangd/Annotations.cpp
  clang-tools-extra/unittests/clangd/Annotations.h
  clang-tools-extra/unittests/clangd/SyncAPI.cpp
  clang-tools-extra/unittests/clangd/SyncAPI.h
  clang-tools-extra/unittests/clangd/TestTU.cpp
  clang-tools-extra/unittests/clangd/TestTU.h

Index: clang-tools-extra/unittests/clangd/TestTU.h
===
--- clang-tools-extra/unittests/clangd/TestTU.h
+++ clang-tools-extra/unittests/clangd/TestTU.h
@@ -1,21 +1,23 @@
-//===--- TestTU.h - Scratch source files for testing *- C++-*-===//
+//===--- TestTU.h - Scratch source files for testing -*- C++-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
 //
 // Many tests for indexing, code completion etc are most naturally expressed
 // using code examples.
 // TestTU lets test define these examples in a common way without dealing with
 // the mechanics of VFS and compiler interactions, and then easily grab the
 // AST, particular symbols, etc.
 //
 //===-===//
+
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+
 #include "ClangdUnit.h"
 #include "index/Index.h"
 #include "gtest/gtest.h"
@@ -64,4 +66,5 @@
 
 } // namespace clangd
 } // namespace clang
-#endif
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
Index: clang-tools-extra/unittests/clangd/TestTU.cpp
===
--- clang-tools-extra/unittests/clangd/TestTU.cpp
+++ clang-tools-extra/unittests/clangd/TestTU.cpp
@@ -5,7 +5,8 @@
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
+
 #include "TestTU.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
Index: clang-tools-extra/unittests/clangd/SyncAPI.h
===
--- clang-tools-extra/unittests/clangd/SyncAPI.h
+++ clang-tools-extra/unittests/clangd/SyncAPI.h
@@ -5,10 +5,14 @@
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
+//
 // This file contains synchronous versions of ClangdServer's async API. We
 // deliberately don't expose the sync API outside tests to encourage using the
 // async versions in clangd code.
+//
+//===--===//
+
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 
@@ -49,4 +53,4 @@
 } // namespace clangd
 } 

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

Just to address any generality concerns:

This patch fixes the problem of calling libdevice math functions for all 
platform combinations. It ensures that the OpenMP NVPTX target region will NOT 
call any host math functions (which ever host that may be) IF equivalent device 
functions are available.

I think there was a confusion regarding header file inclusion. This patch does 
not address any issues that might arise from the user including header files 
(be it math.h or some other header). Any failure related to header file 
inclusion (such as the reported x restriction issue on x86) is unrelated to 
what this patch aims to do. Before the functionality in this patch can kick in, 
any user-included headers must successfully pass all checks in place for the 
NVPTX toolchain. A fix in the direction of the one proposed in one of the 
comments above is probably required. The fix would also needs its own separate 
patch.


Repository:
  rC Clang

https://reviews.llvm.org/D47849



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added a comment.

Could somebody verify that the `DISPATCHER_CONTEXT` struct is defined in 
`` for the Win8 and Win10 SDKs? I can't install them right now.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50707: NFC: Enforce good formatting across multiple clang-tools-extra files

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Fixing header guards and file comments is fine, but I'm not a fan of 
reformatting-only changes in source code as they tends to make `git blame` 
harder.


https://reviews.llvm.org/D50707



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

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

[Owen Pan via Phabricator via cfe-commits]

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

Updated the test case.


Repository:
  rC Clang

https://reviews.llvm.org/D50697

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1145,6 +1145,22 @@
"  break;\n"
"}",
Style);
+  Style.ColumnLimit = 80;
+  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.IndentCaseLabels = true;
+  EXPECT_EQ("switch (n) {\n"
+"  default /*comments*/:\n"
+"return true;\n"
+"  case 0:\n"
+"return false;\n"
+"}",
+format("switch (n) {\n"
+   "default/*comments*/:\n"
+   "  return true;\n"
+   "case 0:\n"
+   "  return false;\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1145,6 +1145,22 @@
"  break;\n"
"}",
Style);
+  Style.ColumnLimit = 80;
+  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.IndentCaseLabels = true;
+  EXPECT_EQ("switch (n) {\n"
+"  default /*comments*/:\n"
+"return true;\n"
+"  case 0:\n"
+"return false;\n"
+"}",
+format("switch (n) {\n"
+   "default/*comments*/:\n"
+   "  return true;\n"
+   "case 0:\n"
+   "  return false;\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50641: [clangd][test] Fix exit messages in tests

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Many thanks for fixing this.

Adding some failure monitoring seems like a nice idea. On the other hand, 
polluting every test with stderr redirection doesn't look like a nice idea.
As a middle ground, I could imagine some extra checking being done if 
`-lit-test` is passed, e.g. returning a non-zero error code on request parsing 
errors would trigger test failures in these particular cases, right?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50641



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

[PATCH] D50641: [clangd][test] Fix exit messages in tests

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Haven't noticed Alex's comments, sorry for a duplicate suggestion about exiting 
with failure error code


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50641



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

[PATCH] D50691: [CMake] Fix the LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY option

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rCXX libc++

https://reviews.llvm.org/D50691



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

[PATCH] D50709: [clang-doc] Fix unused var

[Julie Hockett via Phabricator via cfe-commits]

juliehockett created this revision.
juliehockett added reviewers: leonardchan, jakehehrlich, lebedev.ri.
juliehockett added a project: clang-tools-extra.

https://reviews.llvm.org/D50709

Files:
  clang-tools-extra/clang-doc/Mapper.cpp


Index: clang-tools-extra/clang-doc/Mapper.cpp
===
--- clang-tools-extra/clang-doc/Mapper.cpp
+++ clang-tools-extra/clang-doc/Mapper.cpp
@@ -30,7 +30,7 @@
 return true;
 
   // Skip function-internal decls.
-  if (const DeclContext *F = D->getParentFunctionOrMethod())
+  if (D->getParentFunctionOrMethod())
 return true;
 
   llvm::SmallString<128> USR;


Index: clang-tools-extra/clang-doc/Mapper.cpp
===
--- clang-tools-extra/clang-doc/Mapper.cpp
+++ clang-tools-extra/clang-doc/Mapper.cpp
@@ -30,7 +30,7 @@
 return true;
 
   // Skip function-internal decls.
-  if (const DeclContext *F = D->getParentFunctionOrMethod())
+  if (D->getParentFunctionOrMethod())
 return true;
 
   llvm::SmallString<128> USR;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50403: [clang-format]AlignConsecutiveAssignments

[Peter Doak via Phabricator via cfe-commits]

PDoakORNL added a comment.

So it's been a week, is there an owner for clang/lib/Format?


Repository:
  rC Clang

https://reviews.llvm.org/D50403



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

[PATCH] D50709: [clang-doc] Fix unused var

[Roman Lebedev via Phabricator via cfe-commits]

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

Looks like NFC.


https://reviews.llvm.org/D50709



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

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 160598.
gtbercea added a comment.
Herald added a subscriber: jholewinski.

Add __NO_MATH_INLINES macro for the NVPTX toolchain to prevent any host 
assembly from seeping onto the device.


Repository:
  rC Clang

https://reviews.llvm.org/D47849

Files:
  include/clang/Driver/ToolChain.h
  lib/Basic/Targets/NVPTX.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_cuda_device_functions.h
  lib/Headers/__clang_cuda_libdevice_declares.h
  test/CodeGen/nvptx_device_math_functions.c
  test/Driver/openmp-offload-gpu.c

Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -76,9 +76,9 @@
 // RUN:  -no-canonical-prefixes -save-temps %t.o -fopenmp-use-target-bundling 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-CUBIN-UNBUNDLING-NVLINK %s
 
-/// Use DAG to ensure that cubin file has been unbundled.
+/// Use DAG to ensure that object file has not been unbundled.
 // CHK-CUBIN-UNBUNDLING-NVLINK-DAG: nvlink{{.*}}" {{.*}}"[[CUBIN:.*\.cubin]]"
-// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: clang-offload-bundler{{.*}}" "-type=o" {{.*}}"-outputs={{.*}}[[CUBIN]]
+// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: clang-offload-bundler{{.*}}" "-type=o" {{.*}}[[CUBIN]]
 // CHK-CUBIN-UNBUNDLING-NVLINK-DAG-SAME: "-unbundle"
 
 /// ###
Index: test/CodeGen/nvptx_device_math_functions.c
===
--- /dev/null
+++ test/CodeGen/nvptx_device_math_functions.c
@@ -0,0 +1,20 @@
+// Test calling of device math functions.
+///==///
+
+// RUN: %clang -fmath-errno -S -emit-llvm -o - %s -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda | FileCheck -check-prefix CHECK-YES %s
+
+void test_sqrt(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @llvm.nvvm.sqrt.rn.d(double
+double l1 = sqrt(a1);
+  }
+}
+
+void test_pow(float a0, double a1, long double a2) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @__internal_accurate_pow(double
+double l1 = pow(a1, a1);
+  }
+}
Index: lib/Headers/__clang_cuda_libdevice_declares.h
===
--- lib/Headers/__clang_cuda_libdevice_declares.h
+++ lib/Headers/__clang_cuda_libdevice_declares.h
@@ -24,443 +24,455 @@
 #ifndef __CLANG_CUDA_LIBDEVICE_DECLARES_H__
 #define __CLANG_CUDA_LIBDEVICE_DECLARES_H__
 
+#if defined(_OPENMP)
+#define __DEVICE__
+#elif defined(__CUDA__)
+#define __DEVICE__ __device__
+#endif
+
+#if defined(__cplusplus)
 extern "C" {
+#endif
 
-__device__ int __nv_abs(int __a);
-__device__ double __nv_acos(double __a);
-__device__ float __nv_acosf(float __a);
-__device__ double __nv_acosh(double __a);
-__device__ float __nv_acoshf(float __a);
-__device__ double __nv_asin(double __a);
-__device__ float __nv_asinf(float __a);
-__device__ double __nv_asinh(double __a);
-__device__ float __nv_asinhf(float __a);
-__device__ double __nv_atan2(double __a, double __b);
-__device__ float __nv_atan2f(float __a, float __b);
-__device__ double __nv_atan(double __a);
-__device__ float __nv_atanf(float __a);
-__device__ double __nv_atanh(double __a);
-__device__ float __nv_atanhf(float __a);
-__device__ int __nv_brev(int __a);
-__device__ long long __nv_brevll(long long __a);
-__device__ int __nv_byte_perm(int __a, int __b, int __c);
-__device__ double __nv_cbrt(double __a);
-__device__ float __nv_cbrtf(float __a);
-__device__ double __nv_ceil(double __a);
-__device__ float __nv_ceilf(float __a);
-__device__ int __nv_clz(int __a);
-__device__ int __nv_clzll(long long __a);
-__device__ double __nv_copysign(double __a, double __b);
-__device__ float __nv_copysignf(float __a, float __b);
-__device__ double __nv_cos(double __a);
-__device__ float __nv_cosf(float __a);
-__device__ double __nv_cosh(double __a);
-__device__ float __nv_coshf(float __a);
-__device__ double __nv_cospi(double __a);
-__device__ float __nv_cospif(float __a);
-__device__ double __nv_cyl_bessel_i0(double __a);
-__device__ float __nv_cyl_bessel_i0f(float __a);
-__device__ double __nv_cyl_bessel_i1(double __a);
-__device__ float __nv_cyl_bessel_i1f(float __a);
-__device__ double __nv_dadd_rd(double __a, double __b);
-__device__ double __nv_dadd_rn(double __a, double __b);
-__device__ double __nv_dadd_ru(double __a, double __b);
-__device__ double __nv_dadd_rz(double __a, double __b);
-__device__ double __nv_ddiv_rd(double __a, double __b);
-__device__ double __nv_ddiv_rn(double __a, double __b);
-__device__ double __nv_ddiv_ru(double __a, double __b);
-__device__ double __nv_ddiv_rz(double __a, double __b);
-__device__ double __nv_dmul_rd(double __a, double __b);
-__device__ double __nv_dmul_rn(dou

[clang-tools-extra] r339685 - [clang-doc] Fix unused variable

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Tue Aug 14 08:38:59 2018
New Revision: 339685

URL: http://llvm.org/viewvc/llvm-project?rev=339685&view=rev
Log:
[clang-doc] Fix unused variable

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

Modified:
clang-tools-extra/trunk/clang-doc/Mapper.cpp

Modified: clang-tools-extra/trunk/clang-doc/Mapper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Mapper.cpp?rev=339685&r1=339684&r2=339685&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Mapper.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Mapper.cpp Tue Aug 14 08:38:59 2018
@@ -30,7 +30,7 @@ template  bool MapASTVisitor
 return true;
 
   // Skip function-internal decls.
-  if (const DeclContext *F = D->getParentFunctionOrMethod())
+  if (D->getParentFunctionOrMethod())
 return true;
 
   llvm::SmallString<128> USR;


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

[PATCH] D50709: [clang-doc] Fix unused var

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339685: [clang-doc] Fix unused variable (authored by 
juliehockett, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50709?vs=160593&id=160600#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50709

Files:
  clang-tools-extra/trunk/clang-doc/Mapper.cpp


Index: clang-tools-extra/trunk/clang-doc/Mapper.cpp
===
--- clang-tools-extra/trunk/clang-doc/Mapper.cpp
+++ clang-tools-extra/trunk/clang-doc/Mapper.cpp
@@ -30,7 +30,7 @@
 return true;
 
   // Skip function-internal decls.
-  if (const DeclContext *F = D->getParentFunctionOrMethod())
+  if (D->getParentFunctionOrMethod())
 return true;
 
   llvm::SmallString<128> USR;


Index: clang-tools-extra/trunk/clang-doc/Mapper.cpp
===
--- clang-tools-extra/trunk/clang-doc/Mapper.cpp
+++ clang-tools-extra/trunk/clang-doc/Mapper.cpp
@@ -30,7 +30,7 @@
 return true;
 
   // Skip function-internal decls.
-  if (const DeclContext *F = D->getParentFunctionOrMethod())
+  if (D->getParentFunctionOrMethod())
 return true;
 
   llvm::SmallString<128> USR;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50711: [AST] Pack the unsigned of PackExpansionType into Type

[Bruno Ricci via Phabricator via cfe-commits]

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

The bit-fields of Type have enough space for
the `unsigned NumExpansions` of PackExpansionType


Repository:
  rC Clang

https://reviews.llvm.org/D50711

Files:
  include/clang/AST/Type.h


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1638,6 +1638,21 @@
 unsigned NumArgs;
   };
 
+  class PackExpansionTypeBitfields {
+friend class PackExpansionType;
+
+unsigned : NumTypeBits;
+
+/// The number of expansions that this pack expansion will
+/// generate when substituted (+1).
+///
+/// This field will only have a non-zero value when some of the parameter
+/// packs that occur within the pattern have been substituted but others
+/// have not. Intentionally not a bitfield since we have plenty
+/// of space left.
+unsigned NumExpansions;
+  };
+
   union {
 TypeBitfields TypeBits;
 ArrayTypeBitfields ArrayTypeBits;
@@ -1650,6 +1665,7 @@
 TypeWithKeywordBitfields TypeWithKeywordBits;
 VectorTypeBitfields VectorTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
+PackExpansionTypeBitfields PackExpansionTypeBits;
 
 static_assert(sizeof(TypeBitfields) <= 8,
   "TypeBitfields is larger than 8 bytes!");
@@ -1674,6 +1690,8 @@
 static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8,
   "TemplateSpecializationTypeBitfields is larger"
   " than 8 bytes!");
+static_assert(sizeof(PackExpansionTypeBitfields) <= 8,
+  "PackExpansionTypeBitfields is larger than 8 bytes");
   };
 
 private:
@@ -5189,22 +5207,16 @@
   /// The pattern of the pack expansion.
   QualType Pattern;
 
-  /// The number of expansions that this pack expansion will
-  /// generate when substituted (+1), or indicates that
-  ///
-  /// This field will only have a non-zero value when some of the parameter
-  /// packs that occur within the pattern have been substituted but others have
-  /// not.
-  unsigned NumExpansions;
-
   PackExpansionType(QualType Pattern, QualType Canon,
 Optional NumExpansions)
   : Type(PackExpansion, Canon, /*Dependent=*/Pattern->isDependentType(),
  /*InstantiationDependent=*/true,
  /*VariablyModified=*/Pattern->isVariablyModifiedType(),
  /*ContainsUnexpandedParameterPack=*/false),
-Pattern(Pattern),
-NumExpansions(NumExpansions ? *NumExpansions + 1 : 0) {}
+Pattern(Pattern) {
+PackExpansionTypeBits.NumExpansions =
+NumExpansions ? *NumExpansions + 1 : 0;
+  }
 
 public:
   /// Retrieve the pattern of this pack expansion, which is the
@@ -5215,9 +5227,8 @@
   /// Retrieve the number of expansions that this pack expansion will
   /// generate, if known.
   Optional getNumExpansions() const {
-if (NumExpansions)
-  return NumExpansions - 1;
-
+if (PackExpansionTypeBits.NumExpansions)
+  return PackExpansionTypeBits.NumExpansions - 1;
 return None;
   }
 


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1638,6 +1638,21 @@
 unsigned NumArgs;
   };
 
+  class PackExpansionTypeBitfields {
+friend class PackExpansionType;
+
+unsigned : NumTypeBits;
+
+/// The number of expansions that this pack expansion will
+/// generate when substituted (+1).
+///
+/// This field will only have a non-zero value when some of the parameter
+/// packs that occur within the pattern have been substituted but others
+/// have not. Intentionally not a bitfield since we have plenty
+/// of space left.
+unsigned NumExpansions;
+  };
+
   union {
 TypeBitfields TypeBits;
 ArrayTypeBitfields ArrayTypeBits;
@@ -1650,6 +1665,7 @@
 TypeWithKeywordBitfields TypeWithKeywordBits;
 VectorTypeBitfields VectorTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
+PackExpansionTypeBitfields PackExpansionTypeBits;
 
 static_assert(sizeof(TypeBitfields) <= 8,
   "TypeBitfields is larger than 8 bytes!");
@@ -1674,6 +1690,8 @@
 static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8,
   "TemplateSpecializationTypeBitfields is larger"
   " than 8 bytes!");
+static_assert(sizeof(PackExpansionTypeBitfields) <= 8,
+  "PackExpansionTypeBitfields is larger than 8 bytes");
   };
 
 private:
@@ -5189,22 +5207,16 @@
   /// The pattern of the pack expansion.
   QualType Pattern;
 
-  /// The number of expansions that this pack expansion will
-  /// generate when substituted (+1), or indicates that
-  ///
-  /// This field wil

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191
+void templated_thrower() { throw T{}(); }
+// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 
'int' is not derived from 'std::exception'
+

JonasToth wrote:
> JonasToth wrote:
> > alexfh wrote:
> > > hokein wrote:
> > > > I think giving message on the template function here is confusing to 
> > > > users even it gets instantiated somewhere in this TU -- because users 
> > > > have to find the location that triggers the template instantiation.
> > > > 
> > > > Maybe 
> > > > 1) Add a note which gives the instantiation location to the message, or
> > > > 2) ignore template case (some clang-tidy checks do this)
> > > In this particular case it seems to be useful to get warnings from 
> > > template instantiations. But the message will indeed be confusing.
> > > 
> > > Ideally, the message should have "in instantiation of xxx requested here" 
> > > notes attached, as clang warnings do. But this is not working 
> > > automatically, and it's implemented in Sema 
> > > (`Sema::PrintInstantiationStack()` in 
> > > lib/Sema/SemaTemplateInstantiate.cpp).
> > > 
> > > I wonder whether it's feasible to produce similar notes after Sema is 
> > > dead? Maybe not the whole instantiation stack, but at least it should be 
> > > possible to figure out that the enclosing function is a template 
> > > instantiation or is a member function of an type that is an instantiation 
> > > of a template. That would be useful for other checks as well.
> > It should be possible to figure out, that the type comes from template 
> > instantiation and that information could be added to the warning.
> > 
> > I will take a look at Sema and think about something like this. 
> > Unfortunatly i dont have a lot of time :/
> I did look further into the issue, i think it is non-trivial.
> 
> The newly added case is not a templated exception perse, but there is a 
> exception-factory, which is templated, that creates a normal exception.
> 
> I did add another note for template instantiations, but i could not figure 
> out a way to give better diagnostics for the new use-case.
@hokein and @alexfh Do you still have your concerns (the exception is not a 
template value, but the factory creating them) or is this fix acceptable?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714



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

[PATCH] D50712: [AST] Pack the unsigned of DependentTemplateSpecializationType into Type

[Bruno Ricci via Phabricator via cfe-commits]

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

The bit-fields of `Type` have enough space for the member
`unsigned NumArgs` of DependentTemplateSpecializationType


Repository:
  rC Clang

https://reviews.llvm.org/D50712

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

Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2604,7 +2604,8 @@
   : TypeWithKeyword(Keyword, DependentTemplateSpecialization, Canon, true, true,
 /*VariablyModified=*/false,
 NNS && NNS->containsUnexpandedParameterPack()),
-NNS(NNS), Name(Name), NumArgs(Args.size()) {
+NNS(NNS), Name(Name) {
+  DependentTemplateSpecializationTypeBits.NumArgs = Args.size();
   assert((!NNS || NNS->isDependent()) &&
  "DependentTemplateSpecializatonType requires dependent qualifier");
   TemplateArgument *ArgBuffer = getArgBuffer();
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1589,6 +1589,8 @@
 unsigned Keyword : 8;
   };
 
+  enum { NumTypeWithKeywordBits = 8 };
+
   class VectorTypeBitfields {
 friend class VectorType;
 friend class DependentVectorType;
@@ -1638,6 +1640,18 @@
 unsigned NumArgs;
   };
 
+  class DependentTemplateSpecializationTypeBitfields {
+friend class DependentTemplateSpecializationType;
+
+unsigned : NumTypeBits;
+unsigned : NumTypeWithKeywordBits;
+
+/// The number of template arguments named in this class template
+/// specialization. Intentionally not a bitfield since we have plenty
+/// of space left.
+unsigned NumArgs;
+  };
+
   class PackExpansionTypeBitfields {
 friend class PackExpansionType;
 
@@ -1665,6 +1679,8 @@
 TypeWithKeywordBitfields TypeWithKeywordBits;
 VectorTypeBitfields VectorTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
+DependentTemplateSpecializationTypeBitfields
+  DependentTemplateSpecializationTypeBits;
 PackExpansionTypeBitfields PackExpansionTypeBits;
 
 static_assert(sizeof(TypeBitfields) <= 8,
@@ -1690,6 +1706,9 @@
 static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8,
   "TemplateSpecializationTypeBitfields is larger"
   " than 8 bytes!");
+static_assert(sizeof(DependentTemplateSpecializationTypeBitfields) <= 8,
+  "DependentTemplateSpecializationTypeBitfields is larger"
+  " than 8 bytes!");
 static_assert(sizeof(PackExpansionTypeBitfields) <= 8,
   "PackExpansionTypeBitfields is larger than 8 bytes");
   };
@@ -5119,10 +5138,6 @@
   /// The identifier of the template.
   const IdentifierInfo *Name;
 
-  /// The number of template arguments named in this class template
-  /// specialization.
-  unsigned NumArgs;
-
   DependentTemplateSpecializationType(ElaboratedTypeKeyword Keyword,
   NestedNameSpecifier *NNS,
   const IdentifierInfo *Name,
@@ -5147,12 +5162,14 @@
   }
 
   /// Retrieve the number of template arguments.
-  unsigned getNumArgs() const { return NumArgs; }
+  unsigned getNumArgs() const {
+return DependentTemplateSpecializationTypeBits.NumArgs;
+  }
 
   const TemplateArgument &getArg(unsigned Idx) const; // in TemplateBase.h
 
   ArrayRef template_arguments() const {
-return {getArgs(), NumArgs};
+return {getArgs(), getNumArgs()};
   }
 
   using iterator = const TemplateArgument *;
@@ -5164,7 +5181,7 @@
   QualType desugar() const { return QualType(this, 0); }
 
   void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) {
-Profile(ID, Context, getKeyword(), NNS, Name, {getArgs(), NumArgs});
+Profile(ID, Context, getKeyword(), NNS, Name, {getArgs(), getNumArgs()});
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 160602.
lewis-revill added a comment.

Fixed this issue by adding -nostdsysteminc to the Clang target options, 
preventing the frontend from generating additional include paths.


Repository:
  rC Clang

https://reviews.llvm.org/D50246

Files:
  lib/Driver/ToolChains/RISCV.cpp
  lib/Driver/ToolChains/RISCV.h
  test/Driver/riscv32-toolchain.c

Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -24,7 +24,7 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-ILP32 %s
 
-// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++{{/|}}8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: "--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
Index: lib/Driver/ToolChains/RISCV.h
===
--- lib/Driver/ToolChains/RISCV.h
+++ lib/Driver/ToolChains/RISCV.h
@@ -23,6 +23,9 @@
  const llvm::opt::ArgList &Args);
 
   bool IsIntegratedAssemblerDefault() const override { return true; }
+  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
@@ -32,6 +35,9 @@
 
 protected:
   Tool *buildLinker() const override;
+
+private:
+  std::string computeSysRoot() const;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/RISCV.cpp
===
--- lib/Driver/ToolChains/RISCV.cpp
+++ lib/Driver/ToolChains/RISCV.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -27,7 +28,7 @@
const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
   GCCInstallation.init(Triple, Args);
-  getFilePaths().push_back(D.SysRoot + "/lib");
+  getFilePaths().push_back(computeSysRoot() + "/lib");
   if (GCCInstallation.isValid()) {
 getFilePaths().push_back(GCCInstallation.getInstallPath().str());
 getProgramPaths().push_back(
@@ -39,30 +40,52 @@
   return new tools::RISCV::Linker(*this);
 }
 
+void RISCVToolChain::addClangTargetOptions(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind) const {
+  CC1Args.push_back("-nostdsysteminc");
+}
+
 void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
ArgStringList &CC1Args) const {
   if (DriverArgs.hasArg(options::OPT_nostdinc))
 return;
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-SmallString<128> Dir(getDriver().SysRoot);
+SmallString<128> Dir(computeSysRoot());
 llvm::sys::path::append(Dir, "include");
 addSystemInclude(DriverArgs, CC1Args, Dir.str());
   }
 }
 
 void RISCVToolChain::addLibStdCxxIncludePaths(
 const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
-  StringRef LibDir = GCCInstallation.getParentLibPath();
   const GCCVersion &Version = GCCInstallation.getVersion();
   StringRef TripleStr = GCCInstallation.getTriple().str();
   const Multilib &Multilib = GCCInstallation.getMultilib();
-  addLibStdCXXIncludePaths(
-  LibDir.str() + "/../" + TripleStr.str() + "/include/c++/" + Version.Text,
+  addLibStdCXXIncludePaths(computeSysRoot() + "/include/c++/" + Version.Text,
   "", TripleStr, "", "", Multilib.includeSuffix(), DriverArgs, CC1Args);
 }
 
+std::string RISCVToolChain::computeSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+return getDriver().SysRoot;
+
+  if (!GCCInstallation.isValid())
+return std::string();
+
+  StringRef LibDir = GCCInstallation.getParentLibPath();
+  StringRef TripleStr = GCCInstallation.getTriple().str();
+  std::string SysRootDir = LibDir.str() + "/../" + TripleStr.str();
+
+  if (!llvm::sys::fs::exists(SysRootDir))
+return std::string();
+
+  return SysRootDir;
+}
+
 void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   

[PATCH] D50713: [AST] Pack the unsigned of SubstTemplateTypeParmPackType into Type

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: rsmith, rjmccall, erichkeane.
riccibruno added a project: clang.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, chrib, kristof.beyls.

The bit-fields of `Type` have enough space for the member
`unsigned NumArgs` of SubstTemplateTypeParmPackType.


Repository:
  rC Clang

https://reviews.llvm.org/D50713

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


Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -3285,11 +3285,12 @@
   QualType Canon,
   const TemplateArgument &ArgPack)
 : Type(SubstTemplateTypeParmPack, Canon, true, true, false, true),
-  Replaced(Param),
-  Arguments(ArgPack.pack_begin()), NumArguments(ArgPack.pack_size()) {}
+  Replaced(Param), Arguments(ArgPack.pack_begin()) {
+  SubstTemplateTypeParmPackTypeBits.NumArgs = ArgPack.pack_size();
+}
 
 TemplateArgument SubstTemplateTypeParmPackType::getArgumentPack() const {
-  return TemplateArgument(llvm::makeArrayRef(Arguments, NumArguments));
+  return TemplateArgument(llvm::makeArrayRef(Arguments, getNumArgs()));
 }
 
 void SubstTemplateTypeParmPackType::Profile(llvm::FoldingSetNodeID &ID) {
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1626,6 +1626,16 @@
 unsigned Keyword : 2;
   };
 
+  class SubstTemplateTypeParmPackTypeBitfields {
+friend class SubstTemplateTypeParmPackType;
+
+unsigned : NumTypeBits;
+
+/// The number of template arguments in \c Arguments.
+/// Intentionally not a bitfield since we have plenty of space left.
+unsigned NumArgs;
+  };
+
   class TemplateSpecializationTypeBitfields {
 friend class TemplateSpecializationType;
 
@@ -1678,6 +1688,7 @@
 ReferenceTypeBitfields ReferenceTypeBits;
 TypeWithKeywordBitfields TypeWithKeywordBits;
 VectorTypeBitfields VectorTypeBits;
+SubstTemplateTypeParmPackTypeBitfields SubstTemplateTypeParmPackTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
 DependentTemplateSpecializationTypeBitfields
   DependentTemplateSpecializationTypeBits;
@@ -1703,6 +1714,9 @@
   "TypeWithKeywordBitfields is larger than 8 bytes!");
 static_assert(sizeof(VectorTypeBitfields) <= 8,
   "VectorTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(SubstTemplateTypeParmPackTypeBitfields) <= 8,
+  "SubstTemplateTypeParmPackTypeBitfields is larger"
+  " than 8 bytes!");
 static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8,
   "TemplateSpecializationTypeBitfields is larger"
   " than 8 bytes!");
@@ -4543,9 +4557,6 @@
   /// parameter pack is instantiated with.
   const TemplateArgument *Arguments;
 
-  /// The number of template arguments in \c Arguments.
-  unsigned NumArguments;
-
   SubstTemplateTypeParmPackType(const TemplateTypeParmType *Param,
 QualType Canon,
 const TemplateArgument &ArgPack);
@@ -4558,6 +4569,10 @@
 return Replaced;
   }
 
+  unsigned getNumArgs() const {
+return SubstTemplateTypeParmPackTypeBits.NumArgs;
+  }
+
   bool isSugared() const { return false; }
   QualType desugar() const { return QualType(this, 0); }
 


Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -3285,11 +3285,12 @@
   QualType Canon,
   const TemplateArgument &ArgPack)
 : Type(SubstTemplateTypeParmPack, Canon, true, true, false, true),
-  Replaced(Param),
-  Arguments(ArgPack.pack_begin()), NumArguments(ArgPack.pack_size()) {}
+  Replaced(Param), Arguments(ArgPack.pack_begin()) {
+  SubstTemplateTypeParmPackTypeBits.NumArgs = ArgPack.pack_size();
+}
 
 TemplateArgument SubstTemplateTypeParmPackType::getArgumentPack() const {
-  return TemplateArgument(llvm::makeArrayRef(Arguments, NumArguments));
+  return TemplateArgument(llvm::makeArrayRef(Arguments, getNumArgs()));
 }
 
 void SubstTemplateTypeParmPackType::Profile(llvm::FoldingSetNodeID &ID) {
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1626,6 +1626,16 @@
 unsigned Keyword : 2;
   };
 
+  class SubstTemplateTypeParmPackTypeBitfields {
+friend class SubstTemplateTypeParmPackType;
+
+unsigned : NumTypeBits;
+
+/// The number of template arguments in \c Arguments.
+/// Intentionally not a bitfield since we have plenty of space left.
+unsigned NumArgs;
+  };
+
   class TemplateSpecializationTypeBitfields {
 friend class Template

[PATCH] D50707: NFC: Enforce good formatting across multiple clang-tools-extra files

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 160605.
kbobyrev added a comment.

I have updated the patch so that it only affects comments, header guards and 
inserts few newlines. Actual source code is not affected so that `git blame` 
log could be less cryptic.


https://reviews.llvm.org/D50707

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/CodeCompletionStrings.h
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/Context.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/Quality.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Merge.cpp
  clang-tools-extra/clangd/index/Merge.h
  clang-tools-extra/clangd/index/SymbolYAML.h
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/modularize/ModuleAssistant.cpp
  clang-tools-extra/unittests/clangd/Annotations.cpp
  clang-tools-extra/unittests/clangd/Annotations.h
  clang-tools-extra/unittests/clangd/SyncAPI.cpp
  clang-tools-extra/unittests/clangd/SyncAPI.h
  clang-tools-extra/unittests/clangd/TestTU.cpp
  clang-tools-extra/unittests/clangd/TestTU.h

Index: clang-tools-extra/unittests/clangd/TestTU.h
===
--- clang-tools-extra/unittests/clangd/TestTU.h
+++ clang-tools-extra/unittests/clangd/TestTU.h
@@ -1,21 +1,23 @@
-//===--- TestTU.h - Scratch source files for testing *- C++-*-===//
+//===--- TestTU.h - Scratch source files for testing -*- C++-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
 //
 // Many tests for indexing, code completion etc are most naturally expressed
 // using code examples.
 // TestTU lets test define these examples in a common way without dealing with
 // the mechanics of VFS and compiler interactions, and then easily grab the
 // AST, particular symbols, etc.
 //
 //===-===//
+
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+
 #include "ClangdUnit.h"
 #include "index/Index.h"
 #include "gtest/gtest.h"
@@ -64,4 +66,5 @@
 
 } // namespace clangd
 } // namespace clang
-#endif
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
Index: clang-tools-extra/unittests/clangd/TestTU.cpp
===
--- clang-tools-extra/unittests/clangd/TestTU.cpp
+++ clang-tools-extra/unittests/clangd/TestTU.cpp
@@ -5,7 +5,8 @@
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
+
 #include "TestTU.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
Index: clang-tools-extra/unittests/clangd/SyncAPI.h
===
--- clang-tools-extra/unittests/clangd/SyncAPI.h
+++ clang-tools-extra/unittests/clangd/SyncAPI.h
@@ -5,10 +5,14 @@
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
-//===-===//
+//===--===//
+//
 // This file contains synchronous versions of ClangdServer's async API. We
 // deliberately don't expose the sync API outside tests to encourage using the
 // async versions in clangd code.
+//
+//===--===//
+
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 
@@ -49,4 +53,4 @@
 } // namespace clangd
 } // namespace clang
 
-#e