[PATCH] D52074: [PowerPC] [Clang] Add vector int128 pack/unpack builtins
wuzish added a comment. Please make this patch commit after https://reviews.llvm.org/D52072 Repository: rC Clang https://reviews.llvm.org/D52074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52074: [PowerPC] [Clang] Add vector int128 pack/unpack builtins
wuzish created this revision. wuzish added reviewers: hfinkel, nemanjai, kbarton. Herald added a subscriber: kristina. > unsigned long long __builtin_unpack_vector_int128 (vector __int128_t, int); > vector __int128_t __builtin_pack_vector_int128 (unsigned long long, unsigned > long long); Builtins should behave the same way as in GCC. Repository: rC Clang https://reviews.llvm.org/D52074 Files: clang/include/clang/Basic/BuiltinsPPC.def clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Sema/SemaChecking.cpp clang/test/CodeGen/builtins-ppc-error.c clang/test/CodeGen/builtins-ppc-p7-disabled.c clang/test/CodeGen/builtins-ppc-vsx.c Index: clang/test/CodeGen/builtins-ppc-vsx.c === --- clang/test/CodeGen/builtins-ppc-vsx.c +++ clang/test/CodeGen/builtins-ppc-vsx.c @@ -49,6 +49,7 @@ vector bool long long res_vbll; vector signed long long res_vsll; vector unsigned long long res_vull; +vector signed __int128 res_vslll; double res_d; float res_af[4]; @@ -1803,3 +1804,31 @@ // CHECK-NEXT-LE: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> // CHECK-NEXT-LE: bitcast <4 x i32> %{{[0-9]+}} to <2 x double> } + +void testVectorInt128Pack(){ +// CHECK-LABEL: testVectorInt128Pack +// CHECK-LABEL-LE: testVectorInt128Pack + res_vslll = __builtin_pack_vector_int128(aull[0], aull[1]); +// CHECK: %[[V1:[0-9]+]] = insertelement <2 x i64> undef, i64 %{{[0-9]+}}, i64 0 +// CHECK-NEXT: %[[V2:[0-9]+]] = insertelement <2 x i64> %[[V1]], i64 %{{[0-9]+}}, i64 1 +// CHECK-NEXT: bitcast <2 x i64> %[[V2]] to <1 x i128> + +// CHECK-LE: %[[V1:[0-9]+]] = insertelement <2 x i64> undef, i64 %{{[0-9]+}}, i64 1 +// CHECK-NEXT-LE: %[[V2:[0-9]+]] = insertelement <2 x i64> %[[V1]], i64 %{{[0-9]+}}, i64 0 +// CHECK-NEXT-LE: bitcast <2 x i64> %[[V2]] to <1 x i128> + + __builtin_unpack_vector_int128(res_vslll, 0); +// CHECK: %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64> +// CHECK-NEXT: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 0 + +// CHECK-LE: %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64> +// CHECK-NEXT-LE: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 1 + + __builtin_unpack_vector_int128(res_vslll, 1); +// CHECK: %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64> +// CHECK-NEXT: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 1 + +// CHECK-LE: %[[V1:[0-9]+]] = bitcast <1 x i128> %{{[0-9]+}} to <2 x i64> +// CHECK-NEXT-LE: %{{[0-9]+}} = extractelement <2 x i64> %[[V1]], i32 0 + +} Index: clang/test/CodeGen/builtins-ppc-p7-disabled.c === --- clang/test/CodeGen/builtins-ppc-p7-disabled.c +++ clang/test/CodeGen/builtins-ppc-p7-disabled.c @@ -6,13 +6,17 @@ // RUN: not %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - 2>&1 \ // RUN: -target-cpu pwr7 | FileCheck %s -check-prefix=CHECK-32 +vector signed __int128 vslll = {33}; + void call_p7_builtins(void) { int a = __builtin_divwe(33, 11); unsigned int b = __builtin_divweu(33U, 11U); unsigned long long d = __builtin_divde(33ULL, 11ULL); unsigned long long e = __builtin_divdeu(33ULL, 11ULL); unsigned long long f = __builtin_bpermd(33ULL, 11ULL); + __builtin_pack_vector_int128(33ULL, 11ULL); + __builtin_unpack_vector_int128(vslll, 1); } // CHECK: error: this builtin is only valid on POWER7 or later CPUs @@ -25,6 +29,10 @@ // CHECK: __builtin_divdeu // CHECK: error: this builtin is only valid on POWER7 or later CPUs // CHECK: __builtin_bpermd +// CHECK: error: this builtin is only valid on POWER7 or later CPUs +// CHECK: __builtin_pack_vector_int128 +// CHECK: error: this builtin is only valid on POWER7 or later CPUs +// CHECK: __builtin_unpack_vector_int128 // CHECK-32: error: this builtin is only available on 64-bit targets // CHECK-32: __builtin_divde // CHECK-32: error: this builtin is only available on 64-bit targets Index: clang/test/CodeGen/builtins-ppc-error.c === --- clang/test/CodeGen/builtins-ppc-error.c +++ clang/test/CodeGen/builtins-ppc-error.c @@ -14,6 +14,7 @@ extern vector signed int vui; extern vector float vf; extern vector unsigned char vuc; +extern vector signed __int128 vsllli; void testInsertWord(void) { int index = 5; @@ -67,3 +68,8 @@ void testVCTUXS(int index) { vec_vctuxs(vf, index); //expected-error {{argument to '__builtin_altivec_vctuxs' must be a constant integer}} } + +void testUnpack128(int index) { + __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument to '__builtin_unpack_vector_int128' must be a constant integer}} + __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 5 is outside the valid range [0, 1]}} +} Index: clang/lib/Sema/SemaChecking.cpp === --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/
[PATCH] D52071: [clangd] Don't override the preamble while completing inside it, it doesn't work.
sammccall created this revision. sammccall added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. To stay fast, enable single-file-mode instead. This is fine since completions in the preamble are simple. The net effect for now is to suppress the spurious TopLevel completions when completing inside the preamble. Once Sema has include directive completion, this will be more important. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52071 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -657,6 +657,22 @@ UnorderedElementsAre(Named("local"), Named("preamble"))); } +// This verifies that we get normal preprocessor completions in the preamble. +// This is a regression test for an old bug: if we override the preamble and +// try to complete inside it, clang kicks our completion point just outside the +// preamble, resulting in always getting top-level completions. +TEST(CompletionTest, CompletionInPreamble) { + EXPECT_THAT(completions(R"cpp( +#ifnd^ef FOO_H_ +#define BAR_H_ +#include +int foo() {} +#endif +)cpp") + .Completions, + ElementsAre(Named("ifndef"))); +}; + TEST(CompletionTest, DynamicIndexMultiFile) { MockFSProvider FS; MockCompilationDatabase CDB; Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -40,6 +40,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Index/USRGeneration.h" +#include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" @@ -1053,11 +1054,19 @@ // We reuse the preamble whether it's valid or not. This is a // correctness/performance tradeoff: building without a preamble is slow, and // completion is latency-sensitive. + // However, if we're completing *inside* the preamble section of the draft, + // overriding the preamble will break sema completion. Fortunately we can just + // skip all includes in this case; these completions are really simple. + bool CompletingInPreamble = + ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size > + *Offset; // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise // the remapped buffers do not get freed. auto Clang = prepareCompilerInstance( - std::move(CI), Input.Preamble, std::move(ContentsBuffer), - std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer); + std::move(CI), CompletingInPreamble ? nullptr : Input.Preamble, + std::move(ContentsBuffer), std::move(Input.PCHs), std::move(Input.VFS), + DummyDiagsConsumer); + Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble; Clang->setCodeCompletionConsumer(Consumer.release()); SyntaxOnlyAction Action; Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -657,6 +657,22 @@ UnorderedElementsAre(Named("local"), Named("preamble"))); } +// This verifies that we get normal preprocessor completions in the preamble. +// This is a regression test for an old bug: if we override the preamble and +// try to complete inside it, clang kicks our completion point just outside the +// preamble, resulting in always getting top-level completions. +TEST(CompletionTest, CompletionInPreamble) { + EXPECT_THAT(completions(R"cpp( +#ifnd^ef FOO_H_ +#define BAR_H_ +#include +int foo() {} +#endif +)cpp") + .Completions, + ElementsAre(Named("ifndef"))); +}; + TEST(CompletionTest, DynamicIndexMultiFile) { MockFSProvider FS; MockCompilationDatabase CDB; Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -40,6 +40,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Index/USRGeneration.h" +#include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" @@ -1053,11 +1054,19 @@ // We reuse the preamble whether it's valid or not. This is a // correctness/performance tradeoff: building without a preamble is slow, and // completion is latency-sensitive. + // However, if we're completing *inside* the preamble section of the draft, + // overriding
[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.
shuaiwang added a comment. In https://reviews.llvm.org/D52008#1232923, @lebedev.ri wrote: > Thanks for working on this! I tried, and it appears to not fix the issue at > hand. > > - ``` struct C1 { C1(const C1* c, int num); }; > > int x = 0; auto y = std::make_unique(nullptr, x); // <- still > considered a mutation? ``` * ``` struct C3 {}; // some class > > struct C2 { C2(const int* whatever, int n, C3 zz); }; > > int x = 0; std::vector v; v.emplace_back(nullptr, x, {}); // <- still > considered a mutation? ``` > > And so on. These are hand-reduced, so hopefully you can reproduce? I've patched https://reviews.llvm.org/D51870 and tried these cases, they work correctly with this change plus the change making `std::move` & `std::forward` considered casts. I also tested struct D { D(int&); }; void h() { std::vector v; for (int i = 0; i < 10; ++i) { v.emplace_back(i); } } and that's also correctly considered as mutation at `emplace_back` Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:60 +public: + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); + JonasToth wrote: > shuaiwang wrote: > > JonasToth wrote: > > > Why do we need a separate class for this? > > > I think you can just create another object of `ExprMutAnalyzer` and do > > > the analysis with `findDeclMutation(FunctioParm)`. > > > > > > The `Stmt` is the `functionDecl().getBody()`. Right now it could be that > > > you pass in an functionDecl without body. > > > > > > Could there something happen with extern templates that the body is not > > > accessible? > > It's mostly for the special handling of constructors in which case > > initializer list also could mutate param. > Hmm, still why not within `ExprMutAnalyzer`? > You could make it a class living within ExprMutAnalyzer in the private > section. Then its clear its an implementation detail. Oh actually I'm planning to use this also in UnnecessaryValueParamCheck Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:201 equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(; + const auto NotInstantiated = unless(hasDeclaration(isInstantiated())); const auto AsNonConstRefArg = anyOf( JonasToth wrote: > I think this will not all cases correctly. > > Say > ``` > template > struct Foo { > static void bar() { SomethingWith(T()); } > }; > ``` > > `bar` it self is not a template and `NotInstantiated` will be `false` (only > 90% sure on that) as the declaration of `bar` does not match. > In another check I had to do this: `unless(has(expr(anyOf(isTypeDependent(), > isValueDependent()` to ensure that there are no unresolved constructs in > the stmt. I think it's fine here since we only care about skipping those with forwarding references. Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:367 +// function body, check them eagerly here since they're typically trivial. +for (const CXXCtorInitializer *Init : Ctor->inits()) { + ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context); JonasToth wrote: > Just to be sure, this will recurse ? > > ``` > struct Foo { > std::string s; > Foo(std::string s) : s (std::move(s)) {} > }; > ``` > > `std::move` will be resolved through the new mechanism right? This diff along won't handle `std::move` "properly" yet. We'll look into definition of `std::move`, it'll be something like ``` return static_cast::type&&>(t); ``` and we'll simply conclude `t` is mutated: because it's being returned. So for you case, we'll see `s` is mutated by `std::move(s)` and stop there, when we treat `std::move` as cast, we'll go one step further and found `std::move(s)` is passed as non-const argument to constructor of std::string, and we'll stop there concluding `s` is mutated. Repository: rC Clang https://reviews.llvm.org/D52008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.
shuaiwang updated this revision to Diff 165420. shuaiwang marked 10 inline comments as done. shuaiwang added a comment. More test cases addressing review comments Repository: rC Clang https://reviews.llvm.org/D52008 Files: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h lib/Analysis/ExprMutationAnalyzer.cpp unittests/Analysis/ExprMutationAnalyzerTest.cpp Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp === --- unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -595,6 +595,96 @@ EXPECT_FALSE(isMutated(Results, AST.get())); } +TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) { + auto AST = + tooling::buildASTFromCode("template void g(T&& t) { t = 10; }" +"void f() { int x; g(x); }"); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)")); + + AST = tooling::buildASTFromCode( + "void h(int&);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)")); + + AST = tooling::buildASTFromCode( + "void h(int&, int);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x, y; g(x, y); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x, y)")); + Results = match(withEnclosingCompound(declRefTo("y")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode( + "void h(int, int&);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x, y; g(y, x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(y, x)")); + Results = match(withEnclosingCompound(declRefTo("y")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode( + "struct S { template S(T&& t) { t = 10; } };" + "void f() { int x; S s(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x")); + + AST = tooling::buildASTFromCode( + "struct S { template S(T&& t) : m(++t) { } int m; };" + "void f() { int x; S s(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x")); +} + +TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) { + auto AST = tooling::buildASTFromCode("template void g(T&&) {}" + "void f() { int x; g(x); }"); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode("template void g(T&& t) { t; }" + "void f() { int x; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = + tooling::buildASTFromCode("template void g(Args&&...) {}" +"void f() { int x; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = + tooling::buildASTFromCode("template void g(Args&&...) {}" +"void f() { int y, x; g(y, x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode( + "void h(int, int&);" + "template void g(Args&&... args) { h(args...); }" + "void f() { int x, y; g(x, y); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode( + "struct S { template S(T&& t) { t; } };" + "void f() { int x; S s(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = tooling::buildASTFromCode( + "struct S { template S(T&& t) : m(t) { } int m; };" + "void f() { int x; S s(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); +} + TEST(ExprMutationAnalyzerTest, ArrayElementModified) { const auto AST = tooling::buildASTFromCode("void f() { int x[2]; x[0] = 10; }"); Index: lib/Analysis/ExprMutationAnalyzer.cpp ==
[PATCH] D51905: Front-end of the implementation of the interleaving algorithm
zhaomo updated this revision to Diff 165415. zhaomo added a comment. Fixed issues pointed out by Peter and Vlad. Richard: I just saw comments but I will travel tomorrow and this weekend so I will reply to your comments when I get a chance. https://reviews.llvm.org/D51905 Files: clang/include/clang/Driver/CC1Options.td clang/include/clang/Frontend/CodeGenOptions.h clang/lib/CodeGen/CGCXXABI.h clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGVTables.cpp clang/lib/CodeGen/CodeGenFunction.h clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/ItaniumCXXABI.cpp clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/tbaa-for-vptr.cpp clang/test/CodeGenCXX/alignment.cpp clang/test/CodeGenCXX/arm.cpp clang/test/CodeGenCXX/cfi-cross-dso.cpp clang/test/CodeGenCXX/constructor-destructor-return-this.cpp clang/test/CodeGenCXX/constructor-init.cpp clang/test/CodeGenCXX/delete.cpp clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp clang/test/CodeGenCXX/interleaving-member-function-pointers.cpp clang/test/CodeGenCXX/interleaving-virtual-base.cpp clang/test/CodeGenCXX/interleaving-virtual-calls.cpp clang/test/CodeGenCXX/ubsan-vtable-checks.cpp clang/test/CodeGenCXX/virtual-base-cast.cpp compiler-rt/test/cfi/CMakeLists.txt compiler-rt/test/cfi/anon-namespace.cpp compiler-rt/test/cfi/cross-dso/lit.local.cfg compiler-rt/test/cfi/icall/lit.local.cfg compiler-rt/test/cfi/lit.cfg compiler-rt/test/cfi/lit.site.cfg.in compiler-rt/test/cfi/mfcall.cpp compiler-rt/test/cfi/multiple-inheritance.cpp compiler-rt/test/cfi/simple-fail.cpp compiler-rt/test/cfi/vdtor.cpp compiler-rt/test/lit.common.configured.in Index: compiler-rt/test/lit.common.configured.in === --- compiler-rt/test/lit.common.configured.in +++ compiler-rt/test/lit.common.configured.in @@ -35,6 +35,7 @@ set_default("use_thinlto", False) set_default("use_lto", config.use_thinlto) set_default("use_newpm", False) +set_default("use_interleaving", False) set_default("android", @ANDROID_PYBOOL@) config.available_features.add('target-is-%s' % config.target_arch) Index: compiler-rt/test/cfi/vdtor.cpp === --- compiler-rt/test/cfi/vdtor.cpp +++ compiler-rt/test/cfi/vdtor.cpp @@ -14,7 +14,7 @@ // RUN: %run %t5 2>&1 | FileCheck --check-prefix=NCFI %s // RUN: %clangxx_cfi_diag -o %t6 %s -// RUN: %run %t6 2>&1 | FileCheck --check-prefix=CFI-DIAG %s +// RUN: %interleave_diag %run %t6 2>&1 | FileCheck --check-prefix=CFI-DIAG %s // Tests that the CFI enforcement also applies to virtual destructor calls made // via 'delete'. Index: compiler-rt/test/cfi/simple-fail.cpp === --- compiler-rt/test/cfi/simple-fail.cpp +++ compiler-rt/test/cfi/simple-fail.cpp @@ -47,12 +47,12 @@ // RUN: %expect_crash %run %t16 2>&1 | FileCheck --check-prefix=CFI %s // RUN: %clangxx_cfi_diag -o %t17 %s -// RUN: %run %t17 2>&1 | FileCheck --check-prefix=CFI-DIAG %s +// RUN: %interleave_diag %run %t17 2>&1 | FileCheck --check-prefix=CFI-DIAG %s // RUN: %clangxx -o %t18 %s // RUN: %run %t18 2>&1 | FileCheck --check-prefix=NCFI %s -// RUN: %clangxx_cfi -DCHECK_NO_SANITIZE_CFI -o %t19 %s +// RUN: %clangxx_cfi_no_interleaving -DCHECK_NO_SANITIZE_CFI -o %t19 %s // RUN: %run %t19 2>&1 | FileCheck --check-prefix=NCFI %s // Tests that the CFI mechanism crashes the program when making a virtual call @@ -95,6 +95,7 @@ // CFI-DIAG-NEXT: note: vtable is of type '{{(struct )?}}A' ((B *)a)->f(); // UB here + // INTERLEAVING-NCFI-NOT: {{^2$}} // CFI-NOT: {{^2$}} // NCFI: {{^2$}} fprintf(stderr, "2\n"); Index: compiler-rt/test/cfi/multiple-inheritance.cpp === --- compiler-rt/test/cfi/multiple-inheritance.cpp +++ compiler-rt/test/cfi/multiple-inheritance.cpp @@ -20,7 +20,7 @@ // RUN: %clangxx_cfi_diag -o %t6 %s // RUN: %run %t6 2>&1 | FileCheck --check-prefix=CFI-DIAG2 %s -// RUN: %run %t6 x 2>&1 | FileCheck --check-prefix=CFI-DIAG1 %s +// RUN: %interleave_diag %run %t6 x 2>&1 | FileCheck --check-prefix=CFI-DIAG1 %s // Tests that the CFI mechanism is sensitive to multiple inheritance and only // permits calls via virtual tables for the correct base class. Index: compiler-rt/test/cfi/mfcall.cpp === --- compiler-rt/test/cfi/mfcall.cpp +++ compiler-rt/test/cfi/mfcall.cpp @@ -1,4 +1,4 @@ -// UNSUPPORTED: windows-msvc +// UNSUPPORTED: windows-msvc, interleaving // RUN: %clangxx_cfi -o %t %s // RUN: %expect_crash %run %t a Index: compiler-rt/test/cfi/lit.site.cfg.in === --- compiler-rt/test/cfi/lit.site.cfg.in +++ compiler-rt/test/cfi/lit.site.cfg.in @@ -8,6 +8,7 @@
[PATCH] D51905: Front-end of the implementation of the interleaving algorithm
zhaomo added a comment. In https://reviews.llvm.org/D51905#1231383, @vlad.tsyrklevich wrote: > This change causes all compiler-rt cfi tests to be UNSUPPORTED for me > locally, do you have any idea why that might be? The lit changes don't make > it immediately clear. Not sure why that happened. How did you run the compiler-rt tests? Did you use ninja check-cfi? https://reviews.llvm.org/D51905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51680: [analyzer][UninitializedObjectChecker] New flag to ignore records based on it's fields
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I hope we'll be able to come up with a reasonable default regex. Comment at: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp:1-4 +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind" \ +// RUN: -std=c++11 -verify %s If you like, you can make a directory for your tests, instead of a filename prefix. Btw, thanks for pointing out that we can use `\` in run-lines, i never noticed it before you started using it. https://reviews.llvm.org/D51680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51679: [analyzer][UninitializedObjectChecker] Refactored checker options
NoQ accepted this revision. NoQ added inline comments. This revision is now accepted and ready to land. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h:61-63 + bool IsPedantic; + bool ShouldConvertNotesToWarnings; + bool CheckPointeeInitialization; `... = false;`, just to initialize fields in the constructor :] Repository: rC Clang https://reviews.llvm.org/D51679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once
NoQ requested changes to this revision. NoQ added a comment. This revision now requires changes to proceed. Or, wait, hmm, no, we can still de-duplicate the reports with a global set stored in the checker object. Because we would probably like to de-duplicate across different paths as well. https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Hmm, i guess it's necessary to de-duplicate across multiple reports as well. https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52015: [XRay][clang] Emit "never-instrument" attribute
This revision was automatically updated to reflect the committed changes. Closed by commit rC342200: [XRay][clang] Emit "never-instrument" attribute (authored by dberris, committed by ). Changed prior to commit: https://reviews.llvm.org/D52015?vs=165407&id=165408#toc Repository: rC Clang https://reviews.llvm.org/D52015 Files: lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp test/CodeGen/xray-attributes-noxray-supported.cpp test/CodeGen/xray-attributes-supported.cpp Index: lib/CodeGen/CodeGenModule.cpp === --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -1967,9 +1967,6 @@ bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc, StringRef Category) const { - if (!LangOpts.XRayInstrument) -return false; - const auto &XRayFilter = getContext().getXRayFilter(); using ImbueAttr = XRayFunctionFilter::ImbueAttribute; auto Attr = ImbueAttr::NONE; Index: lib/CodeGen/CodeGenFunction.cpp === --- lib/CodeGen/CodeGenFunction.cpp +++ lib/CodeGen/CodeGenFunction.cpp @@ -901,21 +901,21 @@ } // Apply xray attributes to the function (as a string, for now) - bool InstrumentXray = ShouldXRayInstrumentFunction() && -CGM.getCodeGenOpts().XRayInstrumentationBundle.has( -XRayInstrKind::Function); - if (D && InstrumentXray) { + if (D) { if (const auto *XRayAttr = D->getAttr()) { - if (XRayAttr->alwaysXRayInstrument()) -Fn->addFnAttr("function-instrument", "xray-always"); - if (XRayAttr->neverXRayInstrument()) -Fn->addFnAttr("function-instrument", "xray-never"); - if (const auto *LogArgs = D->getAttr()) { -Fn->addFnAttr("xray-log-args", - llvm::utostr(LogArgs->getArgumentCount())); + if (CGM.getCodeGenOpts().XRayInstrumentationBundle.has( + XRayInstrKind::Function)) { +if (XRayAttr->alwaysXRayInstrument() && ShouldXRayInstrumentFunction()) + Fn->addFnAttr("function-instrument", "xray-always"); +if (XRayAttr->neverXRayInstrument()) + Fn->addFnAttr("function-instrument", "xray-never"); +if (const auto *LogArgs = D->getAttr()) + if (ShouldXRayInstrumentFunction()) +Fn->addFnAttr("xray-log-args", + llvm::utostr(LogArgs->getArgumentCount())); } } else { - if (!CGM.imbueXRayAttrs(Fn, Loc)) + if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc)) Fn->addFnAttr( "xray-instruction-threshold", llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold)); Index: test/CodeGen/xray-attributes-supported.cpp === --- test/CodeGen/xray-attributes-supported.cpp +++ test/CodeGen/xray-attributes-supported.cpp @@ -1,10 +1,17 @@ -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s // Make sure that the LLVM attribute for XRay-annotated functions do show up.
[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once
NoQ added a comment. Hmm, so we're reporting the same region twice in the same bug report through different field chains, right? Why is a state trait necessary here? Maybe just de-duplicate them locally before throwing the report? https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342200 - [XRay][clang] Emit "never-instrument" attribute
Author: dberris Date: Thu Sep 13 18:59:12 2018 New Revision: 342200 URL: http://llvm.org/viewvc/llvm-project?rev=342200&view=rev Log: [XRay][clang] Emit "never-instrument" attribute Summary: Before this change, we only emit the XRay attributes in LLVM IR when the -fxray-instrument flag is provided. This may cause issues with thinlto when the final binary is being built/linked with -fxray-instrument, and the constitutent LLVM IR gets re-lowered with xray instrumentation. With this change, we can honour the "never-instrument "attributes provided in the source code and preserve those in the IR. This way, even in thinlto builds, we retain the attributes which say whether functions should never be XRay instrumented. This change addresses llvm.org/PR38922. Reviewers: mboerger, eizan Subscribers: mehdi_amini, dexonsmith, cfe-commits, llvm-commits Differential Revision: https://reviews.llvm.org/D52015 Added: cfe/trunk/test/CodeGen/xray-attributes-noxray-supported.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/test/CodeGen/xray-attributes-supported.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=342200&r1=342199&r2=342200&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Sep 13 18:59:12 2018 @@ -901,21 +901,21 @@ void CodeGenFunction::StartFunction(Glob } // Apply xray attributes to the function (as a string, for now) - bool InstrumentXray = ShouldXRayInstrumentFunction() && -CGM.getCodeGenOpts().XRayInstrumentationBundle.has( -XRayInstrKind::Function); - if (D && InstrumentXray) { + if (D) { if (const auto *XRayAttr = D->getAttr()) { - if (XRayAttr->alwaysXRayInstrument()) -Fn->addFnAttr("function-instrument", "xray-always"); - if (XRayAttr->neverXRayInstrument()) -Fn->addFnAttr("function-instrument", "xray-never"); - if (const auto *LogArgs = D->getAttr()) { -Fn->addFnAttr("xray-log-args", - llvm::utostr(LogArgs->getArgumentCount())); + if (CGM.getCodeGenOpts().XRayInstrumentationBundle.has( + XRayInstrKind::Function)) { +if (XRayAttr->alwaysXRayInstrument() && ShouldXRayInstrumentFunction()) + Fn->addFnAttr("function-instrument", "xray-always"); +if (XRayAttr->neverXRayInstrument()) + Fn->addFnAttr("function-instrument", "xray-never"); +if (const auto *LogArgs = D->getAttr()) + if (ShouldXRayInstrumentFunction()) +Fn->addFnAttr("xray-log-args", + llvm::utostr(LogArgs->getArgumentCount())); } } else { - if (!CGM.imbueXRayAttrs(Fn, Loc)) + if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc)) Fn->addFnAttr( "xray-instruction-threshold", llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold)); Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=342200&r1=342199&r2=342200&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Sep 13 18:59:12 2018 @@ -1967,9 +1967,6 @@ bool CodeGenModule::isInSanitizerBlackli bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc, StringRef Category) const { - if (!LangOpts.XRayInstrument) -return false; - const auto &XRayFilter = getContext().getXRayFilter(); using ImbueAttr = XRayFunctionFilter::ImbueAttribute; auto Attr = ImbueAttr::NONE; Added: cfe/trunk/test/CodeGen/xray-attributes-noxray-supported.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/xray-attributes-noxray-supported.cpp?rev=342200&view=auto == --- cfe/trunk/test/CodeGen/xray-attributes-noxray-supported.cpp (added) +++ cfe/trunk/test/CodeGen/xray-attributes-noxray-supported.cpp Thu Sep 13 18:59:12 2018 @@ -0,0 +1,29 @@ +// We want to ensure that the "never instrument" attributes show up even if we +// explicitly turn off XRay instrumentation. +// +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray
[PATCH] D51057: [analyzer][UninitializedObjectChecker] Fixed dereferencing
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks good i guess. I'm glad this is sorted out^^ https://reviews.llvm.org/D51057 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52015: [XRay][clang] Emit "never-instrument" attribute
dberris updated this revision to Diff 165407. dberris retitled this revision from "[XRay][clang] Always emit XRay attributes for LLVM IR" to "[XRay][clang] Emit "never-instrument" attribute". dberris added a comment. Retitle, add different test case for -fno-xray-instrument. https://reviews.llvm.org/D52015 Files: clang/lib/CodeGen/CodeGenFunction.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/test/CodeGen/xray-attributes-noxray-supported.cpp clang/test/CodeGen/xray-attributes-supported.cpp compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc Index: compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc === --- /dev/null +++ compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc @@ -0,0 +1,11 @@ +// Test that we cannot actually find XRay instrumentation when we build with +// -fno-xray-instrument but have code that's marked as 'xray_always_instrument'. +// +// RUN: %clangxx -fno-xray-instrument -c %s -o %t.o +// RUN: not %llvm_xray extract -symbolize %t.o 2>&1 | FileCheck %s +// REQUIRES: x86_64-target-arch +// REQUIRES: built-in-llvm-tree + +// CHECK: llvm-xray: Cannot extract instrumentation map +// CHECK-NOT: {{.*always_instrumented.*}} +[[clang::xray_always_instrument]] int always_instrumented() { return 42; } Index: clang/test/CodeGen/xray-attributes-supported.cpp === --- clang/test/CodeGen/xray-attributes-supported.cpp +++ clang/test/CodeGen/xray-attributes-supported.cpp @@ -1,10 +1,17 @@ -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s -// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s // Make sure that the LLVM attribute for XRay-annotated functions do show up. [[clang::xray_always_instrument]] void foo() { Index: clang/test/CodeGen/xray-attributes-noxray-supported.cpp === --- /dev/null +++ clang/test/CodeGen/xray-attributes-noxray-supported.cpp @@ -0,0 +1,29 @@ +// We want to ensure that the "never instrument" attributes show up even if we +// explicitly turn off XRay instrumentation. +// +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s +// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \ +// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s + +[[clang::xray_always_instrument]] voi
[PATCH] D51822: Support generating unique identifiers for Stmt objects
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks as expected. I hope that assertion does actually hold. Should AST folks have a look as well? Comment at: clang/lib/AST/Stmt.cpp:310 + return *Out / alignof(Stmt); + +} Redundant empty line. https://reviews.llvm.org/D51822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52066: [Driver] Fix missing MultiArch include dir on powerpcspe
glaubitz created this revision. glaubitz added reviewers: kristina, dschuff. Herald added subscribers: cfe-commits, nemanjai. On powerpc-linux-gnuspe, the header files are located in their own include directory named /usr/lib/powerpc-linux-gnuspe, so add this directory to PPCMultiarchIncludeDirs. Repository: rC Clang https://reviews.llvm.org/D52066 Files: lib/Driver/ToolChains/Linux.cpp Index: lib/Driver/ToolChains/Linux.cpp === --- lib/Driver/ToolChains/Linux.cpp +++ lib/Driver/ToolChains/Linux.cpp @@ -699,7 +699,8 @@ "/usr/include/mips64el-linux-gnu", "/usr/include/mips64el-linux-gnuabi64"}; const StringRef PPCMultiarchIncludeDirs[] = { - "/usr/include/powerpc-linux-gnu"}; + "/usr/include/powerpc-linux-gnu", + "/usr/include/powerpc-linux-gnuspe"}; const StringRef PPC64MultiarchIncludeDirs[] = { "/usr/include/powerpc64-linux-gnu"}; const StringRef PPC64LEMultiarchIncludeDirs[] = { Index: lib/Driver/ToolChains/Linux.cpp === --- lib/Driver/ToolChains/Linux.cpp +++ lib/Driver/ToolChains/Linux.cpp @@ -699,7 +699,8 @@ "/usr/include/mips64el-linux-gnu", "/usr/include/mips64el-linux-gnuabi64"}; const StringRef PPCMultiarchIncludeDirs[] = { - "/usr/include/powerpc-linux-gnu"}; + "/usr/include/powerpc-linux-gnu", + "/usr/include/powerpc-linux-gnuspe"}; const StringRef PPC64MultiarchIncludeDirs[] = { "/usr/include/powerpc64-linux-gnu"}; const StringRef PPC64LEMultiarchIncludeDirs[] = { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342199 - [ODRHash] Fix early exit that skipped code.
Author: rtrieu Date: Thu Sep 13 18:15:28 2018 New Revision: 342199 URL: http://llvm.org/viewvc/llvm-project?rev=342199&view=rev Log: [ODRHash] Fix early exit that skipped code. There is a bit of code at the end of AddDeclaration that should be run on every exit of the function. However, there was an early exit beforehand that could be triggered, which causes a small amount of data to skip the hashing, leading to false positive mismatch. Use a separate function so that this code is always run. Modified: cfe/trunk/include/clang/AST/ODRHash.h cfe/trunk/lib/AST/ODRHash.cpp cfe/trunk/test/Modules/Inputs/odr_hash-Unresolved/class.h Modified: cfe/trunk/include/clang/AST/ODRHash.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ODRHash.h?rev=342199&r1=342198&r2=342199&view=diff == --- cfe/trunk/include/clang/AST/ODRHash.h (original) +++ cfe/trunk/include/clang/AST/ODRHash.h Thu Sep 13 18:15:28 2018 @@ -91,6 +91,9 @@ public: void AddBoolean(bool value); static bool isWhitelistedDecl(const Decl* D, const DeclContext *Parent); + +private: + void AddDeclarationNameImpl(DeclarationName Name); }; } // end namespace clang Modified: cfe/trunk/lib/AST/ODRHash.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=342199&r1=342198&r2=342199&view=diff == --- cfe/trunk/lib/AST/ODRHash.cpp (original) +++ cfe/trunk/lib/AST/ODRHash.cpp Thu Sep 13 18:15:28 2018 @@ -34,8 +34,17 @@ void ODRHash::AddIdentifierInfo(const Id void ODRHash::AddDeclarationName(DeclarationName Name, bool TreatAsDecl) { if (TreatAsDecl) +// Matches the NamedDecl check in AddDecl AddBoolean(true); + AddDeclarationNameImpl(Name); + + if (TreatAsDecl) +// Matches the ClassTemplateSpecializationDecl check in AddDecl +AddBoolean(false); +} + +void ODRHash::AddDeclarationNameImpl(DeclarationName Name) { // Index all DeclarationName and use index numbers to refer to them. auto Result = DeclNameMap.insert(std::make_pair(Name, DeclNameMap.size())); ID.AddInteger(Result.first->second); @@ -91,9 +100,6 @@ void ODRHash::AddDeclarationName(Declara } } } - - if (TreatAsDecl) -AddBoolean(false); } void ODRHash::AddNestedNameSpecifier(const NestedNameSpecifier *NNS) { Modified: cfe/trunk/test/Modules/Inputs/odr_hash-Unresolved/class.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/odr_hash-Unresolved/class.h?rev=342199&r1=342198&r2=342199&view=diff == --- cfe/trunk/test/Modules/Inputs/odr_hash-Unresolved/class.h (original) +++ cfe/trunk/test/Modules/Inputs/odr_hash-Unresolved/class.h Thu Sep 13 18:15:28 2018 @@ -6,6 +6,7 @@ class S { void run() { int x; A::Check(&Field, 1); +A::Check(&Field, 1); } }; #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51905: Front-end of the implementation of the interleaving algorithm
rsmith added inline comments. Comment at: clang/include/clang/Driver/CC1Options.td:356-357 +HelpText<"Enable VTable interleaving">; +def disable_vtable_interleaving : Flag<["-"], "disable-vtable-interleaving">, +HelpText<"Disable VTable interleaving">; //===--===// We usually only expose the non-default flag in `-cc1`, so that there are no ordering concerns and we can determine whether a feature is enabled with just `hasArg`. Also, `-fvtable-interleaving` would seem like a more natural flag name to me. Comment at: clang/include/clang/Frontend/CodeGenOptions.h:274 + /// the interleaving layout is decided. + bool EnableVTableInterleaving; + `Enable` here seems redundant. Is thee a reason to declare this explicitly rather than adding it to CodeGenOptions.def? Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:973-976 +// The name of a offset global variable has the format "__[type +// id]$[offset]". +std::string Name = +"__" + RD->getNameAsString() + "$" + std::to_string(Offset); The `__`s here are atypical. If you want a name that can't collide with a name generated by the frontend, putting a period in it is the usual strategy. Also, a name prefix that indicates what this global is for would make the IR more readable and make this less likely to collide with other things. Maybe `clang.vtable.index..` or similar? Also, there's no guarantee that `RD->getNameAsString()` produces something unique within the translation unit, so you'll need to make sure that name collisions are handled somehow (I *think* `GlobalVariable` already deals with this for you, but I'm not sure... however, I think its strategy is to add a number to the end of the mangling, which will lead to some quite peculiar results given that you already have a number there). It might be easier to base the name of this symbol on the mangled name of the class; then you could give the global variable the same linkage as the class, which might save your interleaving pass a little bit of work later on as the duplicates will already have been combined -- but that's up to you, and I don't have enough of the big picture here to give you advice. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:987 + } + return llvm::ConstantExpr::getPtrToInt(OffsetGV, OffsetType); +} Representing this offset as a `ptrtoint` on a global variable seems really strange. Is there really no better way to model this in IR? (I mean, if not, then carry on as you are, but this seems like a hack.) Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1767 + bool InBounds = shouldInterleaveVTables(VTableClass) ? false : true; return llvm::ConstantExpr::getGetElementPtr(VTable->getValueType(), VTable, zhaomo wrote: > pcc wrote: > > Remind me why this needs to be here? (And the explanation needs to be in a > > comment.) > The calculation of address point is essentially base_vtable_addr + offset, > where offset is from the indices of gep. > In the interleaving pass, we replace base_vtable_addr with > (addr_point_in_interleaved_layout - offset). > > The LLVM language reference says that the base address of a inbounds gep must > be an in bound address of the object. The new base address > addr_point_in_interleaved_layout - offset, however, may not be an in bound > address. I still find the need for this confusing. Suppose we have: ``` struct A { virtual void f(); }; struct B { virtual void g(); virtual void h(); }; struct C : A, B {}; ``` such that `C` has a naive vtable containing 7 elements {{offset, typeid, &A::f}, {offset, typeid, &B::f, &B::h}}, with address point indexes (0, 2) and (1, 2). Here we emit a `gep inbounds @vtable, 0, 1, 2` to get the B-in-C vtable. Now, if this were instead emitted as `gep inbounds (cast @vtable to i8*), 24`, and the interleaving process were replacing @vtable with a non-`inbounds` GEP on its interleaved ubervtable, I could understand the need to drop `inbounds` here -- because the replacement for @vtable might be before the start of the ubervtable. But based on my reading of the algorithm description, that's not what happens: first we split `@vtable` into multiple individual vtables, and that splitting process presumably must identify these GEPs to figure out which slice of the vtable they need. That presumably either removes this GEP entirely, or leaves this being a correct and trivial `inbounds` GEP on a split-out piece of the vtable. (I'm imagining we effectively rewrite `gep inbounds @vtable, 0, 1, 2` as `@vtable-slice-2` as part of the splitting.) And then after interleaving, I'm imagining we replace all uses of `@vtable-slice-2` with the appropriate address point in the ubervtable, which is again an `inbounds` GEP (but that would b
[clang-tools-extra] r342198 - [clangd] Fix TUScheduler typos
Author: maskray Date: Thu Sep 13 17:56:11 2018 New Revision: 342198 URL: http://llvm.org/viewvc/llvm-project?rev=342198&view=rev Log: [clangd] Fix TUScheduler typos Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp clang-tools-extra/trunk/clangd/TUScheduler.h Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=342198&r1=342197&r2=342198&view=diff == --- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original) +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Thu Sep 13 17:56:11 2018 @@ -18,7 +18,7 @@ // preamble. However, unlike AST, the same preamble can be read concurrently, so // we run each of async preamble reads on its own thread. // -// To limit the concurrent load that clangd produces we mantain a semaphore that +// To limit the concurrent load that clangd produces we maintain a semaphore that // keeps more than a fixed number of threads from running concurrently. // // Rationale for cancelling updates. @@ -165,7 +165,7 @@ public: /// The processing thread is spawned using \p Tasks. However, when \p Tasks /// is null, all requests will be processed on the calling thread /// synchronously instead. \p Barrier is acquired when processing each - /// request, it is be used to limit the number of actively running threads. + /// request, it is used to limit the number of actively running threads. static ASTWorkerHandle create(PathRef FileName, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks, Semaphore &Barrier, @@ -188,7 +188,7 @@ public: void getCurrentPreamble( llvm::unique_function)>); /// Wait for the first build of preamble to finish. Preamble itself can be - /// accessed via getPossibleStalePreamble(). Note that this function will + /// accessed via getPossiblyStalePreamble(). Note that this function will /// return after an unsuccessful build of the preamble too, i.e. result of /// getPossiblyStalePreamble() can be null even after this function returns. void waitForFirstPreamble() const; @@ -207,7 +207,7 @@ private: void startTask(llvm::StringRef Name, llvm::unique_function Task, llvm::Optional UpdateType); /// Determines the next action to perform. - /// All actions that should never run are disarded. + /// All actions that should never run are discarded. /// Returns a deadline for the next action. If it's expired, run now. /// scheduleLocked() is called again at the deadline, or if requests arrive. Deadline scheduleLocked(); @@ -227,7 +227,7 @@ private: const bool RunSync; /// Time to wait after an update to see whether another update obsoletes it. const steady_clock::duration UpdateDebounce; - /// File that ASTWorker is reponsible for. + /// File that ASTWorker is responsible for. const Path FileName; /// Whether to keep the built preambles in memory or on disk. const bool StorePreambleInMemory; @@ -417,7 +417,7 @@ void ASTWorker::update( // We want to report the diagnostics even if this update was cancelled. // It seems more useful than making the clients wait indefinitely if they // spam us with updates. -// Note *AST can be still be null if buildAST fails. +// Note *AST can still be null if buildAST fails. if (*AST) { OnUpdated((*AST)->getDiagnostics()); trace::Span Span("Running main AST callback"); Modified: clang-tools-extra/trunk/clangd/TUScheduler.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=342198&r1=342197&r2=342198&view=diff == --- clang-tools-extra/trunk/clangd/TUScheduler.h (original) +++ clang-tools-extra/trunk/clangd/TUScheduler.h Thu Sep 13 17:56:11 2018 @@ -176,7 +176,7 @@ private: }; /// Runs \p Action asynchronously with a new std::thread. The context will be -/// propogated. +/// propagated. template std::future runAsync(llvm::unique_function Action) { return std::async(std::launch::async, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51824: StmtPrinter: allow customizing the end-of-line character
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great to me and pretty trivial, should somebody who's more familiar with this code take a look? Comment at: clang/lib/AST/StmtPrinter.cpp:72 PrintingPolicy Policy; +std::string NewlineSymbol; const ASTContext *Context; This variable is used so many times, maybe give it a shorter name? Repository: rC Clang https://reviews.llvm.org/D51824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51865: [clang-tidy] Added a test -export-fixes with relative paths.
steveire accepted this revision. steveire added a comment. This revision is now accepted and ready to land. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342196 - Relax alignment assumptions in a test after r342194
Author: vedantk Date: Thu Sep 13 17:16:37 2018 New Revision: 342196 URL: http://llvm.org/viewvc/llvm-project?rev=342196&view=rev Log: Relax alignment assumptions in a test after r342194 Don't hardcode align 8. Fixes a bot failure: http://lab.llvm.org:8011/builders/clang-cmake-armv8-full/builds/6373 Modified: cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp Modified: cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp?rev=342196&r1=342195&r2=342196&view=diff == --- cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp (original) +++ cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp Thu Sep 13 17:16:37 2018 @@ -4,8 +4,8 @@ // CHECK-LABEL: define{{.*}}lambda_in_func void lambda_in_func(int &ref) { // CHECK: [[ref_slot:%.*]] = getelementptr inbounds %class.anon, %class.anon* {{.*}}, i32 0, i32 0, !dbg [[lambda_decl_loc:![0-9]+]] - // CHECK-NEXT: %1 = load i32*, i32** %ref.addr, align 8, !dbg [[capture_init_loc:![0-9]+]] - // CHECK-NEXT: store i32* %1, i32** %0, align 8, !dbg [[lambda_decl_loc]] + // CHECK-NEXT: %1 = load i32*, i32** %ref.addr, align {{.*}}, !dbg [[capture_init_loc:![0-9]+]] + // CHECK-NEXT: store i32* %1, i32** %0, align {{.*}}, !dbg [[lambda_decl_loc]] // CHECK-NEXT: call void {{.*}}, !dbg [[lambda_call_loc:![0-9]+]] auto helper = [ // CHECK: [[lambda_decl_loc]] = !DILocation(line: [[@LINE]], column: 17 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52036: [Analyzer] Make plist tests less fragile
NoQ added a comment. What do you think about the following approach: 1. Define a `lit` substitution `%diff_plist` that resolves to `diff -u -w -I "/" -I ".:" -I "version" -`. 2. Update these flags in your downstream fork. It's kinda cool because it'll allow us to test exactly what we want in the exact output that we normally expect ourselves to produce. Step 1 is a good thing to do anyway because it reduces duplication. https://reviews.llvm.org/D52036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52064: [Sema] Add a note pointing to the first use of an implicit capture
vsk updated this revision to Diff 165403. vsk marked an inline comment as done. https://reviews.llvm.org/D52064 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Sema/SemaLambda.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp clang/test/SemaCXX/lambda-expressions.cpp Index: clang/test/SemaCXX/lambda-expressions.cpp === --- clang/test/SemaCXX/lambda-expressions.cpp +++ clang/test/SemaCXX/lambda-expressions.cpp @@ -77,8 +77,18 @@ struct G { G(); G(G&); int a; }; // expected-note 6 {{not viable}} G g; [=]() { const G* gg = &g; return gg->a; }; -[=]() { return [=]{ const G* gg = &g; return gg->a; }(); }; // expected-error {{no matching constructor for initialization of 'G'}} -(void)^{ return [=]{ const G* gg = &g; return gg->a; }(); }; // expected-error 2 {{no matching constructor for initialization of 'const G'}} +[=]() { + return [=]{ // expected-error {{no matching constructor for initialization of 'G'}} +const G* gg = &g; // expected-note {{implicitly capturing 'g', first used here}} +return gg->a; + }(); +}; +(void)^{ + return [=]{ // expected-error {{no matching constructor for initialization of 'const G'}} +const G* gg = &g; // expected-error {{no matching constructor for initialization of 'const G'}} expected-note {{implicitly capturing 'g', first used here}} +return gg->a; + }(); +}; const int h = a; // expected-note {{declared}} []() { return h; }; // expected-error {{variable 'h' cannot be implicitly captured in a lambda with no capture-default specified}} expected-note {{lambda expression begins here}} @@ -378,7 +388,9 @@ namespace PR18473 { template void f() { T t(0); -(void) [=]{ int n = t; }; // expected-error {{deleted}} +(void)[=] { // expected-error {{call to deleted constructor of 'PR18473::NoCopy'}} + int n = t; // expected-note {{implicitly capturing 't', first used here}} +}; } template void f(); Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp === --- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp +++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp @@ -16,7 +16,7 @@ void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) { (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}} (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} -ncr.foo(); +ncr.foo(); // expected-note{{implicitly capturing 'ncr', first used here}} }(); [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}} Index: clang/lib/Sema/SemaTemplateInstantiate.cpp === --- clang/lib/Sema/SemaTemplateInstantiate.cpp +++ clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -200,6 +200,7 @@ case DeclaringSpecialMember: case DefiningSynthesizedFunction: case ExceptionSpecEvaluation: + case ImplicitLambdaCaptureInitialization: return false; // This function should never be called when Kind's value is Memoization. @@ -653,6 +654,13 @@ break; } +case CodeSynthesisContext::ImplicitLambdaCaptureInitialization: { + Diags.Report(Active->PointOfInstantiation, + diag::note_implicitly_capturing_var_first_used_here) + << cast(Active->Entity); + break; +} + case CodeSynthesisContext::Memoization: break; } @@ -698,6 +706,7 @@ case CodeSynthesisContext::DeclaringSpecialMember: case CodeSynthesisContext::DefiningSynthesizedFunction: +case CodeSynthesisContext::ImplicitLambdaCaptureInitialization: // This happens in a context unrelated to template instantiation, so // there is no SFINAE. return None; Index: clang/lib/Sema/SemaLambda.cpp === --- clang/lib/Sema/SemaLambda.cpp +++ clang/lib/Sema/SemaLambda.cpp @@ -21,6 +21,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "clang/Sema/SemaLambda.h" +#include "llvm/ADT/ScopeExit.h" using namespace clang; using namespace sema; @@ -1401,6 +1402,18 @@ SourceLocation Loc = IsImplicitCapture ? ImplicitCaptureLoc : Capture.getLocation(); + if (IsImplicitCapture) { +Sema::CodeSynthesisContext Ctx; +Ctx.Entity = Var; +Ctx.Kind = Sema::CodeSynthesisContext::ImplicitLambdaCaptureInitialization; +Ctx.PointOfInstantiation = Capture.getLocation(); +S.pushCodeSynthesisContext(Ctx); + } + auto PopCodeSynthesisStackOnExit = llvm::make_scope_exit([
[PATCH] D52064: [Sema] Add a note pointing to the first use of an implicit capture
vsk added inline comments. Comment at: clang/test/SemaCXX/lambda-expressions.cpp:87 +(void)^{ // expected-error@+1 {{no matching constructor for initialization of 'const G'}} + return [=]{ // expected-error@+1 {{no matching constructor for initialization of 'const G'}} +const G* gg = &g; // expected-note {{implicitly capturing 'g', first used here}} rsmith wrote: > Why are these @+1? A 'no matching constructor' error is present on the line containing "[=]" (pointing to the '=' sign), as well as on the line containing "gg = &g" (pointing to the last 'g'). I'll try to capture that in a neater way. Stepping back a bit, I think clang does this to make it clear that an implicit capture is part of the problem. It does seem strange to me that we'd emit the same error twice, but according to baseline version of this test, that's the expected behavior. Let me know if I should try and change that diagnostic. https://reviews.llvm.org/D52064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r342195 - Update a clang-tidy test for r342194
Author: vedantk Date: Thu Sep 13 16:50:20 2018 New Revision: 342195 URL: http://llvm.org/viewvc/llvm-project?rev=342195&view=rev Log: Update a clang-tidy test for r342194 The location of implicit captures has changed. Update a use-after-move checker test to reflect that. This fixes a bot failure: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/36500 Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp?rev=342195&r1=342194&r2=342195&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp Thu Sep 13 16:50:20 2018 @@ -339,7 +339,7 @@ void lambdas() { A a; std::move(a); auto lambda = [=]() { a.foo(); }; -// CHECK-MESSAGES: [[@LINE-1]]:27: warning: 'a' used after it was moved +// CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'a' used after it was moved // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here } // Same tests but for capture by reference. @@ -354,7 +354,7 @@ void lambdas() { A a; std::move(a); auto lambda = [&]() { a.foo(); }; -// CHECK-MESSAGES: [[@LINE-1]]:27: warning: 'a' used after it was moved +// CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'a' used after it was moved // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here } // But don't warn if the move happened after the capture. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52064: [Sema] Add a note pointing to the first use of an implicit capture
rsmith added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1427 if (RefResult.isInvalid()) return ExprError(); Expr *Ref = RefResult.get(); This early exit leaves your CodeSynthesisContext on the stack. Consider using RAII? Comment at: clang/test/SemaCXX/lambda-expressions.cpp:87 +(void)^{ // expected-error@+1 {{no matching constructor for initialization of 'const G'}} + return [=]{ // expected-error@+1 {{no matching constructor for initialization of 'const G'}} +const G* gg = &g; // expected-note {{implicitly capturing 'g', first used here}} Why are these @+1? https://reviews.llvm.org/D52064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52064: [Sema] Add a note pointing to the first use of an implicit capture
vsk created this revision. vsk added reviewers: rsmith, erichkeane. When it's not possible to initialize an implicit capture, add a note pointing to the first use of the captured variable. Example (the `note` is new): lambda-expressions.cpp:81:15: error: no matching constructor for initialization of 'G' return [=]{ ^ lambda-expressions.cpp:82:24: note: implicitly capturing 'g', first used here const G* gg = &g; ^ As suggested in https://reviews.llvm.org/D50927. https://reviews.llvm.org/D52064 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Sema/SemaLambda.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp clang/test/SemaCXX/lambda-expressions.cpp Index: clang/test/SemaCXX/lambda-expressions.cpp === --- clang/test/SemaCXX/lambda-expressions.cpp +++ clang/test/SemaCXX/lambda-expressions.cpp @@ -77,8 +77,18 @@ struct G { G(); G(G&); int a; }; // expected-note 6 {{not viable}} G g; [=]() { const G* gg = &g; return gg->a; }; -[=]() { return [=]{ const G* gg = &g; return gg->a; }(); }; // expected-error {{no matching constructor for initialization of 'G'}} -(void)^{ return [=]{ const G* gg = &g; return gg->a; }(); }; // expected-error 2 {{no matching constructor for initialization of 'const G'}} +[=]() { + return [=]{ // expected-error {{no matching constructor for initialization of 'G'}} +const G* gg = &g; // expected-note {{implicitly capturing 'g', first used here}} +return gg->a; + }(); +}; +(void)^{ // expected-error@+1 {{no matching constructor for initialization of 'const G'}} + return [=]{ // expected-error@+1 {{no matching constructor for initialization of 'const G'}} +const G* gg = &g; // expected-note {{implicitly capturing 'g', first used here}} +return gg->a; + }(); +}; const int h = a; // expected-note {{declared}} []() { return h; }; // expected-error {{variable 'h' cannot be implicitly captured in a lambda with no capture-default specified}} expected-note {{lambda expression begins here}} @@ -378,7 +388,9 @@ namespace PR18473 { template void f() { T t(0); -(void) [=]{ int n = t; }; // expected-error {{deleted}} +(void)[=] { // expected-error {{call to deleted constructor of 'PR18473::NoCopy'}} + int n = t; // expected-note {{implicitly capturing 't', first used here}} +}; } template void f(); Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp === --- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp +++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp @@ -16,7 +16,7 @@ void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) { (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}} (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} -ncr.foo(); +ncr.foo(); // expected-note{{implicitly capturing 'ncr', first used here}} }(); [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}} Index: clang/lib/Sema/SemaTemplateInstantiate.cpp === --- clang/lib/Sema/SemaTemplateInstantiate.cpp +++ clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -200,6 +200,7 @@ case DeclaringSpecialMember: case DefiningSynthesizedFunction: case ExceptionSpecEvaluation: + case ImplicitLambdaCaptureInitialization: return false; // This function should never be called when Kind's value is Memoization. @@ -653,6 +654,13 @@ break; } +case CodeSynthesisContext::ImplicitLambdaCaptureInitialization: { + Diags.Report(Active->PointOfInstantiation, + diag::note_implicitly_capturing_var_first_used_here) + << cast(Active->Entity); + break; +} + case CodeSynthesisContext::Memoization: break; } @@ -698,6 +706,7 @@ case CodeSynthesisContext::DeclaringSpecialMember: case CodeSynthesisContext::DefiningSynthesizedFunction: +case CodeSynthesisContext::ImplicitLambdaCaptureInitialization: // This happens in a context unrelated to template instantiation, so // there is no SFINAE. return None; Index: clang/lib/Sema/SemaLambda.cpp === --- clang/lib/Sema/SemaLambda.cpp +++ clang/lib/Sema/SemaLambda.cpp @@ -1401,6 +1401,14 @@ SourceLocation Loc = IsImplicitCapture ? ImplicitCaptureLoc : Capture.getLocation(); + if (IsImplicitCapture) { +Sema::CodeSynthesisContext Ctx; +Ctx.Entity = Var; +Ctx.Kind
[PATCH] D51905: Front-end of the implementation of the interleaving algorithm
zhaomo added inline comments. Comment at: compiler-rt/test/cfi/lit.cfg:33 dso = '-fsanitize-cfi-cross-dso -fvisibility=default ' + interleave = '-Xclang -enable-vtable-interleaving -Wl,-plugin-opt,-enable-vtable-interleaving -Wl,-plugin-opt=save-temps -fno-sanitize=cfi-mfcall ' if config.android: vlad.tsyrklevich wrote: > Can get rid of at least save-temps here, do we need to pass > -enable-vtable-interleaving to the linker? I was debugging with save-temps and I forgot to remove it. Currently we need to pass -enable-vtable-interleaving to the linker because the front-end and back-end are controlled by two separate flags. https://reviews.llvm.org/D51905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50927: [Sema] Remove location from implicit capture init expr
This revision was automatically updated to reflect the committed changes. Closed by commit rL342194: [Sema] Remove location from implicit capture init expr (authored by vedantk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50927?vs=162918&id=165401#toc Repository: rL LLVM https://reviews.llvm.org/D50927 Files: cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp cfe/trunk/test/SemaCXX/uninitialized.cpp cfe/trunk/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp Index: cfe/trunk/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp === --- cfe/trunk/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp +++ cfe/trunk/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp @@ -75,11 +75,11 @@ TEST(RecursiveASTVisitor, VisitsImplicitLambdaCaptureInit) { DeclRefExprVisitor Visitor; Visitor.setShouldVisitImplicitCode(true); - // We're expecting the "i" in the lambda to be visited twice: - // - Once for the DeclRefExpr in the lambda capture initialization (whose - // source code location is set to the first use of the variable). - // - Once for the DeclRefExpr for the use of "i" inside the lambda. - Visitor.ExpectMatch("i", 1, 24, /*Times=*/2); + // We're expecting "i" to be visited twice: once for the initialization expr + // for the captured variable "i" outside of the lambda body, and again for + // the use of "i" inside the lambda. + Visitor.ExpectMatch("i", 1, 20, /*Times=*/1); + Visitor.ExpectMatch("i", 1, 24, /*Times=*/1); EXPECT_TRUE(Visitor.runOver( "void f() { int i; [=]{ i; }; }", DeclRefExprVisitor::Lang_CXX11)); Index: cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp === --- cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp +++ cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm \ +// RUN: -debug-info-kind=line-tables-only -dwarf-column-info -std=c++11 %s -o - | FileCheck %s + +// CHECK-LABEL: define{{.*}}lambda_in_func +void lambda_in_func(int &ref) { + // CHECK: [[ref_slot:%.*]] = getelementptr inbounds %class.anon, %class.anon* {{.*}}, i32 0, i32 0, !dbg [[lambda_decl_loc:![0-9]+]] + // CHECK-NEXT: %1 = load i32*, i32** %ref.addr, align 8, !dbg [[capture_init_loc:![0-9]+]] + // CHECK-NEXT: store i32* %1, i32** %0, align 8, !dbg [[lambda_decl_loc]] + // CHECK-NEXT: call void {{.*}}, !dbg [[lambda_call_loc:![0-9]+]] + + auto helper = [ // CHECK: [[lambda_decl_loc]] = !DILocation(line: [[@LINE]], column: 17 + &]() { // CHECK: [[capture_init_loc]] = !DILocation(line: [[@LINE]], column: 18 +++ref; + }; + helper(); // CHECK: [[lambda_call_loc]] = !DILocation(line: [[@LINE]] +} Index: cfe/trunk/test/SemaCXX/uninitialized.cpp === --- cfe/trunk/test/SemaCXX/uninitialized.cpp +++ cfe/trunk/test/SemaCXX/uninitialized.cpp @@ -884,8 +884,10 @@ int x; }; A a0([] { return a0.x; }); // ok - void f() { -A a1([=] { return a1.x; }); // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}} + void f() { +A a1([=] { // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}} + return a1.x; +}); A a2([&] { return a2.x; }); // ok } } Index: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp === --- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp +++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp @@ -15,8 +15,8 @@ void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) { (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}} - (void)[=] { -ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} + (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} +ncr.foo(); }(); [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}} Index: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp === --- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp +++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp @@ -88,8 +88,8 @@ template void odr_used2(R &r, Boom boom) { const std::type_info &ti - = typeid([=,&r] () -> R& { - boom.tickle(); // expected-note{{in ins
[PATCH] D50927: [Sema] Remove location from implicit capture init expr
This revision was automatically updated to reflect the committed changes. Closed by commit rC342194: [Sema] Remove location from implicit capture init expr (authored by vedantk, committed by ). Changed prior to commit: https://reviews.llvm.org/D50927?vs=162918&id=165400#toc Repository: rL LLVM https://reviews.llvm.org/D50927 Files: lib/Sema/SemaLambda.cpp test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp test/CodeGenCXX/debug-info-lambda.cpp test/SemaCXX/uninitialized.cpp unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -1392,13 +1392,14 @@ Class->addDecl(Conversion); } -static ExprResult performLambdaVarCaptureInitialization(Sema &S, -const Capture &Capture, -FieldDecl *Field) { +static ExprResult performLambdaVarCaptureInitialization( +Sema &S, const Capture &Capture, FieldDecl *Field, +SourceLocation ImplicitCaptureLoc, bool IsImplicitCapture) { assert(Capture.isVariableCapture() && "not a variable capture"); auto *Var = Capture.getVariable(); - SourceLocation Loc = Capture.getLocation(); + SourceLocation Loc = + IsImplicitCapture ? ImplicitCaptureLoc : Capture.getLocation(); // C++11 [expr.prim.lambda]p21: // When the lambda-expression is evaluated, the entities that @@ -1607,8 +1608,8 @@ Var, From.getEllipsisLoc())); Expr *Init = From.getInitExpr(); if (!Init) { -auto InitResult = -performLambdaVarCaptureInitialization(*this, From, *CurField); +auto InitResult = performLambdaVarCaptureInitialization( +*this, From, *CurField, CaptureDefaultLoc, IsImplicit); if (InitResult.isInvalid()) return ExprError(); Init = InitResult.get(); Index: unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp === --- unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp +++ unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp @@ -75,11 +75,11 @@ TEST(RecursiveASTVisitor, VisitsImplicitLambdaCaptureInit) { DeclRefExprVisitor Visitor; Visitor.setShouldVisitImplicitCode(true); - // We're expecting the "i" in the lambda to be visited twice: - // - Once for the DeclRefExpr in the lambda capture initialization (whose - // source code location is set to the first use of the variable). - // - Once for the DeclRefExpr for the use of "i" inside the lambda. - Visitor.ExpectMatch("i", 1, 24, /*Times=*/2); + // We're expecting "i" to be visited twice: once for the initialization expr + // for the captured variable "i" outside of the lambda body, and again for + // the use of "i" inside the lambda. + Visitor.ExpectMatch("i", 1, 20, /*Times=*/1); + Visitor.ExpectMatch("i", 1, 24, /*Times=*/1); EXPECT_TRUE(Visitor.runOver( "void f() { int i; [=]{ i; }; }", DeclRefExprVisitor::Lang_CXX11)); Index: test/SemaCXX/uninitialized.cpp === --- test/SemaCXX/uninitialized.cpp +++ test/SemaCXX/uninitialized.cpp @@ -884,8 +884,10 @@ int x; }; A a0([] { return a0.x; }); // ok - void f() { -A a1([=] { return a1.x; }); // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}} + void f() { +A a1([=] { // expected-warning{{variable 'a1' is uninitialized when used within its own initialization}} + return a1.x; +}); A a2([&] { return a2.x; }); // ok } } Index: test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp === --- test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp +++ test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp @@ -15,8 +15,8 @@ void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) { (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}} - (void)[=] { -ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} + (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} +ncr.foo(); }(); [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}} Index: test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp === --- test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp +++ test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp @@ -88,8 +88,8 @@ template void odr_used2(R &r, Boom boom) { const std::type_info &ti -
r342194 - [Sema] Remove location from implicit capture init expr
Author: vedantk Date: Thu Sep 13 16:28:25 2018 New Revision: 342194 URL: http://llvm.org/viewvc/llvm-project?rev=342194&view=rev Log: [Sema] Remove location from implicit capture init expr A lambda's closure is initialized when the lambda is declared. For implicit captures, the initialization code emitted from EmitLambdaExpr references source locations *within the lambda body* in the function containing the lambda. This results in a poor debugging experience: we step to the line containing the lambda, then into lambda, out again, over and over, until every capture's field is initialized. To improve stepping behavior, assign the starting location of the lambda to expressions which initialize an implicit capture within it. rdar://39807527 Differential Revision: https://reviews.llvm.org/D50927 Added: cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp Modified: cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp cfe/trunk/test/SemaCXX/uninitialized.cpp cfe/trunk/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp Modified: cfe/trunk/lib/Sema/SemaLambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=342194&r1=342193&r2=342194&view=diff == --- cfe/trunk/lib/Sema/SemaLambda.cpp (original) +++ cfe/trunk/lib/Sema/SemaLambda.cpp Thu Sep 13 16:28:25 2018 @@ -1392,13 +1392,14 @@ static void addBlockPointerConversion(Se Class->addDecl(Conversion); } -static ExprResult performLambdaVarCaptureInitialization(Sema &S, -const Capture &Capture, -FieldDecl *Field) { +static ExprResult performLambdaVarCaptureInitialization( +Sema &S, const Capture &Capture, FieldDecl *Field, +SourceLocation ImplicitCaptureLoc, bool IsImplicitCapture) { assert(Capture.isVariableCapture() && "not a variable capture"); auto *Var = Capture.getVariable(); - SourceLocation Loc = Capture.getLocation(); + SourceLocation Loc = + IsImplicitCapture ? ImplicitCaptureLoc : Capture.getLocation(); // C++11 [expr.prim.lambda]p21: // When the lambda-expression is evaluated, the entities that @@ -1607,8 +1608,8 @@ ExprResult Sema::BuildLambdaExpr(SourceL Var, From.getEllipsisLoc())); Expr *Init = From.getInitExpr(); if (!Init) { -auto InitResult = -performLambdaVarCaptureInitialization(*this, From, *CurField); +auto InitResult = performLambdaVarCaptureInitialization( +*this, From, *CurField, CaptureDefaultLoc, IsImplicit); if (InitResult.isInvalid()) return ExprError(); Init = InitResult.get(); Modified: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp?rev=342194&r1=342193&r2=342194&view=diff == --- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp (original) +++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp Thu Sep 13 16:28:25 2018 @@ -15,8 +15,8 @@ public: void capture_by_copy(NonCopyable nc, NonCopyable &ncr, const NonConstCopy nco) { (void)[nc] { }; // expected-error{{capture of variable 'nc' as type 'NonCopyable' calls private copy constructor}} - (void)[=] { -ncr.foo(); // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} + (void)[=] { // expected-error{{capture of variable 'ncr' as type 'NonCopyable' calls private copy constructor}} +ncr.foo(); }(); [nco] {}(); // expected-error{{no matching constructor for initialization of 'const NonConstCopy'}} Modified: cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp?rev=342194&r1=342193&r2=342194&view=diff == --- cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp (original) +++ cfe/trunk/test/CXX/expr/expr.prim/expr.prim.lambda/templates.cpp Thu Sep 13 16:28:25 2018 @@ -88,8 +88,8 @@ namespace p2 { template void odr_used2(R &r, Boom boom) { const std::type_info &ti - = typeid([=,&r] () -> R& { - boom.tickle(); // expected-note{{in instantiation of member function}} + = typeid([=,&r] () -> R& { // expected-note{{in instantiation of member function 'p2::Boom::Boom' requested here}} + boom.tickle(); return r; }()); } Added: cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/deb
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
erik.pilkington added a comment. Looks good to me! @rsmith should probably have the final word though. Thanks for fixing this. Repository: rC Clang https://reviews.llvm.org/D51997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51905: Front-end of the implementation of the interleaving algorithm
zhaomo added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:939 +/// We should only interleave vtables when the module has the hidden +/// LTO visibility, cfi-vcall is enabled and EnableVTableInterleaving vlad.tsyrklevich wrote: > doxygen comments normally go in the corresponding header file. I just followed the style of the rest of the file. All the comments above functions use ///. Do I need to change them? https://reviews.llvm.org/D51905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51187: [RFC] Thread safety analysis: Track status of scoped capability
aaronpuchert planned changes to this revision. aaronpuchert added a comment. This doesn't work with loops yet: void relockLoop() { RelockableExclusiveMutexLock scope(&mu); while (b) { scope.Unlock(); // releasing mutex 'scope' that was not held scope.Lock(); // acquiring mutex 'mu' that is already held } } There should be no warnings here — this code is fine. Repository: rC Clang https://reviews.llvm.org/D51187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342192 - Fix crash on call to __builtin_memcpy with a null pointer to an
Author: rsmith Date: Thu Sep 13 15:47:33 2018 New Revision: 342192 URL: http://llvm.org/viewvc/llvm-project?rev=342192&view=rev Log: Fix crash on call to __builtin_memcpy with a null pointer to an incomplete type. Also improve the diagnostics for similar situations. Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td cfe/trunk/lib/AST/APValue.cpp cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp cfe/trunk/test/SemaCXX/constexpr-string.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=342192&r1=342191&r2=342192&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Thu Sep 13 15:47:33 2018 @@ -163,6 +163,10 @@ def note_constexpr_unsupported_unsized_a def note_constexpr_unsized_array_indexed : Note< "indexing of array without known bound is not allowed " "in a constant expression">; +def note_constexpr_memcpy_null : Note< + "%select{source|destination}2 of " + "'%select{%select{memcpy|wmemcpy}1|%select{memmove|wmemmove}1}0' " + "is %3">; def note_constexpr_memcpy_type_pun : Note< "cannot constant evaluate '%select{memcpy|memmove}0' from object of " "type %1 to object of type %2">; Modified: cfe/trunk/lib/AST/APValue.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/APValue.cpp?rev=342192&r1=342191&r2=342192&view=diff == --- cfe/trunk/lib/AST/APValue.cpp (original) +++ cfe/trunk/lib/AST/APValue.cpp Thu Sep 13 15:47:33 2018 @@ -416,18 +416,26 @@ void APValue::printPretty(raw_ostream &O << GetApproxValue(getComplexFloatImag()) << "i"; return; case APValue::LValue: { -LValueBase Base = getLValueBase(); -if (!Base) { - Out << "0"; - return; -} - bool IsReference = Ty->isReferenceType(); QualType InnerTy = IsReference ? Ty.getNonReferenceType() : Ty->getPointeeType(); if (InnerTy.isNull()) InnerTy = Ty; +LValueBase Base = getLValueBase(); +if (!Base) { + if (isNullPointer()) { +Out << (Ctx.getLangOpts().CPlusPlus11 ? "nullptr" : "0"); + } else if (IsReference) { +Out << "*(" << InnerTy.stream(Ctx.getPrintingPolicy()) << "*)" +<< getLValueOffset().getQuantity(); + } else { +Out << "(" << Ty.stream(Ctx.getPrintingPolicy()) << ")" +<< getLValueOffset().getQuantity(); + } + return; +} + if (!hasLValuePath()) { // No lvalue path: just print the offset. CharUnits O = getLValueOffset(); Modified: cfe/trunk/lib/AST/ExprConstant.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=342192&r1=342191&r2=342192&view=diff == --- cfe/trunk/lib/AST/ExprConstant.cpp (original) +++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Sep 13 15:47:33 2018 @@ -6191,12 +6191,12 @@ bool PointerExprEvaluator::VisitBuiltinC BuiltinOp == Builtin::BI__builtin_wmemmove; // The result of mem* is the first argument. -if (!Visit(E->getArg(0)) || Result.Designator.Invalid) +if (!Visit(E->getArg(0))) return false; LValue Dest = Result; LValue Src; -if (!EvaluatePointer(E->getArg(1), Src, Info) || Src.Designator.Invalid) +if (!EvaluatePointer(E->getArg(1), Src, Info)) return false; APSInt N; @@ -6209,6 +6209,20 @@ bool PointerExprEvaluator::VisitBuiltinC if (!N) return true; +// Otherwise, if either of the operands is null, we can't proceed. Don't +// try to determine the type of the copied objects, because there aren't +// any. +if (!Src.Base || !Dest.Base) { + APValue Val; + (!Src.Base ? Src : Dest).moveInto(Val); + Info.FFDiag(E, diag::note_constexpr_memcpy_null) + << Move << WChar << !!Src.Base + << Val.getAsString(Info.Ctx, E->getArg(0)->getType()); + return false; +} +if (Src.Designator.Invalid || Dest.Designator.Invalid) + return false; + // We require that Src and Dest are both pointers to arrays of // trivially-copyable type. (For the wide version, the designator will be // invalid if the designated object is not a wchar_t.) Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=342192&r1=342191&r2=342192&view=diff == --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original) +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Thu Sep 13 15:47:33 2018 @@ -253,7 +253,7 @@
[PATCH] D51855: [constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array
rsmith added a comment. Thank you! Comment at: include/clang/Basic/DiagnosticASTKinds.td:172 "non-trivially-copyable type %1">; +def note_constexpr_memcpy_incompletetype : Note< + "cannot constant evaluate '%select{memcpy|memmove}0' between objects of " Nit: add an underscore between `incomplete` and `type`. Comment at: lib/AST/ExprConstant.cpp:6225-6228 +if (T->isIncompleteType()) { + Info.FFDiag(E, diag::note_constexpr_memcpy_incompletetype) << Move << T; + return false; +} Please reorder this before the trivial-copyability check. We don't know whether a type is trivially-copyable if it's not complete, so it makes more sense to check these things in the opposite order. Testcase: `struct A; extern A x, y; constexpr void f() { __builtin_memcpy(&x, &y, 4); }` ... currently produces a bogus diagnostic about `A` not being trivially-copyable but instead should produce your new diagnostic that `A` is incomplete. Repository: rC Clang https://reviews.llvm.org/D51855 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
ldionne added a comment. I created https://bugs.llvm.org/show_bug.cgi?id=38942 to keep track of the problem for partial specializations and explicit specializations. Moving forward with this patch will allow me to land the `no_extern_template` attribute. Repository: rC Clang https://reviews.llvm.org/D51997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status
kristina added a comment. Fine with me, I wasn't really suggesting a revision just making a remark more or less. Sorry if it came off that way. Repository: rC Clang https://reviews.llvm.org/D51921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342177 - Diagnose likely typos in #include directives.
Author: rsmith Date: Thu Sep 13 14:10:08 2018 New Revision: 342177 URL: http://llvm.org/viewvc/llvm-project?rev=342177&view=rev Log: Diagnose likely typos in #include directives. Summary: When someone writes #include "" or #include " some_file " the compiler returns "file not fuond..." with fonts and quotes that may make it hard to see there are excess quotes or surprising bytes in the filename. Assuming that files are usually logically named and start and end with an alphanumeric character, we can check for the file's existence by stripping the non-alphanumeric leading or trailing characters. If the file is found, emit a non-fatal error with a FixItHint. Patch by Christy Lee! Reviewers: aaron.ballman, erikjv, rsmith Reviewed By: rsmith Subscribers: lebedev.ri, xbolva00, sammccall, modocache, erikjv, aaron.ballman, cfe-commits Differential Revision: https://reviews.llvm.org/D51333 Added: cfe/trunk/test/Preprocessor/empty_file_to_include.h cfe/trunk/test/Preprocessor/include-likely-typo.c Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td cfe/trunk/lib/Lex/PPDirectives.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=342177&r1=342176&r2=342177&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Thu Sep 13 14:10:08 2018 @@ -422,8 +422,10 @@ def warn_pp_hdrstop_filename_ignored : W "#pragma hdrstop filename not supported, " "/Fp can be used to specify precompiled header filename">, InGroup; -def err_pp_file_not_found_not_fatal : Error< +def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with include; use \"quotes\" instead">; +def err_pp_file_not_found_typo_not_fatal +: Error<"'%0' file not found, did you mean '%1'?">; def err_pp_error_opening_file : Error< "error opening file '%0': %1">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; Modified: cfe/trunk/lib/Lex/PPDirectives.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=342177&r1=342176&r2=342177&view=diff == --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) +++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Sep 13 14:10:08 2018 @@ -1879,12 +1879,40 @@ void Preprocessor::HandleIncludeDirectiv Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +StringRef OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Added: cfe/trunk/test/Preprocessor/empty_file_to_include.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/empty_file_to_include.h?rev=342177&view=auto == --- cfe/trunk/test/Preprocessor/empty_file_to_include.h (added) +++ cfe/trunk/test/Preprocessor/empty_file_to_include.h Thu Sep 13 14:10:08 2018 @@ -0,0 +1,7 @@ + +#ifndef EMPTY_FILE_TO_INCLUDE_H +#define EMPTY_FILE_TO_INCLUDE_H + +// empty file + +#endif Added: cfe/trunk/test/Preprocessor/include-likely-typo.c URL: http://llvm.org/vie
[PATCH] D51333: Diagnose likely typos in include statements
This revision was automatically updated to reflect the committed changes. Closed by commit rL342177: Diagnose likely typos in #include directives. (authored by rsmith, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51333?vs=165361&id=165376#toc Repository: rL LLVM https://reviews.llvm.org/D51333 Files: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td cfe/trunk/lib/Lex/PPDirectives.cpp cfe/trunk/test/Preprocessor/empty_file_to_include.h cfe/trunk/test/Preprocessor/include-likely-typo.c Index: cfe/trunk/test/Preprocessor/include-likely-typo.c === --- cfe/trunk/test/Preprocessor/include-likely-typo.c +++ cfe/trunk/test/Preprocessor/include-likely-typo.c @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 %s -verify + +#include "" // expected-error {{'' file not found, did you mean 'empty_file_to_include.h'?}} Index: cfe/trunk/test/Preprocessor/empty_file_to_include.h === --- cfe/trunk/test/Preprocessor/empty_file_to_include.h +++ cfe/trunk/test/Preprocessor/empty_file_to_include.h @@ -0,0 +1,7 @@ + +#ifndef EMPTY_FILE_TO_INCLUDE_H +#define EMPTY_FILE_TO_INCLUDE_H + +// empty file + +#endif Index: cfe/trunk/lib/Lex/PPDirectives.cpp === --- cfe/trunk/lib/Lex/PPDirectives.cpp +++ cfe/trunk/lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +StringRef OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td @@ -422,8 +422,10 @@ "#pragma hdrstop filename not supported, " "/Fp can be used to specify precompiled header filename">, InGroup; -def err_pp_file_not_found_not_fatal : Error< +def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with include; use \"quotes\" instead">; +def err_pp_file_not_found_typo_not_fatal +: Error<"'%0' file not found, did you mean '%1'?">; def err_pp_error_opening_file : Error< "error opening file '%0': %1">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; Index: cfe/trunk/test/Preprocessor/include-likely-typo.c === --- cfe/trunk/test/Preprocessor/include-likely-typo.c +++ cfe/trunk/test/Preprocessor/include-likely-typo.c @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 %s -verify + +#include "" // expected-error {{'' file not found, did you mean 'empty_file_to_include.h'?}} Index: cfe/trunk/test/Preprocessor/empty_file_to_include.h === --- cfe/trunk/test/Preprocessor/empty_file_to_include.h +++ cfe/trunk/test/Preprocessor/empty_file_to_include.h @@ -0,0 +1,7 @@ + +#ifndef EMPTY_FILE_TO_INCLUDE_H +#define EMPTY_FILE_TO_INCLUDE_H + +// empty file + +#endif Index: cfe/trunk/lib/Lex/PPDirectives.cpp === --- cfe/trunk/lib/Lex/PPDi
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
ldionne updated this revision to Diff 165375. ldionne added a comment. Drop support for partial specializations and explicit specializations. Address comments by Erik. Repository: rC Clang https://reviews.llvm.org/D51997 Files: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/test/SemaCXX/PR38913.cpp Index: clang/test/SemaCXX/PR38913.cpp === --- /dev/null +++ clang/test/SemaCXX/PR38913.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s + +// PR38913 +// Check that we instantiate attributes on declarations for... + +// ...a member class of a class template specialization +template struct A { struct __attribute__((abi_tag("ATAG"))) X { }; }; +A::X* a() { return 0; } // CHECK-DAG: @_Z1aB4ATAGv + +// ...a member class template +template struct B { template struct __attribute__((abi_tag("BTAG"))) X { }; }; +B::X* b() { return 0; } // CHECK-DAG: @_Z1bB4BTAGv Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp === --- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1258,6 +1258,9 @@ if (QualifierLoc) RecordInst->setQualifierInfo(QualifierLoc); + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs, + StartingScope); + ClassTemplateDecl *Inst = ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(), D->getIdentifier(), InstParams, RecordInst); @@ -1491,6 +1494,9 @@ if (SubstQualifier(D, Record)) return nullptr; + SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs, + StartingScope); + Record->setImplicit(D->isImplicit()); // FIXME: Check against AS_none is an ugly hack to work around the issue that // the tag decls introduced by friend class declarations don't have an access Index: clang/test/SemaCXX/PR38913.cpp === --- /dev/null +++ clang/test/SemaCXX/PR38913.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s + +// PR38913 +// Check that we instantiate attributes on declarations for... + +// ...a member class of a class template specialization +template struct A { struct __attribute__((abi_tag("ATAG"))) X { }; }; +A::X* a() { return 0; } // CHECK-DAG: @_Z1aB4ATAGv + +// ...a member class template +template struct B { template struct __attribute__((abi_tag("BTAG"))) X { }; }; +B::X* b() { return 0; } // CHECK-DAG: @_Z1bB4BTAGv Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp === --- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1258,6 +1258,9 @@ if (QualifierLoc) RecordInst->setQualifierInfo(QualifierLoc); + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs, + StartingScope); + ClassTemplateDecl *Inst = ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(), D->getIdentifier(), InstParams, RecordInst); @@ -1491,6 +1494,9 @@ if (SubstQualifier(D, Record)) return nullptr; + SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs, + StartingScope); + Record->setImplicit(D->isImplicit()); // FIXME: Check against AS_none is an ugly hack to work around the issue that // the tag decls introduced by friend class declarations don't have an access ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
ldionne added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1268 + // Instantiate the attributes on both the template declaration and the associated record declaration. + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs, StartingScope); + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs, StartingScope); rsmith wrote: > ldionne wrote: > > erik.pilkington wrote: > > > Why are you instantiating the attributes onto the ClassTemplateDecl? At > > > the very least it seems wrong to instantiate attributes from the pattern > > > to the ClassTemplateDecl. > > My understanding was that > > - `Inst` represented `B::X` (the template) > > - `RecordInst` represented `B::X` (the template specialization, > > which is a "real struct") > > - `Pattern` represented `B::X` > > > > If that is right, then it seemed to make sense that both `B::X` and > > `B::X` should have the attribute, since `B::X` has it. I > > might be misunderstanding something though. > That's almost right. > > * `Inst` represents `B::X` (the template declaration in the > instantiation of `B`). > * `RecordInst` represents `B::X` (the class declaration within the > template declaration in the instantiation of `B`). > * `Pattern` represents `B::X` (the class declaration within the > template declaration in the definition of `B`). > > And we don't yet have a value for `U`, so there is no `B::X` here. > > That is: > > ``` > template > struct B { > template // #1 > struct X; // Pattern > }; > // implicit instantiation of B: > template<> struct B { > template // Inst > struct X; // RecordInst > }; > ``` > > A template-declaration can never have attributes on it (there is no syntactic > location where they can be written), so we should never be instantiating > attributes onto it. If it could, we should instantiate those attributes from > `#1`, not from `Pattern`. Thanks a lot for the explanation -- this is really helpful. It's now clear that this line should not be there. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2971 InstD->setExternLoc(D->getExternLoc()); InstD->setTemplateKeywordLoc(D->getTemplateKeywordLoc()); ldionne wrote: > I tried a couple variations of things like: > > ``` > SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, InstD, LateAttrs, > StartingScope); > ``` > > and > > ``` > SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, > ClassTemplate->getTemplatedDecl(), LateAttrs, StartingScope); > ``` > > For some reason, none of them end up instantiating the attribute on the > `CXXRecordDecl` that is used to determine the mangling. > So, assuming this: ``` template struct Foo { template struct X { }; template<> struct ATTRS X { }; }; ``` Based on what @rsmith says about partial specializations, my understanding is that: - `D` is the explicit specialization inside the general class template: `Foo::X` - `InstD` is the explicit specialization inside the specialization of `Foo`: `Foo::X` And hence, what we want to do here is apply the attributes from `Foo::X` onto `Foo::X`: ``` SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, InstD, LateAttrs, StartingScope); ``` However, like I said previously this is not sufficient and I'll just not implement this for specializations for the time being (for the sake of making progress on `no_extern_template`). Repository: rC Clang https://reviews.llvm.org/D51997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status
sammccall added a comment. (Just checking - IIUC neither of you are asking for changes, and this is waiting on review of the latest snapshot?) Repository: rC Clang https://reviews.llvm.org/D51921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov
calixte added inline comments. Comment at: include/clang/Driver/CC1Options.td:236 +def coverage_exclude_EQ : Joined<["-"], "coverage-exclude=">, + Alias; def coverage_exit_block_before_body : Flag<["-"], "coverage-exit-block-before-body">, vsk wrote: > Have you checked whether gcc supports similar options? If so, it would be > great if we could match their name & behavior. The only one I found -finstrument-functions-exclude-file-list (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html). But no regex and no way to include one file only. I took the names from gcovr: https://manpages.debian.org/jessie/gcovr/gcovr.1.en.html Repository: rC Clang https://reviews.llvm.org/D52034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables
ormris created this revision. ormris added reviewers: dblaikie, aprantl, probinson, JDevlieghere, clayborg, jingham. This patch adds the associated template parameters to the DWARF name attribute of all template variable specializations, mirroring how they are referenced in the source code. Original review: https://reviews.llvm.org/D44842 This version updates the LLVM-C interface. Repository: rC Clang https://reviews.llvm.org/D52058 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.h test/CodeGenCXX/debug-info-template-member.cpp Index: test/CodeGenCXX/debug-info-template-member.cpp === --- test/CodeGenCXX/debug-info-template-member.cpp +++ test/CodeGenCXX/debug-info-template-member.cpp @@ -22,6 +22,19 @@ // CHECK: [[X]] = !DIGlobalVariableExpression(var: [[XV:.*]], expr: !DIExpression()) // CHECK: [[XV]] = distinct !DIGlobalVariable(name: "x", // CHECK-SAME:type: ![[OUTER_FOO_INNER_ID:[0-9]+]] +// +// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable( +// CHECK-SAME: name: "var" +// CHECK-SAME: templateParams: {{![0-9]+}} +// CHECK: !DITemplateTypeParameter(name: "T", type: [[TY:![0-9]+]]) +// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable( +// CHECK-SAME: name: "var" +// CHECK-SAME: templateParams: {{![0-9]+}} +// CHECK: !DITemplateTypeParameter(name: "P", type: {{![0-9]+}}) +// CHECK: {{![0-9]+}} = distinct !DIGlobalVariable( +// CHECK-SAME: name: "varray" +// CHECK-SAME: templateParams: {{![0-9]+}} +// CHECK: !DITemplateValueParameter(name: "N", type: [[TY]], value: i32 1) // CHECK: ![[OUTER_FOO_INNER_ID:[0-9]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "inner"{{.*}}, identifier: // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo" @@ -103,3 +116,15 @@ // declaration/reference in the compile unit. // CHECK: !DISubprogram(name: "MyClass" // CHECK-SAME: scope: [[C]] + +template +T var = T(); +template +P var = P(); +template +T varray[N]; +void f3() { + var = 1; + var = 1; + varray[0] = 1; +} Index: lib/CodeGen/CGDebugInfo.h === --- lib/CodeGen/CGDebugInfo.h +++ lib/CodeGen/CGDebugInfo.h @@ -248,6 +248,11 @@ llvm::DINodeArray CollectFunctionTemplateParams(const FunctionDecl *FD, llvm::DIFile *Unit); + /// A helper function to collect debug info for function template + /// parameters. + llvm::DINodeArray CollectVarTemplateParams(const VarDecl *VD, + llvm::DIFile *Unit); + /// A helper function to collect debug info for template /// parameters. llvm::DINodeArray @@ -645,7 +650,9 @@ /// Collect various properties of a VarDecl. void collectVarDeclProps(const VarDecl *VD, llvm::DIFile *&Unit, unsigned &LineNo, QualType &T, StringRef &Name, - StringRef &LinkageName, llvm::DIScope *&VDContext); + StringRef &LinkageName, + llvm::MDTuple *&templateParameters, + llvm::DIScope *&VDContext); /// Allocate a copy of \p A using the DebugInfoNames allocator /// and return a reference to it. If multiple arguments are given the strings Index: lib/CodeGen/CGDebugInfo.cpp === --- lib/CodeGen/CGDebugInfo.cpp +++ lib/CodeGen/CGDebugInfo.cpp @@ -41,6 +41,7 @@ #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Intrinsics.h" +#include "llvm/IR/Metadata.h" #include "llvm/IR/Module.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MD5.h" @@ -1776,6 +1777,30 @@ return llvm::DINodeArray(); } +llvm::DINodeArray CGDebugInfo::CollectVarTemplateParams(const VarDecl *VL, +llvm::DIFile *Unit) { + if (auto *TS = dyn_cast(VL)) { +if (TS->getSpecializedTemplateOrPartial() +.is()) { + const TemplateParameterList *TList = + TS->getSpecializedTemplateOrPartial() + .get() + ->getTemplateParameters(); + return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(), + Unit); +} + +if (TS->getSpecializedTemplateOrPartial().is()) { + const TemplateParameterList *TList = TS->getSpecializedTemplateOrPartial() + .get() + ->getTemplateParameters(); + return CollectTemplateParams(TList, TS->getTemplateArgs().asArray(), + Unit); +} + } + return llvm::DINodeArray(); +} + llvm::DINodeArray CGDebugInfo::CollectCXXTemplateParams( const ClassTemplateSpecializationDecl *TSpecial, llvm::DIFile *Unit) { // Always get the full list of par
[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations
lebedev.ri added a comment. In https://reviews.llvm.org/D51949#1233951, @JonasToth wrote: > Yes, do you think it should be included in the diag? Yes, please :) Else, the message seems a bit too empty. I **don't** think it should point (via `NOTE:`) at the each decl though. > Am 13.09.2018 um 22:09 schrieb Roman Lebedev via Phabricator: > >> lebedev.ri added inline comments. >> >> >> Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:200 >> + >> + diag(WholeDecl->getBeginLoc(), "make only one declaration per statement") >> + << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), >> Replacement); >> >> >> >> I think you can get the count of declarations via >> `std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end())`. >> >> Repository: >> >> rCTE Clang Tools Extra >> >> https://reviews.llvm.org/D51949 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations
JonasToth added a comment. Yes, do you think it should be included in the diag? Am 13.09.2018 um 22:09 schrieb Roman Lebedev via Phabricator: > lebedev.ri added inline comments. > > > Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:200 > + > + diag(WholeDecl->getBeginLoc(), "make only one declaration per statement") > + << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), > Replacement); > > > > I think you can get the count of declarations via > `std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end())`. > > Repository: > > rCTE Clang Tools Extra > > https://reviews.llvm.org/D51949 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations
lebedev.ri added inline comments. Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:200 + + diag(WholeDecl->getBeginLoc(), "make only one declaration per statement") + << FixItHint::CreateReplacement(WholeDecl->getSourceRange(), Replacement); I think you can get the count of declarations via `std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end())`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
christylee updated this revision to Diff 165361. christylee added a comment. Added tests. https://reviews.llvm.org/D51333 Files: include/clang/Basic/DiagnosticLexKinds.td lib/Lex/PPDirectives.cpp test/Preprocessor/empty_file_to_include.h test/Preprocessor/include-likely-typo.c Index: test/Preprocessor/include-likely-typo.c === --- /dev/null +++ test/Preprocessor/include-likely-typo.c @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 %s -verify + +#include "" // expected-error {{'' file not found, did you mean 'empty_file_to_include.h'?}} Index: test/Preprocessor/empty_file_to_include.h === --- /dev/null +++ test/Preprocessor/empty_file_to_include.h @@ -0,0 +1,7 @@ + +#ifndef EMPTY_FILE_TO_INCLUDE_H +#define EMPTY_FILE_TO_INCLUDE_H + +// empty file + +#endif Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +StringRef OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: include/clang/Basic/DiagnosticLexKinds.td === --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -422,8 +422,10 @@ "#pragma hdrstop filename not supported, " "/Fp can be used to specify precompiled header filename">, InGroup; -def err_pp_file_not_found_not_fatal : Error< +def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with include; use \"quotes\" instead">; +def err_pp_file_not_found_typo_not_fatal +: Error<"'%0' file not found, did you mean '%1'?">; def err_pp_error_opening_file : Error< "error opening file '%0': %1">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; Index: test/Preprocessor/include-likely-typo.c === --- /dev/null +++ test/Preprocessor/include-likely-typo.c @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 %s -verify + +#include "" // expected-error {{'' file not found, did you mean 'empty_file_to_include.h'?}} Index: test/Preprocessor/empty_file_to_include.h === --- /dev/null +++ test/Preprocessor/empty_file_to_include.h @@ -0,0 +1,7 @@ + +#ifndef EMPTY_FILE_TO_INCLUDE_H +#define EMPTY_FILE_TO_INCLUDE_H + +// empty file + +#endif Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + //
[PATCH] D51713: Support -fno-omit-frame-pointer with -pg.
This revision was automatically updated to reflect the committed changes. Closed by commit rL342165: Support -fno-omit-frame-pointer with -pg. (authored by srhines, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51713 Files: cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/test/Driver/clang_f_opts.c Index: cfe/trunk/test/Driver/clang_f_opts.c === --- cfe/trunk/test/Driver/clang_f_opts.c +++ cfe/trunk/test/Driver/clang_f_opts.c @@ -531,3 +531,8 @@ // RUN: %clang -### -S -fno-delete-null-pointer-checks -fdelete-null-pointer-checks %s 2>&1 | FileCheck -check-prefix=CHECK-NULL-POINTER-CHECKS %s // CHECK-NO-NULL-POINTER-CHECKS: "-fno-delete-null-pointer-checks" // CHECK-NULL-POINTER-CHECKS-NOT: "-fno-delete-null-pointer-checks" + +// RUN: %clang -### -S -fomit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-OMIT-FP-PG %s +// RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s +// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg' +// CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg' Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp === --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp @@ -4910,7 +4910,8 @@ } if (Arg *A = Args.getLastArg(options::OPT_pg)) -if (Args.hasArg(options::OPT_fomit_frame_pointer)) +if (Args.hasFlag(options::OPT_fomit_frame_pointer, + options::OPT_fno_omit_frame_pointer, /*default=*/false)) D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer" << A->getAsString(Args); Index: cfe/trunk/test/Driver/clang_f_opts.c === --- cfe/trunk/test/Driver/clang_f_opts.c +++ cfe/trunk/test/Driver/clang_f_opts.c @@ -531,3 +531,8 @@ // RUN: %clang -### -S -fno-delete-null-pointer-checks -fdelete-null-pointer-checks %s 2>&1 | FileCheck -check-prefix=CHECK-NULL-POINTER-CHECKS %s // CHECK-NO-NULL-POINTER-CHECKS: "-fno-delete-null-pointer-checks" // CHECK-NULL-POINTER-CHECKS-NOT: "-fno-delete-null-pointer-checks" + +// RUN: %clang -### -S -fomit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-OMIT-FP-PG %s +// RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s +// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg' +// CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg' Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp === --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp @@ -4910,7 +4910,8 @@ } if (Arg *A = Args.getLastArg(options::OPT_pg)) -if (Args.hasArg(options::OPT_fomit_frame_pointer)) +if (Args.hasFlag(options::OPT_fomit_frame_pointer, + options::OPT_fno_omit_frame_pointer, /*default=*/false)) D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer" << A->getAsString(Args); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342165 - Support -fno-omit-frame-pointer with -pg.
Author: srhines Date: Thu Sep 13 12:50:02 2018 New Revision: 342165 URL: http://llvm.org/viewvc/llvm-project?rev=342165&view=rev Log: Support -fno-omit-frame-pointer with -pg. Summary: Previously, any instance of -fomit-frame-pointer would make it such that -pg was an invalid flag combination. If -fno-omit-frame-pointer is passed later on the command line (such that it actually takes effect), -pg should be allowed. Reviewers: nickdesaulniers Reviewed By: nickdesaulniers Subscribers: manojgupta, nickdesaulniers, cfe-commits, kongyi, chh, pirama Differential Revision: https://reviews.llvm.org/D51713 Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/test/Driver/clang_f_opts.c Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342165&r1=342164&r2=342165&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Sep 13 12:50:02 2018 @@ -4910,7 +4910,8 @@ void Clang::ConstructJob(Compilation &C, } if (Arg *A = Args.getLastArg(options::OPT_pg)) -if (Args.hasArg(options::OPT_fomit_frame_pointer)) +if (Args.hasFlag(options::OPT_fomit_frame_pointer, + options::OPT_fno_omit_frame_pointer, /*default=*/false)) D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer" << A->getAsString(Args); Modified: cfe/trunk/test/Driver/clang_f_opts.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang_f_opts.c?rev=342165&r1=342164&r2=342165&view=diff == --- cfe/trunk/test/Driver/clang_f_opts.c (original) +++ cfe/trunk/test/Driver/clang_f_opts.c Thu Sep 13 12:50:02 2018 @@ -531,3 +531,8 @@ // RUN: %clang -### -S -fno-delete-null-pointer-checks -fdelete-null-pointer-checks %s 2>&1 | FileCheck -check-prefix=CHECK-NULL-POINTER-CHECKS %s // CHECK-NO-NULL-POINTER-CHECKS: "-fno-delete-null-pointer-checks" // CHECK-NULL-POINTER-CHECKS-NOT: "-fno-delete-null-pointer-checks" + +// RUN: %clang -### -S -fomit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-OMIT-FP-PG %s +// RUN: %clang -### -S -fomit-frame-pointer -fno-omit-frame-pointer -pg %s 2>&1 | FileCheck -check-prefix=CHECK-MIX-NO-OMIT-FP-PG %s +// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg' +// CHECK-MIX-NO-OMIT-FP-PG-NOT: '-fomit-frame-pointer' not allowed with '-pg' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51747: [clangd] Show deprecation diagnostics
sammccall accepted this revision. sammccall added a comment. Awesome, thank you. Just a couple of last nits. Comment at: unittests/clangd/ClangdTests.cpp:984 +std::vector Diagnostics) override { + std::lock_guard Lock(Mutex); + for(const Diag& D : Diagnostics) { Locking is unneccesary. (Diagnostics will be delivered once, and the SyncAPI calls block until diagnostics are delivered) Comment at: unittests/clangd/ClangdTests.cpp:1014 + runAddDocument(Server, SourcePath, Test.code()); + EXPECT_TRUE(DiagConsumer.workedAsExpected()); + DiagConsumer.reset(); nit: the logic is right, but the message could be better. If we change something and it fails it will print `Expected true: DiagConsumer.worksAsExpected(), but was false` or so. There are a number of things that could be wrong. Prefer just to capture a bit more data (all diagnostics) and use a matcher expression: ``` MATCHER(DeprecationWarning, "") { return arg.Category == "Deprecations" && arg.Severity == DiagnosticsEngine::Warning; } ... EXPECT_THAT(Diags, ElementsAre(DeprecationWarning())); ``` That way if there isn't exactly one diagnostic that's a deprecation warning, it'll print the expectation, the full list of diagnostics, and the reason it didn't match. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov
vsk added a comment. Please document the filter behavior (see docs/UsersManual.rst). Comment at: include/clang/Driver/CC1Options.td:236 +def coverage_exclude_EQ : Joined<["-"], "coverage-exclude=">, + Alias; def coverage_exit_block_before_body : Flag<["-"], "coverage-exit-block-before-body">, Have you checked whether gcc supports similar options? If so, it would be great if we could match their name & behavior. Repository: rC Clang https://reviews.llvm.org/D52034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
lebedev.ri added a comment. The tests seem to have disappeared form the diff. https://reviews.llvm.org/D51333 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
christylee added a comment. @rsmith , thanks for the review, I fixed the variable capitalization. If you could land it for me that'll be awesome! In https://reviews.llvm.org/D51333#1233042, @sammccall wrote: > One thing I'd like to do sometime is add code completion of filenames in > #include directives. > This would mean listing the relevant directories from the include path. (VFS > is slow for this today, but I have a patch out for it). > > Maybe it would make sense to do that here and use edit-distance to pick the > suggestion, like typo correction elsewhere? > Could be a way to extend this patch's behavior in the future. I also think it's a good idea. I'm happy to take a crack at it! https://reviews.llvm.org/D51333 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
christylee updated this revision to Diff 165350. https://reviews.llvm.org/D51333 Files: include/clang/Basic/DiagnosticLexKinds.td lib/Lex/PPDirectives.cpp Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +StringRef OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: include/clang/Basic/DiagnosticLexKinds.td === --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -422,8 +422,10 @@ "#pragma hdrstop filename not supported, " "/Fp can be used to specify precompiled header filename">, InGroup; -def err_pp_file_not_found_not_fatal : Error< +def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with include; use \"quotes\" instead">; +def err_pp_file_not_found_typo_not_fatal +: Error<"'%0' file not found, did you mean '%1'?">; def err_pp_error_opening_file : Error< "error opening file '%0': %1">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +StringRef OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: include/clang/Basic/DiagnosticLexKinds.td ==
[PATCH] D51333: Diagnose likely typos in include statements
christylee updated this revision to Diff 165349. https://reviews.llvm.org/D51333 Files: include/clang/Basic/DiagnosticLexKinds.td lib/Lex/PPDirectives.cpp Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +auto OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: include/clang/Basic/DiagnosticLexKinds.td === --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -422,8 +422,10 @@ "#pragma hdrstop filename not supported, " "/Fp can be used to specify precompiled header filename">, InGroup; -def err_pp_file_not_found_not_fatal : Error< +def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with include; use \"quotes\" instead">; +def err_pp_file_not_found_typo_not_fatal +: Error<"'%0' file not found, did you mean '%1'?">; def err_pp_error_opening_file : Error< "error opening file '%0': %1">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +auto OriginalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto Hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << OriginalFilename << Filename << Hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: include/clang/Basic/DiagnosticLexKinds.td
[PATCH] D51333: Diagnose likely typos in include statements
xbolva00 added a comment. Where here, I would love to have a fixint hints for unknown functions. E.g. std::forward somewhere in code-> fixit: "try to add #include " https://reviews.llvm.org/D51333 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52050: [Driver] Fix search paths on x32
glaubitz created this revision. glaubitz added reviewers: rsmith, dschuff. Herald added a subscriber: cfe-commits. When targeting x32, the x32 libraries and headers should be used, not the x86_64 ones (which may not even be available), so prioritise those and use the right multiarch triple. Repository: rC Clang https://reviews.llvm.org/D52050 Files: lib/Driver/ToolChains/Gnu.cpp lib/Driver/ToolChains/Linux.cpp Index: lib/Driver/ToolChains/Linux.cpp === --- lib/Driver/ToolChains/Linux.cpp +++ lib/Driver/ToolChains/Linux.cpp @@ -86,10 +86,13 @@ case llvm::Triple::x86_64: if (IsAndroid) return "x86_64-linux-android"; -// We don't want this for x32, otherwise it will match x86_64 libs -if (TargetEnvironment != llvm::Triple::GNUX32 && -D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu")) - return "x86_64-linux-gnu"; +if (TargetEnvironment == llvm::Triple::GNUX32) { + if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnux32")) +return "x86_64-linux-gnux32"; +} else { + if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu")) +return "x86_64-linux-gnu"; +} break; case llvm::Triple::aarch64: if (IsAndroid) @@ -672,6 +675,8 @@ // in use in any released version of Debian, so we should consider // removing them. "/usr/include/i686-linux-gnu/64", "/usr/include/i486-linux-gnu/64"}; + const StringRef X32MultiarchIncludeDirs[] = { + "/usr/include/x86_64-linux-gnux32"}; const StringRef X86MultiarchIncludeDirs[] = { "/usr/include/i386-linux-gnu", @@ -713,7 +718,10 @@ ArrayRef MultiarchIncludeDirs; switch (getTriple().getArch()) { case llvm::Triple::x86_64: -MultiarchIncludeDirs = X86_64MultiarchIncludeDirs; +if (getTriple().getEnvironment() == llvm::Triple::GNUX32) + MultiarchIncludeDirs = X32MultiarchIncludeDirs; +else + MultiarchIncludeDirs = X86_64MultiarchIncludeDirs; break; case llvm::Triple::x86: MultiarchIncludeDirs = X86MultiarchIncludeDirs; Index: lib/Driver/ToolChains/Gnu.cpp === --- lib/Driver/ToolChains/Gnu.cpp +++ lib/Driver/ToolChains/Gnu.cpp @@ -1829,7 +1829,10 @@ "x86_64-manbo-linux-gnu", "x86_64-linux-gnu", "x86_64-slackware-linux", "x86_64-unknown-linux", "x86_64-amazon-linux"}; - static const char *const X32LibDirs[] = {"/libx32"}; + static const char *const X32LibDirs[] = {"/libx32", "/lib"}; + static const char *const X32Triples[] = { + "x86_64-linux-gnux32","x86_64-unknown-linux-gnux32", + "x86_64-pc-linux-gnux32"}; static const char *const X86LibDirs[] = {"/lib32", "/lib"}; static const char *const X86Triples[] = { "i686-linux-gnu", "i686-pc-linux-gnu", "i486-linux-gnu", @@ -2023,14 +2026,16 @@ } break; case llvm::Triple::x86_64: -LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs)); -TripleAliases.append(begin(X86_64Triples), end(X86_64Triples)); // x32 is always available when x86_64 is available, so adding it as // secondary arch with x86_64 triples if (TargetTriple.getEnvironment() == llvm::Triple::GNUX32) { - BiarchLibDirs.append(begin(X32LibDirs), end(X32LibDirs)); + LibDirs.append(begin(X32LibDirs), end(X32LibDirs)); + TripleAliases.append(begin(X32Triples), end(X32Triples)); + BiarchLibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs)); BiarchTripleAliases.append(begin(X86_64Triples), end(X86_64Triples)); } else { + LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs)); + TripleAliases.append(begin(X86_64Triples), end(X86_64Triples)); BiarchLibDirs.append(begin(X86LibDirs), end(X86LibDirs)); BiarchTripleAliases.append(begin(X86Triples), end(X86Triples)); } Index: lib/Driver/ToolChains/Linux.cpp === --- lib/Driver/ToolChains/Linux.cpp +++ lib/Driver/ToolChains/Linux.cpp @@ -86,10 +86,13 @@ case llvm::Triple::x86_64: if (IsAndroid) return "x86_64-linux-android"; -// We don't want this for x32, otherwise it will match x86_64 libs -if (TargetEnvironment != llvm::Triple::GNUX32 && -D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu")) - return "x86_64-linux-gnu"; +if (TargetEnvironment == llvm::Triple::GNUX32) { + if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnux32")) +return "x86_64-linux-gnux32"; +} else { + if (D.getVFS().exists(SysRoot + "/lib/x86_64-linux-gnu")) +return "x86_64-linux-gnu"; +} break; case llvm::Triple::aarch64: if (IsAndroid) @@ -672,6 +675,8 @@ // in use in any released version of Debian, so we should consider // removing them. "/usr/include/i686-linux-gnu/64", "/usr/include/i486-linux-gnu/64"}; + const StringRef X32Multiarch
[PATCH] D43630: [Driver] Fix search paths on x32
glaubitz added a comment. I have pushed a rebase revision here: https://reviews.llvm.org/D52050 I have no idea how to edit the author though. Repository: rC Clang https://reviews.llvm.org/D43630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43630: [Driver] Fix search paths on x32
glaubitz added a comment. I don't think that "test/Driver/linux-header-search.cpp" needs to be updated. x32 is actually supported in both an x86_64-linux-gnu environment and x86_64-linux-gnux32 environment. In order to update test/Driver/linux-header-search.cpp, we would have to add a debian-10-x32 chroot to the LLVM test environment. However, the patch by @jrtc27 actually doesn't break the previous tests as his patch just allows the native header and library paths as altnernative search paths. The same applies to test/Driver/cross-linux.c. The x32 test there will still remain valid, even with the changes. Repository: rC Clang https://reviews.llvm.org/D43630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good (with a couple of minor code style adjustments). Do you need someone to commit this for you? In https://reviews.llvm.org/D51333#1233042, @sammccall wrote: > One thing I'd like to do sometime is add code completion of filenames in > #include directives. > This would mean listing the relevant directories from the include path. (VFS > is slow for this today, but I have a patch out for it). > > Maybe it would make sense to do that here and use edit-distance to pick the > suggestion, like typo correction elsewhere? > Could be a way to extend this patch's behavior in the future. I agree; I think this would be a very interesting extension for this patch if someone feels motivated. Comment at: lib/Lex/PPDirectives.cpp:1891 + if (!File) { +auto originalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { Nit: please spell out the type here, because it's not obvious from the initializer. Please capitalize the variable name. Comment at: lib/Lex/PPDirectives.cpp:1907 + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") Nit: please capitalize this variable name. https://reviews.llvm.org/D51333 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
rsmith added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1268 + // Instantiate the attributes on both the template declaration and the associated record declaration. + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs, StartingScope); + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs, StartingScope); ldionne wrote: > erik.pilkington wrote: > > Why are you instantiating the attributes onto the ClassTemplateDecl? At the > > very least it seems wrong to instantiate attributes from the pattern to the > > ClassTemplateDecl. > My understanding was that > - `Inst` represented `B::X` (the template) > - `RecordInst` represented `B::X` (the template specialization, > which is a "real struct") > - `Pattern` represented `B::X` > > If that is right, then it seemed to make sense that both `B::X` and > `B::X` should have the attribute, since `B::X` has it. I > might be misunderstanding something though. That's almost right. * `Inst` represents `B::X` (the template declaration in the instantiation of `B`). * `RecordInst` represents `B::X` (the class declaration within the template declaration in the instantiation of `B`). * `Pattern` represents `B::X` (the class declaration within the template declaration in the definition of `B`). And we don't yet have a value for `U`, so there is no `B::X` here. That is: ``` template struct B { template // #1 struct X; // Pattern }; // implicit instantiation of B: template<> struct B { template // Inst struct X; // RecordInst }; ``` A template-declaration can never have attributes on it (there is no syntactic location where they can be written), so we should never be instantiating attributes onto it. If it could, we should instantiate those attributes from `#1`, not from `Pattern`. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3247 InstPartialSpec->setTypeAsWritten(WrittenTy); // Check the completed partial specialization. erik.pilkington wrote: > ldionne wrote: > > I tried adding > > > > ``` > > SemaRef.InstantiateAttrsForDecl(TemplateArgs, > > ClassTemplate->getTemplatedDecl(), InstPartialSpec, LateAttrs, > > StartingScope); > > ``` > > > > here, but just like for explicit specializations, that doesn't instantiate > > the attributes on the `CXXRecordDecl` used to determine mangling. > > > You mean `SemaRef.InstantiateAttrsForDecl(TemplateArgs, PartialSpec, > InstPartialSpec, LateAttrs, StartingScope);`? erik's suggestion would be the right thing to do. This case is for instantiating a class-scope partial specialization inside a template to form another class-scope partial specialization: ``` template struct A { template struct B; template struct ATTRS B; // PartialSpec }; // implicit instantiation of A looks like: template<> struct A { template struct B; template struct ATTRS B; // InstPartialSpec }; ``` ... where we want to instantiate attributes from `PartialSpec` to `InstPartialSpec`. Unlike in the class template case, we don't have separate AST nodes for the template and the class within it for partial specializations (I think our general consensus is that's a design mistake, but addressing it would be a substantial refactoring). Comment at: clang/test/SemaCXX/PR38913.cpp:19 +}; +C::X* c() { return 0; } // CHECK-DAG: @_Z1cB4CTAGv + erik.pilkington wrote: > ldionne wrote: > > This test is failing in the current iteration of the patch. > I think the problem here is that we don't instantiate X because > it's behind the pointer, so we're just considering the primary template. > Looks like we do add the attribute (with the fix above) here: > ``` > template struct C { > template struct X { }; > template struct __attribute__((abi_tag("CTAG"))) X { }; > }; > C::X c() { return 0; } > ``` > But we don't get the right mangling, for some reason. Note that this problem > is present even in the non-member case. > > I don't think attributes on specializations have particularly good support > anyways, so I wouldn't really lose any sleep if we "left this to a > follow-up". Maybe @rsmith has some thoughts here. This should call `TemplateDeclInstantiator::VisitCXXRecordDecl` to create the member class declaration when instantiating the outer class template specialization `C`. I'm not sure why that isn't instantiating the attributes from the declaration. Repository: rC Clang https://reviews.llvm.org/D51997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43630: [Driver] Fix search paths on x32
glaubitz added a comment. I have just rebased the patch and I am looking at the tests now. Repository: rC Clang https://reviews.llvm.org/D43630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.
lebedev.ri added a comment. In https://reviews.llvm.org/D52008#1233667, @shuaiwang wrote: > Just some quick comments, I'll take a deeper look into other comments later. > > This diff along unfortunately won't be able to handle `emplace_back` just yet My apologies, for some reason i though it was supposed to. Once again, thank you for working on this! > , the reason (I believe, haven't fully tested) is that `std::forward` is not > handled properly and almost all std functions involving forwarding references > at least `std::forward`'ed once. > I have some more changes locally that treats `std::move` and `std::forward` > just as casts and that should be able to really push the analysis further > down the forwarding chain instead of stopping at `std::forward` call. > Rephased diff description to be more clear. Sorry for the confusion. No problem! Repository: rC Clang https://reviews.llvm.org/D52008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.
JonasToth added a comment. In https://reviews.llvm.org/D52008#1233667, @shuaiwang wrote: > Just some quick comments, I'll take a deeper look into other comments later. > > This diff along unfortunately won't be able to handle `emplace_back` just > yet, the reason (I believe, haven't fully tested) is that `std::forward` is > not handled properly and almost all std functions involving forwarding > references at least `std::forward`'ed once. > I have some more changes locally that treats `std::move` and `std::forward` > just as casts and that should be able to really push the analysis further > down the forwarding chain instead of stopping at `std::forward` call. > Rephased diff description to be more clear. Sorry for the confusion. I did look up the rules for resolved the universal references. void f() { int x; std::move(x); } Will create a normal reference to `x`. This would be the point were have to recursivly follow all references created from a value and check if they are mutated. Do you see another possibility on handling universal references correctly? Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:60 +public: + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); + shuaiwang wrote: > JonasToth wrote: > > Why do we need a separate class for this? > > I think you can just create another object of `ExprMutAnalyzer` and do the > > analysis with `findDeclMutation(FunctioParm)`. > > > > The `Stmt` is the `functionDecl().getBody()`. Right now it could be that > > you pass in an functionDecl without body. > > > > Could there something happen with extern templates that the body is not > > accessible? > It's mostly for the special handling of constructors in which case > initializer list also could mutate param. Hmm, still why not within `ExprMutAnalyzer`? You could make it a class living within ExprMutAnalyzer in the private section. Then its clear its an implementation detail. Repository: rC Clang https://reviews.llvm.org/D52008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
ldionne marked 2 inline comments as done. ldionne added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1268 + // Instantiate the attributes on both the template declaration and the associated record declaration. + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs, StartingScope); + SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs, StartingScope); erik.pilkington wrote: > Why are you instantiating the attributes onto the ClassTemplateDecl? At the > very least it seems wrong to instantiate attributes from the pattern to the > ClassTemplateDecl. My understanding was that - `Inst` represented `B::X` (the template) - `RecordInst` represented `B::X` (the template specialization, which is a "real struct") - `Pattern` represented `B::X` If that is right, then it seemed to make sense that both `B::X` and `B::X` should have the attribute, since `B::X` has it. I might be misunderstanding something though. Comment at: clang/test/SemaCXX/PR38913.cpp:19 +}; +C::X* c() { return 0; } // CHECK-DAG: @_Z1cB4CTAGv + erik.pilkington wrote: > ldionne wrote: > > This test is failing in the current iteration of the patch. > I think the problem here is that we don't instantiate X because > it's behind the pointer, so we're just considering the primary template. > Looks like we do add the attribute (with the fix above) here: > ``` > template struct C { > template struct X { }; > template struct __attribute__((abi_tag("CTAG"))) X { }; > }; > C::X c() { return 0; } > ``` > But we don't get the right mangling, for some reason. Note that this problem > is present even in the non-member case. > > I don't think attributes on specializations have particularly good support > anyways, so I wouldn't really lose any sleep if we "left this to a > follow-up". Maybe @rsmith has some thoughts here. I'll drop support for specializations for now. The current patch is enough to unblock me on the `no_extern_template` attribute, which is the goal of all of this. Repository: rC Clang https://reviews.llvm.org/D51997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations
JonasToth added inline comments. Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:35 +namespace { +SourceLocation findNextTerminator(SourceLocation Start, const SourceManager &SM, + const LangOptions &LangOpts) { Remove duplication, make it a variadic template and use `Token.isOneOf(TemplateArgs...)` Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:74 +/// This function assumes, that \p DS only contains VarDecl elements. +std::vector DeclSlice(const DeclStmt *DS, const SourceManager &SM, + const LangOptions &LangOpts) { Make this an `llvm::Expected` and bail on all macro occurences or any other invalid sourcelocations. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.
shuaiwang added a comment. Just some quick comments, I'll take a deeper look into other comments later. This diff along unfortunately won't be able to handle `emplace_back` just yet, the reason (I believe, haven't fully tested) is that `std::forward` is not handled properly and almost all std functions involving forwarding references at least `std::forward`'ed once. I have some more changes locally that treats `std::move` and `std::forward` just as casts and that should be able to really push the analysis further down the forwarding chain instead of stopping at `std::forward` call. Rephased diff description to be more clear. Sorry for the confusion. Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:60 +public: + FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context); + JonasToth wrote: > Why do we need a separate class for this? > I think you can just create another object of `ExprMutAnalyzer` and do the > analysis with `findDeclMutation(FunctioParm)`. > > The `Stmt` is the `functionDecl().getBody()`. Right now it could be that you > pass in an functionDecl without body. > > Could there something happen with extern templates that the body is not > accessible? It's mostly for the special handling of constructors in which case initializer list also could mutate param. Repository: rC Clang https://reviews.llvm.org/D52008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52047: [clangd] Add a "benchmark" for tracking memory
kbobyrev created this revision. kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall. kbobyrev added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. While this looks like misuse of benchmark library, this can be still when experimenting with index compression techniques as discussed offline. https://reviews.llvm.org/D52047 Files: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp === --- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp +++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp @@ -66,6 +66,27 @@ return Requests; } +// This is not a *real* benchmark: it shows size of built MemIndex (in bytes). +// Same for the next "benchmark". +// FIXME(kbobyrev): Should this be separated into the BackingMemorySize +// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead? +static void MemSize(benchmark::State &State) { + const auto Mem = buildMem(); + for (auto _ : State) +// Divide size of Mem by 1000 so that it will be correctly displayed in the +// benchmark report (possible options for time units are ms, ns and us). +State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() / + 1000); +} +BENCHMARK(MemSize)->UseManualTime()->Unit(benchmark::kMillisecond); + +static void DexSize(benchmark::State &State) { + const auto Dex = buildDex(); + for (auto _ : State) +State.SetIterationTime(Dex->estimateMemoryUsage() / 1000); +} +BENCHMARK(DexSize)->UseManualTime()->Unit(benchmark::kMillisecond); + static void MemQueries(benchmark::State &State) { const auto Mem = buildMem(); const auto Requests = extractQueriesFromLogs(); Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp === --- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp +++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp @@ -66,6 +66,27 @@ return Requests; } +// This is not a *real* benchmark: it shows size of built MemIndex (in bytes). +// Same for the next "benchmark". +// FIXME(kbobyrev): Should this be separated into the BackingMemorySize +// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead? +static void MemSize(benchmark::State &State) { + const auto Mem = buildMem(); + for (auto _ : State) +// Divide size of Mem by 1000 so that it will be correctly displayed in the +// benchmark report (possible options for time units are ms, ns and us). +State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() / + 1000); +} +BENCHMARK(MemSize)->UseManualTime()->Unit(benchmark::kMillisecond); + +static void DexSize(benchmark::State &State) { + const auto Dex = buildDex(); + for (auto _ : State) +State.SetIterationTime(Dex->estimateMemoryUsage() / 1000); +} +BENCHMARK(DexSize)->UseManualTime()->Unit(benchmark::kMillisecond); + static void MemQueries(benchmark::State &State) { const auto Mem = buildMem(); const auto Requests = extractQueriesFromLogs(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51982: [clangd] Introduce PostingList interface
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE342155: [clangd] Introduce PostingList interface (authored by omtcyfz, committed by ). Changed prior to commit: https://reviews.llvm.org/D51982?vs=165324&id=165325#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51982 Files: clangd/CMakeLists.txt clangd/index/dex/Dex.cpp clangd/index/dex/Dex.h clangd/index/dex/Iterator.cpp clangd/index/dex/Iterator.h clangd/index/dex/PostingList.cpp clangd/index/dex/PostingList.h unittests/clangd/DexTests.cpp Index: unittests/clangd/DexTests.cpp === --- unittests/clangd/DexTests.cpp +++ unittests/clangd/DexTests.cpp @@ -47,8 +47,8 @@ } TEST(DexIterators, DocumentIterator) { - const PostingList L = {4, 7, 8, 20, 42, 100}; - auto DocIterator = create(L); + const PostingList L({4, 7, 8, 20, 42, 100}); + auto DocIterator = L.iterator(); EXPECT_EQ(DocIterator->peek(), 4U); EXPECT_FALSE(DocIterator->reachedEnd()); @@ -70,28 +70,28 @@ } TEST(DexIterators, AndWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto AndEmpty = createAnd(create(L0)); + auto AndEmpty = createAnd(L0.iterator()); EXPECT_TRUE(AndEmpty->reachedEnd()); - auto AndWithEmpty = createAnd(create(L0), create(L1)); + auto AndWithEmpty = createAnd(L0.iterator(), L1.iterator()); EXPECT_TRUE(AndWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre()); } TEST(DexIterators, AndTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto And = createAnd(create(L1), create(L0)); + auto And = createAnd(L1.iterator(), L0.iterator()); EXPECT_FALSE(And->reachedEnd()); EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); - And = createAnd(create(L0), create(L1)); + And = createAnd(L0.iterator(), L1.iterator()); And->advanceTo(0); EXPECT_EQ(And->peek(), 0U); @@ -107,11 +107,11 @@ } TEST(DexIterators, AndThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto And = createAnd(create(L0), create(L1), create(L2)); + auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_EQ(And->peek(), 7U); And->advanceTo(300); EXPECT_EQ(And->peek(), 320U); @@ -121,24 +121,24 @@ } TEST(DexIterators, OrWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto OrEmpty = createOr(create(L0)); + auto OrEmpty = createOr(L0.iterator()); EXPECT_TRUE(OrEmpty->reachedEnd()); - auto OrWithEmpty = createOr(create(L0), create(L1)); + auto OrWithEmpty = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(OrWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*OrWithEmpty), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } TEST(DexIterators, OrTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1)); + auto Or = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -161,18 +161,18 @@ Or->advanceTo(9001); EXPECT_TRUE(Or->reachedEnd()); - Or = createOr(create(L0), create(L1)); + Or = createOr(L0.iterator(), L1.iterator()); EXPECT_THAT(consumeIDs(*Or), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } TEST(DexIterators, OrThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1), create(L2)); + auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -221,19 +221,19 @@ // |1, 5, 7, 9| |1, 5||0, 3, 5| // +--+
[clang-tools-extra] r342155 - [clangd] Introduce PostingList interface
Author: omtcyfz Date: Thu Sep 13 10:11:03 2018 New Revision: 342155 URL: http://llvm.org/viewvc/llvm-project?rev=342155&view=rev Log: [clangd] Introduce PostingList interface This patch abstracts `PostingList` interface and reuses existing implementation. It will be used later to test different `PostingList` representations. No functionality change is introduced, this patch is mostly refactoring so that the following patches could focus on functionality while not being too hard to review. Reviewed By: sammccall, ioeric Differential Revision: https://reviews.llvm.org/D51982 Added: clang-tools-extra/trunk/clangd/index/dex/PostingList.cpp clang-tools-extra/trunk/clangd/index/dex/PostingList.h Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt clang-tools-extra/trunk/clangd/index/dex/Dex.cpp clang-tools-extra/trunk/clangd/index/dex/Dex.h clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp clang-tools-extra/trunk/clangd/index/dex/Iterator.h clang-tools-extra/trunk/unittests/clangd/DexTests.cpp Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=342155&r1=342154&r2=342155&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Sep 13 10:11:03 2018 @@ -48,6 +48,7 @@ add_clang_library(clangDaemon index/dex/Dex.cpp index/dex/Iterator.cpp + index/dex/PostingList.cpp index/dex/Trigram.cpp LINK_LIBS Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=342155&r1=342154&r2=342155&view=diff == --- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Thu Sep 13 10:11:03 2018 @@ -82,7 +82,7 @@ std::vector> c // FIXME(kbobyrev): Append LIMIT on top of every BOOST iterator. PathProximitySignals.SymbolURI = ParentURI; BoostingIterators.push_back( - createBoost(create(It->second), PathProximitySignals.evaluate())); + createBoost(It->second.iterator(), PathProximitySignals.evaluate())); } } return BoostingIterators; @@ -112,13 +112,19 @@ void Dex::buildIndex() { Symbols[I] = ScoredSymbols[I].second; } - // Populate TempInvertedIndex with posting lists for index symbols. + // Populate TempInvertedIndex with lists for index symbols. + llvm::DenseMap> TempInvertedIndex; for (DocID SymbolRank = 0; SymbolRank < Symbols.size(); ++SymbolRank) { const auto *Sym = Symbols[SymbolRank]; for (const auto &Token : generateSearchTokens(*Sym)) - InvertedIndex[Token].push_back(SymbolRank); + TempInvertedIndex[Token].push_back(SymbolRank); } + // Convert lists of items to posting lists. + for (const auto &TokenToPostingList : TempInvertedIndex) +InvertedIndex.insert({TokenToPostingList.first, + PostingList(move(TokenToPostingList.second))}); + vlog("Built Dex with estimated memory usage {0} bytes.", estimateMemoryUsage()); } @@ -142,7 +148,7 @@ bool Dex::fuzzyFind(const FuzzyFindReque for (const auto &Trigram : TrigramTokens) { const auto It = InvertedIndex.find(Trigram); if (It != InvertedIndex.end()) - TrigramIterators.push_back(create(It->second)); + TrigramIterators.push_back(It->second.iterator()); } if (!TrigramIterators.empty()) TopLevelChildren.push_back(createAnd(move(TrigramIterators))); @@ -152,7 +158,7 @@ bool Dex::fuzzyFind(const FuzzyFindReque for (const auto &Scope : Req.Scopes) { const auto It = InvertedIndex.find(Token(Token::Kind::Scope, Scope)); if (It != InvertedIndex.end()) - ScopeIterators.push_back(create(It->second)); + ScopeIterators.push_back(It->second.iterator()); } // Add OR iterator for scopes if there are any Scope Iterators. if (!ScopeIterators.empty()) @@ -233,7 +239,7 @@ size_t Dex::estimateMemoryUsage() const Bytes += LookupTable.getMemorySize(); Bytes += InvertedIndex.getMemorySize(); for (const auto &P : InvertedIndex) -Bytes += P.second.size() * sizeof(DocID); +Bytes += P.second.bytes(); return Bytes + BackingDataSize; } Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.h?rev=342155&r1=342154&r2=342155&view=diff == --- clang-tools-extra/trunk/clangd/index/dex/Dex.h (original) +++ clang-tools-extra/trunk/clangd/index/dex/Dex.h Thu Sep 13 10:11:03 2018 @@ -21,6 +21,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_DEX_DEX_H #include "Iterator.h" +#include "PostingList.h" #include "Toke
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev updated this revision to Diff 165324. kbobyrev marked 3 inline comments as done. kbobyrev added a comment. Pull `DocID` to `Iterator.h`. https://reviews.llvm.org/D51982 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/index/dex/Dex.cpp clang-tools-extra/clangd/index/dex/Dex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h clang-tools-extra/clangd/index/dex/PostingList.cpp clang-tools-extra/clangd/index/dex/PostingList.h clang-tools-extra/unittests/clangd/DexTests.cpp Index: clang-tools-extra/unittests/clangd/DexTests.cpp === --- clang-tools-extra/unittests/clangd/DexTests.cpp +++ clang-tools-extra/unittests/clangd/DexTests.cpp @@ -47,8 +47,8 @@ } TEST(DexIterators, DocumentIterator) { - const PostingList L = {4, 7, 8, 20, 42, 100}; - auto DocIterator = create(L); + const PostingList L({4, 7, 8, 20, 42, 100}); + auto DocIterator = L.iterator(); EXPECT_EQ(DocIterator->peek(), 4U); EXPECT_FALSE(DocIterator->reachedEnd()); @@ -70,28 +70,28 @@ } TEST(DexIterators, AndWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto AndEmpty = createAnd(create(L0)); + auto AndEmpty = createAnd(L0.iterator()); EXPECT_TRUE(AndEmpty->reachedEnd()); - auto AndWithEmpty = createAnd(create(L0), create(L1)); + auto AndWithEmpty = createAnd(L0.iterator(), L1.iterator()); EXPECT_TRUE(AndWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre()); } TEST(DexIterators, AndTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto And = createAnd(create(L1), create(L0)); + auto And = createAnd(L1.iterator(), L0.iterator()); EXPECT_FALSE(And->reachedEnd()); EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); - And = createAnd(create(L0), create(L1)); + And = createAnd(L0.iterator(), L1.iterator()); And->advanceTo(0); EXPECT_EQ(And->peek(), 0U); @@ -107,11 +107,11 @@ } TEST(DexIterators, AndThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto And = createAnd(create(L0), create(L1), create(L2)); + auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_EQ(And->peek(), 7U); And->advanceTo(300); EXPECT_EQ(And->peek(), 320U); @@ -121,24 +121,24 @@ } TEST(DexIterators, OrWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto OrEmpty = createOr(create(L0)); + auto OrEmpty = createOr(L0.iterator()); EXPECT_TRUE(OrEmpty->reachedEnd()); - auto OrWithEmpty = createOr(create(L0), create(L1)); + auto OrWithEmpty = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(OrWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*OrWithEmpty), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } TEST(DexIterators, OrTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1)); + auto Or = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -161,18 +161,18 @@ Or->advanceTo(9001); EXPECT_TRUE(Or->reachedEnd()); - Or = createOr(create(L0), create(L1)); + Or = createOr(L0.iterator(), L1.iterator()); EXPECT_THAT(consumeIDs(*Or), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } TEST(DexIterators, OrThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1), create(L2)); + auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -221,19 +221,19 @@ // |1, 5, 7, 9| |1, 5||0, 3, 5|
[PATCH] D51808: [CUDA] Ignore uncallable functions when we check for usual deallocators.
tra added a comment. @rsmith ping. https://reviews.llvm.org/D51808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations
JonasToth updated this revision to Diff 165321. JonasToth added a comment. This is the newer and better working code to slice and separate multiple vardecls in a multi-decl stmt. I want this version only for now, as the multiple variable definitions in one statement are by far the most common variant of the single-decl-per-stmt rule. All other cases are supposed to be diagnosed only. The next steps will be: - add the testsuite of the other abondended check - dont fixit for places like `for(int start = 0, end = 42; ...)` This patch includes: - do proper transformations for normal vardecls - finalize current tests and trim right whitespace of last generated replacement Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/IsolateDeclCheck.cpp clang-tidy/readability/IsolateDeclCheck.h clang-tidy/readability/ReadabilityTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-isolate-decl.rst test/clang-tidy/readability-isolate-decl.cpp Index: test/clang-tidy/readability-isolate-decl.cpp === --- /dev/null +++ test/clang-tidy/readability-isolate-decl.cpp @@ -0,0 +1,82 @@ +// RUN: %check_clang_tidy %s readability-isolate-decl %t + +void f() { + int i; +} + +void f2() { + int i, j, *k, lala = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: int i; + // CHECK-FIXES: {{^ }}int j; + // CHECK-FIXES: {{^ }}int *k; + // CHECK-FIXES: {{^ }}int lala = 42; + + int normal, weird = /* comment */ 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: int normal; + // CHECK-FIXES: {{^ }}int weird = /* comment */ 42; +} + +void f3() { + int i, *pointer1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: int i; + // CHECK-FIXES: {{^ }}int *pointer1; + // + int *pointer2 = nullptr, *pointer3 = &i; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: int *pointer2 = nullptr; + // CHECK-FIXES: {{^ }}int *pointer3 = &i; +} + +void f4() { + double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /* */, l = 2.; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: double d = 42. /* foo */; + // CHECK-FIXES: {{^ }}double z = 43.; + // CHECK-FIXES: {{^ }}double /* hi */ y; + // CHECK-FIXES: {{^ }}double c /* */ /* */; + // CHECK-FIXES: {{^ }}double l = 2.; +} + +struct SomeClass { + SomeClass() = default; + SomeClass(int value); +}; +void f5() { + SomeClass v1, v2(42), v3{42}, v4(42.5); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: SomeClass v1; + // CHECK-FIXES: {{^ }}SomeClass v2(42); + // CHECK-FIXES: {{^ }}SomeClass v3{42}; + // CHECK-FIXES: {{^ }}SomeClass v4(42.5); + + SomeClass v5 = 42, *p1 = nullptr; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: SomeClass v5 = 42; + // CHECK-FIXES: {{^ }}SomeClass *p1 = nullptr; +} + +void f6() { + int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: int array1[] = {1, 2, 3, 4}; + // CHECK-FIXES: {{^ }}int array2[] = {1, 2, 3}; + // CHECK-FIXES: {{^ }}int value1; + // CHECK-FIXES: {{^ }}int value2 = 42; +} + +template +struct TemplatedType { + TemplatedType() = default; + TemplatedType(T value); +}; + +void f7() { + TemplatedType TT1(42), TT2{42}, TT3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: make only one declaration per statement + // CHECK-FIXES: TemplatedType TT1(42); + // CHECK-FIXES: {{^ }}TemplatedType TT2{42}; + // CHECK-FIXES: {{^ }}TemplatedType TT3; +} Index: docs/clang-tidy/checks/readability-isolate-decl.rst === --- /dev/null +++ docs/clang-tidy/checks/readability-isolate-decl.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - readability-isolate-decl + +readability-isolate-decl + + +FIXME: Describe what patterns does the check detect and why. Give examples. Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -225,6 +225,7 @@ readability-identifier-naming readability-implicit-bool-conversion readability-inconsistent-declaration-parameter-name + readability-isolate-decl readability-magic-numbers readability-misleading-indentation readability-misplaced-array-index Index: docs/ReleaseNotes.rst === --- docs/Rel
r342152 - [NFC]Refactor MultiVersion Resolver Emission to combine types
Author: erichkeane Date: Thu Sep 13 09:58:24 2018 New Revision: 342152 URL: http://llvm.org/viewvc/llvm-project?rev=342152&view=rev Log: [NFC]Refactor MultiVersion Resolver Emission to combine types Previously, both types (plus the future target-clones) of multiversioning had a separate ResolverOption structure and emission function. This patch combines the two, at the expense of a slightly more expensive sorting function. Modified: cfe/trunk/include/clang/Basic/Attr.td cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/lib/CodeGen/CodeGenModule.cpp Modified: cfe/trunk/include/clang/Basic/Attr.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=342152&r1=342151&r2=342152&view=diff == --- cfe/trunk/include/clang/Basic/Attr.td (original) +++ cfe/trunk/include/clang/Basic/Attr.td Thu Sep 13 09:58:24 2018 @@ -1977,6 +1977,39 @@ def Target : InheritableAttr { return parse(getFeaturesStr()); } +StringRef getArchitecture() const { + StringRef Features = getFeaturesStr(); + if (Features == "default") return {}; + + SmallVector AttrFeatures; + Features.split(AttrFeatures, ","); + + for (auto &Feature : AttrFeatures) { +Feature = Feature.trim(); +if (Feature.startswith("arch=")) + return Feature.drop_front(sizeof("arch=") - 1); + } + return ""; +} + +// Gets the list of features as simple string-refs with no +/- or 'no-'. +// Only adds the items to 'Out' that are additions. +void getAddedFeatures(llvm::SmallVectorImpl &Out) const { + StringRef Features = getFeaturesStr(); + if (Features == "default") return; + + SmallVector AttrFeatures; + Features.split(AttrFeatures, ","); + + for (auto &Feature : AttrFeatures) { +Feature = Feature.trim(); + +if (!Feature.startswith("no-") && !Feature.startswith("arch=") && +!Feature.startswith("fpmath=") && !Feature.startswith("tune=")) + Out.push_back(Feature); + } +} + template ParsedTargetAttr parse(Compare cmp) const { ParsedTargetAttr Attrs = parse(); Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=342152&r1=342151&r2=342152&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Sep 13 09:58:24 2018 @@ -2363,91 +2363,53 @@ void CodeGenFunction::EmitSanitizerStatR CGM.getSanStats().create(IRB, SSK); } -llvm::Value *CodeGenFunction::FormResolverCondition( -const TargetMultiVersionResolverOption &RO) { - llvm::Value *TrueCondition = nullptr; - if (!RO.ParsedAttribute.Architecture.empty()) -TrueCondition = EmitX86CpuIs(RO.ParsedAttribute.Architecture); - - if (!RO.ParsedAttribute.Features.empty()) { -SmallVector FeatureList; -llvm::for_each(RO.ParsedAttribute.Features, - [&FeatureList](const std::string &Feature) { - FeatureList.push_back(StringRef{Feature}.substr(1)); - }); -llvm::Value *FeatureCmp = EmitX86CpuSupports(FeatureList); -TrueCondition = TrueCondition ? Builder.CreateAnd(TrueCondition, FeatureCmp) - : FeatureCmp; +llvm::Value * +CodeGenFunction::FormResolverCondition(const MultiVersionResolverOption &RO) { + llvm::Value *Condition = nullptr; + + if (!RO.Conditions.Architecture.empty()) +Condition = EmitX86CpuIs(RO.Conditions.Architecture); + + if (!RO.Conditions.Features.empty()) { +llvm::Value *FeatureCond = EmitX86CpuSupports(RO.Conditions.Features); +Condition = +Condition ? Builder.CreateAnd(Condition, FeatureCond) : FeatureCond; } - return TrueCondition; + return Condition; } -void CodeGenFunction::EmitTargetMultiVersionResolver( -llvm::Function *Resolver, -ArrayRef Options) { +void CodeGenFunction::EmitMultiVersionResolver( +llvm::Function *Resolver, ArrayRef Options) { assert((getContext().getTargetInfo().getTriple().getArch() == llvm::Triple::x86 || getContext().getTargetInfo().getTriple().getArch() == llvm::Triple::x86_64) && "Only implemented for x86 targets"); - - // Main function's basic block. - llvm::BasicBlock *CurBlock = createBasicBlock("entry", Resolver); - Builder.SetInsertPoint(CurBlock); - EmitX86CpuInit(); - - llvm::Function *DefaultFunc = nullptr; - for (const TargetMultiVersionResolverOption &RO : Options) { -Builder.SetInsertPoint(CurBlock); -llvm::Value *TrueCondition = FormResolverCondition(RO); - -if (!TrueCondition) { - DefaultFunc = RO.Function; -} else { - llvm::BasicBlock *RetBlock = createBasicBlock("ro_ret",
r342151 - [OPENMP] Fix PR38903: Crash on instantiation of the non-dependent
Author: abataev Date: Thu Sep 13 09:54:05 2018 New Revision: 342151 URL: http://llvm.org/viewvc/llvm-project?rev=342151&view=rev Log: [OPENMP] Fix PR38903: Crash on instantiation of the non-dependent declare reduction. If the declare reduction construct with the non-dependent type is defined in the template construct, the compiler might crash on the template instantition. Reworked the whole instantiation scheme for the declare reduction constructs to fix this problem correctly. Modified: cfe/trunk/include/clang/AST/DeclOpenMP.h cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h cfe/trunk/lib/Parse/ParseOpenMP.cpp cfe/trunk/lib/Sema/SemaOpenMP.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/lib/Serialization/ASTWriterDecl.cpp cfe/trunk/test/OpenMP/declare_reduction_messages.cpp Modified: cfe/trunk/include/clang/AST/DeclOpenMP.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclOpenMP.h?rev=342151&r1=342150&r2=342151&view=diff == --- cfe/trunk/include/clang/AST/DeclOpenMP.h (original) +++ cfe/trunk/include/clang/AST/DeclOpenMP.h Thu Sep 13 09:54:05 2018 @@ -112,9 +112,17 @@ public: private: friend class ASTDeclReader; /// Combiner for declare reduction construct. - Expr *Combiner; + Expr *Combiner = nullptr; /// Initializer for declare reduction construct. - Expr *Initializer; + Expr *Initializer = nullptr; + /// In parameter of the combiner. + Expr *In = nullptr; + /// Out parameter of the combiner. + Expr *Out = nullptr; + /// Priv parameter of the initializer. + Expr *Priv = nullptr; + /// Orig parameter of the initializer. + Expr *Orig = nullptr; /// Reference to the previous declare reduction construct in the same /// scope with the same name. Required for proper templates instantiation if @@ -143,8 +151,19 @@ public: /// Get combiner expression of the declare reduction construct. Expr *getCombiner() { return Combiner; } const Expr *getCombiner() const { return Combiner; } + /// Get In variable of the combiner. + Expr *getCombinerIn() { return In; } + const Expr *getCombinerIn() const { return In; } + /// Get Out variable of the combiner. + Expr *getCombinerOut() { return Out; } + const Expr *getCombinerOut() const { return Out; } /// Set combiner expression for the declare reduction construct. void setCombiner(Expr *E) { Combiner = E; } + /// Set combiner In and Out vars. + void setCombinerData(Expr *InE, Expr *OutE) { +In = InE; +Out = OutE; + } /// Get initializer expression (if specified) of the declare reduction /// construct. @@ -154,11 +173,22 @@ public: InitKind getInitializerKind() const { return static_cast(OMPDeclareReductionDeclBits.InitializerKind); } + /// Get Orig variable of the initializer. + Expr *getInitOrig() { return Orig; } + const Expr *getInitOrig() const { return Orig; } + /// Get Priv variable of the initializer. + Expr *getInitPriv() { return Priv; } + const Expr *getInitPriv() const { return Priv; } /// Set initializer expression for the declare reduction construct. void setInitializer(Expr *E, InitKind IK) { Initializer = E; OMPDeclareReductionDeclBits.InitializerKind = IK; } + /// Set initializer Orig and Priv vars. + void setInitializerData(Expr *OrigE, Expr *PrivE) { +Orig = OrigE; +Priv = PrivE; + } /// Get reference to previous declare reduction construct in the same /// scope with the same name. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=342151&r1=342150&r2=342151&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Sep 13 09:54:05 2018 @@ -1295,27 +1295,19 @@ void CGOpenMPRuntime::emitUserDefinedRed CodeGenFunction *CGF, const OMPDeclareReductionDecl *D) { if (UDRMap.count(D) > 0) return; - ASTContext &C = CGM.getContext(); - if (!In || !Out) { -In = &C.Idents.get("omp_in"); -Out = &C.Idents.get("omp_out"); - } llvm::Function *Combiner = emitCombinerOrInitializer( - CGM, D->getType(), D->getCombiner(), cast(D->lookup(In).front()), - cast(D->lookup(Out).front()), + CGM, D->getType(), D->getCombiner(), + cast(cast(D->getCombinerIn())->getDecl()), + cast(cast(D->getCombinerOut())->getDecl()), /*IsCombiner=*/true); llvm::Function *Initializer = nullptr; if (const Expr *Init = D->getInitializer()) { -if (!Priv || !Orig) { - Priv = &C.Idents.get("omp_priv"); - Orig = &C.Idents.get("omp_orig"); -} Initializer = emitCombinerOrInitializer( CGM, D->getType(), D->getInitializerKind() =
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:39 #include +#include "PostingList.h" kbobyrev wrote: > sammccall wrote: > > Why this dep? Seems circular > `Iterator` interface uses `DocID`, so I guess it should depend on > `PostingList.h`, shouldn't it? FWIW I tried to get rid of this dependency in the first place, but it seemed pretty hard, because the options I saw were either forward declaring (which isn't possible since it's practically typedef) it or introducing another `using DocID = uint32_t` in `Iterator.h`, which I decided not to do because of the code duplication. Another option would be to make both `PostingList.h` and `Iterator.h` depend on some common file, but semantically `DocID` seems to belong to the `Iterator.h`, so I didn't think that would be a good solution, too. Is there something I could do to resolve that? https://reviews.llvm.org/D51982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev marked an inline comment as done. kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:39 #include +#include "PostingList.h" sammccall wrote: > Why this dep? Seems circular `Iterator` interface uses `DocID`, so I guess it should depend on `PostingList.h`, shouldn't it? https://reviews.llvm.org/D51982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev updated this revision to Diff 165320. kbobyrev added a comment. Wording: traversed *in order*. https://reviews.llvm.org/D51982 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/index/dex/Dex.cpp clang-tools-extra/clangd/index/dex/Dex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h clang-tools-extra/clangd/index/dex/PostingList.cpp clang-tools-extra/clangd/index/dex/PostingList.h clang-tools-extra/unittests/clangd/DexTests.cpp Index: clang-tools-extra/unittests/clangd/DexTests.cpp === --- clang-tools-extra/unittests/clangd/DexTests.cpp +++ clang-tools-extra/unittests/clangd/DexTests.cpp @@ -47,8 +47,8 @@ } TEST(DexIterators, DocumentIterator) { - const PostingList L = {4, 7, 8, 20, 42, 100}; - auto DocIterator = create(L); + const PostingList L({4, 7, 8, 20, 42, 100}); + auto DocIterator = L.iterator(); EXPECT_EQ(DocIterator->peek(), 4U); EXPECT_FALSE(DocIterator->reachedEnd()); @@ -70,28 +70,28 @@ } TEST(DexIterators, AndWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto AndEmpty = createAnd(create(L0)); + auto AndEmpty = createAnd(L0.iterator()); EXPECT_TRUE(AndEmpty->reachedEnd()); - auto AndWithEmpty = createAnd(create(L0), create(L1)); + auto AndWithEmpty = createAnd(L0.iterator(), L1.iterator()); EXPECT_TRUE(AndWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre()); } TEST(DexIterators, AndTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto And = createAnd(create(L1), create(L0)); + auto And = createAnd(L1.iterator(), L0.iterator()); EXPECT_FALSE(And->reachedEnd()); EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); - And = createAnd(create(L0), create(L1)); + And = createAnd(L0.iterator(), L1.iterator()); And->advanceTo(0); EXPECT_EQ(And->peek(), 0U); @@ -107,11 +107,11 @@ } TEST(DexIterators, AndThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto And = createAnd(create(L0), create(L1), create(L2)); + auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_EQ(And->peek(), 7U); And->advanceTo(300); EXPECT_EQ(And->peek(), 320U); @@ -121,24 +121,24 @@ } TEST(DexIterators, OrWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto OrEmpty = createOr(create(L0)); + auto OrEmpty = createOr(L0.iterator()); EXPECT_TRUE(OrEmpty->reachedEnd()); - auto OrWithEmpty = createOr(create(L0), create(L1)); + auto OrWithEmpty = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(OrWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*OrWithEmpty), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } TEST(DexIterators, OrTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1)); + auto Or = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -161,18 +161,18 @@ Or->advanceTo(9001); EXPECT_TRUE(Or->reachedEnd()); - Or = createOr(create(L0), create(L1)); + Or = createOr(L0.iterator(), L1.iterator()); EXPECT_THAT(consumeIDs(*Or), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } TEST(DexIterators, OrThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1), create(L2)); + auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -221,19 +221,19 @@ // |1, 5, 7, 9| |1, 5||0, 3, 5| // +--+
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev updated this revision to Diff 165318. kbobyrev marked an inline comment as done. https://reviews.llvm.org/D51982 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/index/dex/Dex.cpp clang-tools-extra/clangd/index/dex/Dex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h clang-tools-extra/clangd/index/dex/PostingList.cpp clang-tools-extra/clangd/index/dex/PostingList.h clang-tools-extra/unittests/clangd/DexTests.cpp Index: clang-tools-extra/unittests/clangd/DexTests.cpp === --- clang-tools-extra/unittests/clangd/DexTests.cpp +++ clang-tools-extra/unittests/clangd/DexTests.cpp @@ -47,8 +47,8 @@ } TEST(DexIterators, DocumentIterator) { - const PostingList L = {4, 7, 8, 20, 42, 100}; - auto DocIterator = create(L); + const PostingList L({4, 7, 8, 20, 42, 100}); + auto DocIterator = L.iterator(); EXPECT_EQ(DocIterator->peek(), 4U); EXPECT_FALSE(DocIterator->reachedEnd()); @@ -70,28 +70,28 @@ } TEST(DexIterators, AndWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto AndEmpty = createAnd(create(L0)); + auto AndEmpty = createAnd(L0.iterator()); EXPECT_TRUE(AndEmpty->reachedEnd()); - auto AndWithEmpty = createAnd(create(L0), create(L1)); + auto AndWithEmpty = createAnd(L0.iterator(), L1.iterator()); EXPECT_TRUE(AndWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre()); } TEST(DexIterators, AndTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto And = createAnd(create(L1), create(L0)); + auto And = createAnd(L1.iterator(), L0.iterator()); EXPECT_FALSE(And->reachedEnd()); EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); - And = createAnd(create(L0), create(L1)); + And = createAnd(L0.iterator(), L1.iterator()); And->advanceTo(0); EXPECT_EQ(And->peek(), 0U); @@ -107,11 +107,11 @@ } TEST(DexIterators, AndThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto And = createAnd(create(L0), create(L1), create(L2)); + auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_EQ(And->peek(), 7U); And->advanceTo(300); EXPECT_EQ(And->peek(), 320U); @@ -121,24 +121,24 @@ } TEST(DexIterators, OrWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto OrEmpty = createOr(create(L0)); + auto OrEmpty = createOr(L0.iterator()); EXPECT_TRUE(OrEmpty->reachedEnd()); - auto OrWithEmpty = createOr(create(L0), create(L1)); + auto OrWithEmpty = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(OrWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*OrWithEmpty), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } TEST(DexIterators, OrTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1)); + auto Or = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -161,18 +161,18 @@ Or->advanceTo(9001); EXPECT_TRUE(Or->reachedEnd()); - Or = createOr(create(L0), create(L1)); + Or = createOr(L0.iterator(), L1.iterator()); EXPECT_THAT(consumeIDs(*Or), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } TEST(DexIterators, OrThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1), create(L2)); + auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -221,19 +221,19 @@ // |1, 5, 7, 9| |1, 5||0, 3, 5| // +--+ +--
[PATCH] D51982: [clangd] Introduce PostingList interface
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Great! Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:11 #include "Iterator.h" +#include "PostingList.h" #include And here Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:39 #include +#include "PostingList.h" Why this dep? Seems circular Comment at: clang-tools-extra/clangd/index/dex/PostingList.h:12 +// which can be characterized by a specific feature (such as fuzzy-find trigram, +// scope, type or any other Search Token). Posting lists are values for +// inverted index, which maps search tokens to corresponding posting lists. And can be traversed in order using an iterator https://reviews.llvm.org/D51982 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
christylee updated this revision to Diff 165311. christylee added a comment. Addressed @rsmith 's comments. https://reviews.llvm.org/D51333 Files: include/clang/Basic/DiagnosticLexKinds.td lib/Lex/PPDirectives.cpp Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +auto originalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << originalFilename << Filename << hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename Index: include/clang/Basic/DiagnosticLexKinds.td === --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -422,8 +422,10 @@ "#pragma hdrstop filename not supported, " "/Fp can be used to specify precompiled header filename">, InGroup; -def err_pp_file_not_found_not_fatal : Error< +def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with include; use \"quotes\" instead">; +def err_pp_file_not_found_typo_not_fatal +: Error<"'%0' file not found, did you mean '%1'?">; def err_pp_error_opening_file : Error< "error opening file '%0': %1">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1879,12 +1879,40 @@ Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); if (File) { SourceRange Range(FilenameTok.getLocation(), CharEnd); - Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) << + Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\""); } } + // Check for likely typos due to leading or trailing non-isAlphanumeric + // characters + if (!File) { +auto originalFilename = Filename; +while (!isAlphanumeric(Filename.front())) { + Filename = Filename.drop_front(); +} +while (!isAlphanumeric(Filename.back())) { + Filename = Filename.drop_back(); +} + +File = LookupFile( +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled, +LookupFrom, LookupFromFile, CurDir, +Callbacks ? &SearchPath : nullptr, +Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped); +if (File) { + SourceRange Range(FilenameTok.getLocation(), CharEnd); + auto hint = isAngled ? FixItHint::CreateReplacement( + Range, "<" + Filename.str() + ">") + : FixItHint::CreateReplacement( + Range, "\"" + Filename.str() + "\""); + Diag(FilenameTok, diag::err_pp_file_not_found_typo_not_fatal) + << originalFilename << Filename << hint; +} + } + // If the file is still not found, just go with the vanilla diagnostic if (!File) Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
[PATCH] D51747: [clangd] Show deprecation diagnostics
ilya-biryukov added a comment. In https://reviews.llvm.org/D51747#1233499, @kadircet wrote: > Np, not planning to land it before we figure out the issue with vim can't > handling lots of diagnostics anyway. Just wondering: what's the problem with Vim? Slowness? Bad UI for diagnostics when there are many of those? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51333: Diagnose likely typos in include statements
christylee added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1901 +FilenameLoc, +LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, false, +LookupFrom, LookupFromFile, CurDir, rsmith wrote: > You're passing in `false` for `isAngled` here, and producing a double-quoted > replacement below. Is that intentional? I would expect that we would preserve > the form of the header-name (quoted or angled) and suggest a replacement with > the same form. My mistake, thanks for catching it! https://reviews.llvm.org/D51333 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r342143 - [clangd] Fix Dexp build
Author: omtcyfz Date: Thu Sep 13 08:35:55 2018 New Revision: 342143 URL: http://llvm.org/viewvc/llvm-project?rev=342143&view=rev Log: [clangd] Fix Dexp build %s/MaxCandidateCount/Limit/g after rL342138. Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp?rev=342143&r1=342142&r2=342143&view=diff == --- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp (original) +++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp Thu Sep 13 08:35:55 2018 @@ -54,7 +54,7 @@ void reportTime(StringRef Name, llvm::fu void fuzzyFind(llvm::StringRef UnqualifiedName, const SymbolIndex &Index) { FuzzyFindRequest Request; - Request.MaxCandidateCount = 10; + Request.Limit = 10; Request.Query = UnqualifiedName; // FIXME(kbobyrev): Print symbol final scores to see the distribution. static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51747: [clangd] Show deprecation diagnostics
kadircet added a comment. In https://reviews.llvm.org/D51747#1233401, @sammccall wrote: > In https://reviews.llvm.org/D51747#1233377, @kadircet wrote: > > > In https://reviews.llvm.org/D51747#1233371, @sammccall wrote: > > > > > Sorry for all the back and forth on this patch, but I have to ask... > > > what's up with switching to lit tests? > > > > > > We've mostly started avoiding these as they're hard to maintain and debug > > > (not to mention write... crazy sed tricks!) > > > > > > They're mostly used in places where we need to test specifically the > > > protocol (smoke testing features, startup/shutdown, protocol edge > > > cases...) > > > > > > Sorry my bad actually forgot to mention. We set additional flags in > > ClangdServer::getCompileCommand and it is not used on the path of TestTU > > actually, instead it creates its own command line flags and uses them to > > invoke the compiler. So I changed to lit tests to make sure it gets invoked > > through getCompileCommand > > > Ah, that's unfortunate :-( > Sorry to be a pain, but can these be in ClangdTests instead? (Should really > be called ClangdServerTests). It's not as nice as TestTU, but... Np, not planning to land it before we figure out the issue with vim can't handling lots of diagnostics anyway. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51747: [clangd] Show deprecation diagnostics
kadircet updated this revision to Diff 165302. kadircet added a comment. This revision is now accepted and ready to land. - Turn back to unit tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 Files: clangd/ClangdServer.cpp unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -963,6 +963,63 @@ Field(&CodeCompletion::Name, "baz"))); } +TEST(ClangdCompilecommand, DiagnosticDeprecated) { + Annotations Test(R"cpp( +void foo() __attribute__(($foodeprecation[[deprecated]])); +class A { +public: + int x __attribute__(($xdeprecation[[deprecated]])); +}; +int main() { + $foo[[foo]](); + return A().$x[[x]]; +} + )cpp"); + auto SourcePath = testPath("source/foo.cpp"); + + MockFSProvider FS; + struct DiagConsumer : public DiagnosticsConsumer { +void onDiagnosticsReady(PathRef File, +std::vector Diagnostics) override { + std::lock_guard Lock(Mutex); + for(const Diag& D : Diagnostics) { +if(D.Category == "Deprecations") { + HadDeprecation = true; + if (D.Severity == DiagnosticsEngine::Error || + D.Severity == DiagnosticsEngine::Fatal) +HadDeprecationAsError = true; +} + } +} + +bool workedAsExpected() { + return !HadDeprecationAsError && HadDeprecation; +} + +void reset() { + HadDeprecation = false; + HadDeprecationAsError = false; +} + + private: +std::mutex Mutex; +bool HadDeprecationAsError = false; +bool HadDeprecation = false; + } DiagConsumer; + MockCompilationDatabase CDB; + CDB.ExtraClangFlags.push_back("-Wno-deprecated"); + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + runAddDocument(Server, SourcePath, Test.code()); + EXPECT_TRUE(DiagConsumer.workedAsExpected()); + DiagConsumer.reset(); + + CDB.ExtraClangFlags.push_back("-Werror"); + runAddDocument(Server, SourcePath, Test.code()); + EXPECT_TRUE(DiagConsumer.workedAsExpected()); +} + + } // namespace } // namespace clangd } // namespace clang Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -531,9 +531,15 @@ if (!C) // FIXME: Suppress diagnostics? Let the user know? C = CDB.getFallbackCommand(File); + // These flags are working for both gcc and clang-cl driver modes. // Inject the resource dir. // FIXME: Don't overwrite it if it's already there. C->CommandLine.push_back("-resource-dir=" + ResourceDir); + // Deprecations are often hidden for full-project build. They're useful in + // context. + C->CommandLine.push_back("-Wdeprecated"); + // Adding -Wdeprecated would trigger errors in projects what set -Werror. + C->CommandLine.push_back("-Wno-error=deprecated"); return std::move(*C); } Index: unittests/clangd/ClangdTests.cpp === --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -963,6 +963,63 @@ Field(&CodeCompletion::Name, "baz"))); } +TEST(ClangdCompilecommand, DiagnosticDeprecated) { + Annotations Test(R"cpp( +void foo() __attribute__(($foodeprecation[[deprecated]])); +class A { +public: + int x __attribute__(($xdeprecation[[deprecated]])); +}; +int main() { + $foo[[foo]](); + return A().$x[[x]]; +} + )cpp"); + auto SourcePath = testPath("source/foo.cpp"); + + MockFSProvider FS; + struct DiagConsumer : public DiagnosticsConsumer { +void onDiagnosticsReady(PathRef File, +std::vector Diagnostics) override { + std::lock_guard Lock(Mutex); + for(const Diag& D : Diagnostics) { +if(D.Category == "Deprecations") { + HadDeprecation = true; + if (D.Severity == DiagnosticsEngine::Error || + D.Severity == DiagnosticsEngine::Fatal) +HadDeprecationAsError = true; +} + } +} + +bool workedAsExpected() { + return !HadDeprecationAsError && HadDeprecation; +} + +void reset() { + HadDeprecation = false; + HadDeprecationAsError = false; +} + + private: +std::mutex Mutex; +bool HadDeprecationAsError = false; +bool HadDeprecation = false; + } DiagConsumer; + MockCompilationDatabase CDB; + CDB.ExtraClangFlags.push_back("-Wno-deprecated"); + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + runAddDocument(Server, SourcePath, Test.code()); + EXPECT_TRUE(DiagConsumer.workedAsExpected()); + DiagConsumer.reset(); + + CDB.ExtraClangFlags.push_back("-Werror"); + runAddDocument(Server, SourcePath, Test.code());
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev updated this revision to Diff 165298. kbobyrev added a comment. Remove artifacts from assertion messages. https://reviews.llvm.org/D51982 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/index/dex/Dex.cpp clang-tools-extra/clangd/index/dex/Dex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h clang-tools-extra/clangd/index/dex/PostingList.cpp clang-tools-extra/clangd/index/dex/PostingList.h clang-tools-extra/unittests/clangd/DexTests.cpp Index: clang-tools-extra/unittests/clangd/DexTests.cpp === --- clang-tools-extra/unittests/clangd/DexTests.cpp +++ clang-tools-extra/unittests/clangd/DexTests.cpp @@ -47,8 +47,8 @@ } TEST(DexIterators, DocumentIterator) { - const PostingList L = {4, 7, 8, 20, 42, 100}; - auto DocIterator = create(L); + const PostingList L({4, 7, 8, 20, 42, 100}); + auto DocIterator = L.iterator(); EXPECT_EQ(DocIterator->peek(), 4U); EXPECT_FALSE(DocIterator->reachedEnd()); @@ -70,28 +70,28 @@ } TEST(DexIterators, AndWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto AndEmpty = createAnd(create(L0)); + auto AndEmpty = createAnd(L0.iterator()); EXPECT_TRUE(AndEmpty->reachedEnd()); - auto AndWithEmpty = createAnd(create(L0), create(L1)); + auto AndWithEmpty = createAnd(L0.iterator(), L1.iterator()); EXPECT_TRUE(AndWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre()); } TEST(DexIterators, AndTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto And = createAnd(create(L1), create(L0)); + auto And = createAnd(L1.iterator(), L0.iterator()); EXPECT_FALSE(And->reachedEnd()); EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); - And = createAnd(create(L0), create(L1)); + And = createAnd(L0.iterator(), L1.iterator()); And->advanceTo(0); EXPECT_EQ(And->peek(), 0U); @@ -107,11 +107,11 @@ } TEST(DexIterators, AndThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto And = createAnd(create(L0), create(L1), create(L2)); + auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_EQ(And->peek(), 7U); And->advanceTo(300); EXPECT_EQ(And->peek(), 320U); @@ -121,24 +121,24 @@ } TEST(DexIterators, OrWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto OrEmpty = createOr(create(L0)); + auto OrEmpty = createOr(L0.iterator()); EXPECT_TRUE(OrEmpty->reachedEnd()); - auto OrWithEmpty = createOr(create(L0), create(L1)); + auto OrWithEmpty = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(OrWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*OrWithEmpty), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } TEST(DexIterators, OrTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1)); + auto Or = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -161,18 +161,18 @@ Or->advanceTo(9001); EXPECT_TRUE(Or->reachedEnd()); - Or = createOr(create(L0), create(L1)); + Or = createOr(L0.iterator(), L1.iterator()); EXPECT_THAT(consumeIDs(*Or), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } TEST(DexIterators, OrThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1), create(L2)); + auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -221,19 +221,19 @@ // |1, 5, 7, 9| |1, 5||0, 3, 5| // +-
[PATCH] D51982: [clangd] Introduce PostingList interface
kbobyrev updated this revision to Diff 165297. kbobyrev added a comment. Don't create abstractions for now. https://reviews.llvm.org/D51982 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/index/dex/Dex.cpp clang-tools-extra/clangd/index/dex/Dex.h clang-tools-extra/clangd/index/dex/Iterator.cpp clang-tools-extra/clangd/index/dex/Iterator.h clang-tools-extra/clangd/index/dex/PostingList.cpp clang-tools-extra/clangd/index/dex/PostingList.h clang-tools-extra/unittests/clangd/DexTests.cpp Index: clang-tools-extra/unittests/clangd/DexTests.cpp === --- clang-tools-extra/unittests/clangd/DexTests.cpp +++ clang-tools-extra/unittests/clangd/DexTests.cpp @@ -47,8 +47,8 @@ } TEST(DexIterators, DocumentIterator) { - const PostingList L = {4, 7, 8, 20, 42, 100}; - auto DocIterator = create(L); + const PostingList L({4, 7, 8, 20, 42, 100}); + auto DocIterator = L.iterator(); EXPECT_EQ(DocIterator->peek(), 4U); EXPECT_FALSE(DocIterator->reachedEnd()); @@ -70,28 +70,28 @@ } TEST(DexIterators, AndWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto AndEmpty = createAnd(create(L0)); + auto AndEmpty = createAnd(L0.iterator()); EXPECT_TRUE(AndEmpty->reachedEnd()); - auto AndWithEmpty = createAnd(create(L0), create(L1)); + auto AndWithEmpty = createAnd(L0.iterator(), L1.iterator()); EXPECT_TRUE(AndWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre()); } TEST(DexIterators, AndTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto And = createAnd(create(L1), create(L0)); + auto And = createAnd(L1.iterator(), L0.iterator()); EXPECT_FALSE(And->reachedEnd()); EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U)); - And = createAnd(create(L0), create(L1)); + And = createAnd(L0.iterator(), L1.iterator()); And->advanceTo(0); EXPECT_EQ(And->peek(), 0U); @@ -107,11 +107,11 @@ } TEST(DexIterators, AndThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto And = createAnd(create(L0), create(L1), create(L2)); + auto And = createAnd(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_EQ(And->peek(), 7U); And->advanceTo(300); EXPECT_EQ(And->peek(), 320U); @@ -121,24 +121,24 @@ } TEST(DexIterators, OrWithEmpty) { - const PostingList L0; - const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000}; + const PostingList L0({}); + const PostingList L1({0, 5, 7, 10, 42, 320, 9000}); - auto OrEmpty = createOr(create(L0)); + auto OrEmpty = createOr(L0.iterator()); EXPECT_TRUE(OrEmpty->reachedEnd()); - auto OrWithEmpty = createOr(create(L0), create(L1)); + auto OrWithEmpty = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(OrWithEmpty->reachedEnd()); EXPECT_THAT(consumeIDs(*OrWithEmpty), ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U)); } TEST(DexIterators, OrTwoLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1)); + auto Or = createOr(L0.iterator(), L1.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -161,18 +161,18 @@ Or->advanceTo(9001); EXPECT_TRUE(Or->reachedEnd()); - Or = createOr(create(L0), create(L1)); + Or = createOr(L0.iterator(), L1.iterator()); EXPECT_THAT(consumeIDs(*Or), ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U)); } TEST(DexIterators, OrThreeLists) { - const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000}; - const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000}; - const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000}; + const PostingList L0({0, 5, 7, 10, 42, 320, 9000}); + const PostingList L1({0, 4, 7, 10, 30, 60, 320, 9000}); + const PostingList L2({1, 4, 7, 11, 30, 60, 320, 9000}); - auto Or = createOr(create(L0), create(L1), create(L2)); + auto Or = createOr(L0.iterator(), L1.iterator(), L2.iterator()); EXPECT_FALSE(Or->reachedEnd()); EXPECT_EQ(Or->peek(), 0U); @@ -221,19 +221,19 @@ // |1, 5, 7, 9| |1, 5||0, 3, 5| // +--+
[PATCH] D52036: [Analyzer] Make plist tests less fragile
miyuki created this revision. miyuki added reviewers: NoQ, dcoughlin, george.karpenkov. Herald added subscribers: Szelethus, mikhail.ramalho, a.sidorin, szepet, eraman, xazax.hun. A recent change https://reviews.llvm.org/D50545 started using diff to compare plist output against reference output. There are two differences that are expected: clang version and absolute source file paths, these differences have been eliminated by adding several -I (ignore) regular expressions to the diff commands. Unfortunately such approach is still fragile: in our downstream version of Clang we use a slightly different format for the version string and it does not match the regular expression used in the tests. This patch adds a new analyzer option, -analyzer-output-no-fragile which disables output of malleable parts into plist files and adjusts the tests accordingly. https://reviews.llvm.org/D52036 Files: include/clang/Driver/CC1Options.td include/clang/StaticAnalyzer/Core/AnalyzerOptions.h lib/Frontend/CompilerInvocation.cpp lib/StaticAnalyzer/Core/PlistDiagnostics.cpp test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist test/Analysis/Inputs/expected-plists/edges-new.mm.plist test/Analysis/Inputs/expected-plists/generics.m.plist test/Analysis/Inputs/expected-plists/inline-plist.c.plist test/Analysis/Inputs/expected-plists/inline-unique-reports.c.plist test/Analysis/Inputs/expected-plists/method-call-path-notes.cpp.plist test/Analysis/Inputs/expected-plists/model-file.cpp.plist test/Analysis/Inputs/expected-plists/null-deref-path-notes.m.plist test/Analysis/Inputs/expected-plists/nullability-notes.m.plist test/Analysis/Inputs/expected-plists/objc-arc.m.plist test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist test/Analysis/Inputs/expected-plists/plist-output-alternate.m.plist test/Analysis/Inputs/expected-plists/plist-output.m.plist test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist test/Analysis/Inputs/expected-plists/unix-fns.c.plist test/Analysis/NewDelete-path-notes.cpp test/Analysis/conditional-path-notes.c test/Analysis/copypaste/Inputs/expected-plists/plist-diagnostics-notes-as-events.cpp.plist test/Analysis/copypaste/Inputs/expected-plists/plist-diagnostics.cpp.plist test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp test/Analysis/copypaste/plist-diagnostics.cpp test/Analysis/cxx-for-range.cpp test/Analysis/diagnostics/Inputs/expected-plists/deref-track-symbolic-region.c.plist test/Analysis/diagnostics/Inputs/expected-plists/report-issues-within-main-file.cpp.plist test/Analysis/diagnostics/Inputs/expected-plists/undef-value-caller.c.plist test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.c.plist test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.m.plist test/Analysis/diagnostics/deref-track-symbolic-region.c test/Analysis/diagnostics/report-issues-within-main-file.cpp test/Analysis/diagnostics/undef-value-caller.c test/Analysis/diagnostics/undef-value-param.c test/Analysis/diagnostics/undef-value-param.m test/Analysis/edges-new.mm test/Analysis/generics.m test/Analysis/inline-plist.c test/Analysis/inline-unique-reports.c test/Analysis/inlining/Inputs/expected-plists/eager-reclamation-path-notes.c.plist test/Analysis/inlining/Inputs/expected-plists/eager-reclamation-path-notes.cpp.plist test/Analysis/inlining/Inputs/expected-plists/path-notes.c.plist test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist test/Analysis/inlining/eager-reclamation-path-notes.c test/Analysis/inlining/eager-reclamation-path-notes.cpp test/Analysis/inlining/path-notes.c test/Analysis/inlining/path-notes.cpp test/Analysis/inlining/path-notes.m test/Analysis/method-call-path-notes.cpp test/Analysis/model-file.cpp test/Analysis/null-deref-path-notes.m test/Analysis/nullability-notes.m test/Analysis/objc-arc.m test/Analysis/plist-macros.cpp test/Analysis/plist-output-alternate.m test/Analysis/plist-output.m test/Analysis/retain-release-path-notes.m test/Analysis/unix-fns.c Index: test/Analysis/unix-fns.c === --- test/Analysis/unix-fns.c +++ test/Analysis/unix-fns.c @@ -1,5 +1,5 @@ -// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true -fblocks -verify -o %t.plist -// RUN: cat %t.plist | diff -u -w -I "/" -I ".:" -I "version" - %S/Inputs/expected-plists/unix-fns.c.plist +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyz
[PATCH] D51747: [clangd] Show deprecation diagnostics
sammccall added a comment. In https://reviews.llvm.org/D51747#1233377, @kadircet wrote: > In https://reviews.llvm.org/D51747#1233371, @sammccall wrote: > > > Sorry for all the back and forth on this patch, but I have to ask... what's > > up with switching to lit tests? > > > > We've mostly started avoiding these as they're hard to maintain and debug > > (not to mention write... crazy sed tricks!) > > > > They're mostly used in places where we need to test specifically the > > protocol (smoke testing features, startup/shutdown, protocol edge cases...) > > > Sorry my bad actually forgot to mention. We set additional flags in > ClangdServer::getCompileCommand and it is not used on the path of TestTU > actually, instead it creates its own command line flags and uses them to > invoke the compiler. So I changed to lit tests to make sure it gets invoked > through getCompileCommand Ah, that's unfortunate :-( Sorry to be a pain, but can these be in ClangdTests instead? (Should really be called ClangdServerTests). It's not as nice as TestTU, but... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52030: [clangd] Test benchmark if we can build it
kbobyrev added a comment. In https://reviews.llvm.org/D52030#1233395, @kbobyrev wrote: > In https://reviews.llvm.org/D52030#1233349, @sammccall wrote: > > > In https://reviews.llvm.org/D52030#1233323, @kbobyrev wrote: > > > > > Looks good, thanks! > > > > > > While we're here: I'm wondering whether we should also introduce very > > > basic test which would just run `clangd-indexer` since it doesn't depend > > > on benchmarks and would be run on all buildbots. > > > > > > Isn't that this test? > > > Yes, but this test checks both. As mentioned, buildbots don't build > benchmarks by default and hence this testcase is not actually run anywhere at > the moment. Sorry, that's wrong. It *is* run on every buildbot at the moment, but `IndexBenchmark` binary doesn't exist on any buildbot IIUC, so the second part of the test is not being run. If you add the dependency on `LLVM_BUILD_BENCHMARKS`, then the whole test would be disabled.+ Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52030: [clangd] Test benchmark if we can build it
kbobyrev added a subscriber: rnk. kbobyrev added a comment. In https://reviews.llvm.org/D52030#1233349, @sammccall wrote: > In https://reviews.llvm.org/D52030#1233323, @kbobyrev wrote: > > > Looks good, thanks! > > > > While we're here: I'm wondering whether we should also introduce very basic > > test which would just run `clangd-indexer` since it doesn't depend on > > benchmarks and would be run on all buildbots. > > > Isn't that this test? Yes, but this test checks both. As mentioned, buildbots don't build benchmarks by default and hence this testcase is not actually run anywhere at the moment. > > >> Also, I was thinking whether we should ask some buildbot owners to enable >> `LLVM_BUILD_BENCHMARKS`, but it probably makes sense when we have more >> benchmarks (ideally, across different LLVM parts). > > Hmm, I thought that was already the case. Isn't the benchmark stuff only > not-built on the windows bots? `LLVM_BUILD_BENCHMARKS` (which adds benchmarks to the list of default targets) is `OFF` on all platforms by default (although `LLVM_INCLUDE_BENCHMARKS` which produces build targets in the first place is enabled on Windows now, thanks to @rnk; it seems that the stage 2 bug we encountered was fixed). Hence, the benchmark library itself is built on all platforms (same as gtest library), but assuming that buildbots run `ninja`/`make` this only compiles default targets (which exclude `IndexBenchmark` due to `LLVM_BUILD_BENCHMARKS` being `OFF` by default). Hence, we should either ask buildbot owners to set `LLVM_BUILD_BENCHMARKS` to `ON` or don't exclude benchmarks from the list of default targets (which might result in some noise due to the full build time increase and is probably not what we want to do. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51847: Print correctly dependency paths on Windows
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC342139: Print correctly dependency paths on Windows (authored by xbolva00, committed by ). Repository: rC Clang https://reviews.llvm.org/D51847 Files: lib/Frontend/DependencyFile.cpp test/Frontend/dependency-gen-escaping.c test/Frontend/dependency-gen.c test/Modules/relative-dep-gen.cpp Index: test/Frontend/dependency-gen-escaping.c === --- test/Frontend/dependency-gen-escaping.c +++ test/Frontend/dependency-gen-escaping.c @@ -21,7 +21,7 @@ // Backslash followed by # or space should escape both characters, because // that's what GNU Make wants. GCC does the right thing with space, but not // #, so Clang does too. (There should be 3 backslashes before the #.) -// SEP2F: a\b\\#c\\\ d.h +// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h // With -fms-compatibility, Backslashes in #include are treated as path separators. // Backslashes are given in the emission for special characters, like 0x20 or 0x23. // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -4,19 +4,19 @@ // RUN: echo > %t.dir/a/b/x.h // RUN: cd %t.dir // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b | FileCheck -check-prefix=CHECK-ONE %s -// CHECK-ONE: {{ }}a/b{{[/\\]}}x.h +// CHECK-ONE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // PR8974 (-include flag) // RUN: %clang -MD -MF - %s -fsyntax-only -include a/b/x.h -DINCLUDE_FLAG_TEST | FileCheck -check-prefix=CHECK-TWO %s -// CHECK-TWO: {{ }}a/b/x.h +// CHECK-TWO: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // rdar://problem/9734352 (paths involving ".") // RUN: %clang -MD -MF - %s -fsyntax-only -I ./a/b | FileCheck -check-prefix=CHECK-THREE %s -// CHECK-THREE: {{ }}a/b{{[/\\]}}x.h +// CHECK-THREE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // RUN: %clang -MD -MF - %s -fsyntax-only -I .//./a/b/ | FileCheck -check-prefix=CHECK-FOUR %s -// CHECK-FOUR: {{ }}a/b{{[/\\]}}x.h +// CHECK-FOUR: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b/. | FileCheck -check-prefix=CHECK-FIVE %s -// CHECK-FIVE: {{ }}a/b/.{{[/\\]}}x.h +// CHECK-FIVE: {{ }}a{{[/\\]}}b{{[/\\]}}.{{[/\\]}}x.h // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h Index: test/Modules/relative-dep-gen.cpp === --- test/Modules/relative-dep-gen.cpp +++ test/Modules/relative-dep-gen.cpp @@ -30,9 +30,9 @@ #include "Inputs/relative-dep-gen-1.h" // CHECK-BUILD: mod.pcm: -// CHECK-BUILD: {{[ \t]}}Inputs/relative-dep-gen{{(-cwd)?}}.modulemap -// CHECK-BUILD: {{[ \t]}}Inputs/relative-dep-gen-1.h -// CHECK-BUILD: {{[ \t]}}Inputs/relative-dep-gen-2.h +// CHECK-BUILD: {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen{{(-cwd)?}}.modulemap +// CHECK-BUILD: {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-1.h +// CHECK-BUILD: {{[ \t]}}Inputs{{[/\\]}}relative-dep-gen-2.h // CHECK-USE: use.o: // CHECK-USE-DAG: {{[ \t]}}relative-dep-gen.cpp // CHECK-EXPLICIT-DAG: mod.pcm Index: lib/Frontend/DependencyFile.cpp === --- lib/Frontend/DependencyFile.cpp +++ lib/Frontend/DependencyFile.cpp @@ -386,28 +386,32 @@ /// for Windows file-naming info. static void PrintFilename(raw_ostream &OS, StringRef Filename, DependencyOutputFormat OutputFormat) { + // Convert filename to platform native path + llvm::SmallString<256> NativePath; + llvm::sys::path::native(Filename.str(), NativePath); + if (OutputFormat == DependencyOutputFormat::NMake) { // Add quotes if needed. These are the characters listed as "special" to // NMake, that are legal in a Windows filespec, and that could cause // misinterpretation of the dependency string. -if (Filename.find_first_of(" #${}^!") != StringRef::npos) - OS << '\"' << Filename << '\"'; +if (NativePath.find_first_of(" #${}^!") != StringRef::npos) + OS << '\"' << NativePath << '\"'; else - OS << Filename; + OS << NativePath; return; } assert(OutputFormat == DependencyOutputFormat::Make); - for (unsigned i = 0, e = Filename.size(); i != e; ++i) { -if (Filename[i] == '#') // Handle '#' the broken gcc way. + for (unsigned i = 0, e = NativePath.size(); i != e; ++i) { +if (NativePath[i] == '#') // Handle '#' the broken gcc way. OS << '\\'; -else if (Filename[i] == ' ') { // Handle space correctly. +else if (NativePath[i] == ' ') { // Handle space correctly. OS << '\\'; unsigned j = i; - while (j > 0 && Filename[--j] == '\\') + while (j > 0 && NativePath[--j] == '\\')
[PATCH] D52028: [clangd] Cleanup FuzzyFindRequest filtering limit semantics
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE342138: [clangd] Cleanup FuzzyFindRequest filtering limit semantics (authored by omtcyfz, committed by ). Changed prior to commit: https://reviews.llvm.org/D52028?vs=165283&id=165284#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52028 Files: clangd/CodeComplete.cpp clangd/FindSymbols.cpp clangd/index/Index.cpp clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/dex/Dex.cpp test/clangd/Inputs/requests.json unittests/clangd/DexTests.cpp unittests/clangd/IndexTests.cpp Index: test/clangd/Inputs/requests.json === --- test/clangd/Inputs/requests.json +++ test/clangd/Inputs/requests.json @@ -1,7 +1,7 @@ -[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]}, -{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, -{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, -{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, -{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, -{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]}, -{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}] +[{"Limit":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]}, +{"Limit":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, +{"Limit":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, +{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, +{"Limit":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]}, +{"Limit":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]}, +{"Limit":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}] Index: unittests/clangd/IndexTests.cpp === --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -78,10 +78,11 @@ auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab()); FuzzyFindRequest Req; Req.Query = "5"; - Req.MaxCandidateCount = 3; + Req.Limit = 3; bool Incomplete; auto Matches = match(*I, Req, &Incomplete); - EXPECT_EQ(Matches.size(), Req.MaxCandidateCount); + EXPECT_TRUE(Req.Limit); + EXPECT_EQ(Matches.size(), *Req.Limit); EXPECT_TRUE(Incomplete); } @@ -91,7 +92,7 @@ RefSlab()); FuzzyFindRequest Req; Req.Query = "lol"; - Req.MaxCandidateCount = 2; + Req.Limit = 2; EXPECT_THAT(match(*I, Req), UnorderedElementsAre("LaughingOutLoud", "LittleOldLady")); } Index: unittests/clangd/DexTests.cpp === --- unittests/clangd/DexTests.cpp +++ unittests/clangd/DexTests.cpp @@ -482,7 +482,7 @@ URISchemes); FuzzyFindRequest Req; Req.Query = "lol"; - Req.MaxCandidateCount = 2; + Req.Limit = 2; EXPECT_THAT(match(*I, Req), UnorderedElementsAre("LaughingOutLoud", "LittleOldLady")); } @@ -498,17 +498,19 @@ FuzzyFindRequest Req; Req.Query = "2"; Dex I(Symbols, URISchemes); + EXPECT_FALSE(Req.Limit); EXPECT_THAT(match(I, Req), ElementsAre("2", "2")); } TEST(DexTest, DexLimitedNumMatches) { auto I = Dex::build(generateNumSymbols(0, 100), URISchemes); FuzzyFindRequest Req; Req.Query = "5"; - Req.MaxCandidateCount = 3; + Req.Limit = 3; bool Incomplete; auto Matches = match(*I, Req, &Incomplete); - EXPECT_EQ(Matches.size(), Req.MaxCandidateCount); + EXPECT_TRUE(Req.Limit); + EXPECT_EQ(Matches.size(), *Req.Limit); EXPECT_TRUE(Incomplete); } @@ -518,7 +520,7 @@ URISchemes); FuzzyFindRequest Req; Req.Query = "lol"; - Req.MaxCandidateCount = 2; + Req.Limit = 2; EXPECT_THAT(match(*I, Req), UnorderedElementsAre("LaughingOutLoud", "LittleOldLady")); } @@ -596,7 +598,7 @@ FuzzyFindRequest Req; Req.Query = "abc"; // The best candidate can change depending on the proximity paths. - Req.MaxCandidateCount = 1; + Req.Limit = 1; // FuzzyFind request comes from the file which is far from the root: expect // CloseSymbol to come out. Index: clangd/C
r342139 - Print correctly dependency paths on Windows
Author: xbolva00 Date: Thu Sep 13 07:27:32 2018 New Revision: 342139 URL: http://llvm.org/viewvc/llvm-project?rev=342139&view=rev Log: Print correctly dependency paths on Windows Summary: Before: main.o: main.c ../include/lib\test.h After: main.o: main.c ../include/lib/test.h Fixes PR38877 Reviewers: zturner Subscribers: xbolva00, cfe-commits Differential Revision: https://reviews.llvm.org/D51847 Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp cfe/trunk/test/Frontend/dependency-gen-escaping.c cfe/trunk/test/Frontend/dependency-gen.c cfe/trunk/test/Modules/relative-dep-gen.cpp Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=342139&r1=342138&r2=342139&view=diff == --- cfe/trunk/lib/Frontend/DependencyFile.cpp (original) +++ cfe/trunk/lib/Frontend/DependencyFile.cpp Thu Sep 13 07:27:32 2018 @@ -386,28 +386,32 @@ bool DFGImpl::AddFilename(StringRef File /// for Windows file-naming info. static void PrintFilename(raw_ostream &OS, StringRef Filename, DependencyOutputFormat OutputFormat) { + // Convert filename to platform native path + llvm::SmallString<256> NativePath; + llvm::sys::path::native(Filename.str(), NativePath); + if (OutputFormat == DependencyOutputFormat::NMake) { // Add quotes if needed. These are the characters listed as "special" to // NMake, that are legal in a Windows filespec, and that could cause // misinterpretation of the dependency string. -if (Filename.find_first_of(" #${}^!") != StringRef::npos) - OS << '\"' << Filename << '\"'; +if (NativePath.find_first_of(" #${}^!") != StringRef::npos) + OS << '\"' << NativePath << '\"'; else - OS << Filename; + OS << NativePath; return; } assert(OutputFormat == DependencyOutputFormat::Make); - for (unsigned i = 0, e = Filename.size(); i != e; ++i) { -if (Filename[i] == '#') // Handle '#' the broken gcc way. + for (unsigned i = 0, e = NativePath.size(); i != e; ++i) { +if (NativePath[i] == '#') // Handle '#' the broken gcc way. OS << '\\'; -else if (Filename[i] == ' ') { // Handle space correctly. +else if (NativePath[i] == ' ') { // Handle space correctly. OS << '\\'; unsigned j = i; - while (j > 0 && Filename[--j] == '\\') + while (j > 0 && NativePath[--j] == '\\') OS << '\\'; -} else if (Filename[i] == '$') // $ is escaped by $$. +} else if (NativePath[i] == '$') // $ is escaped by $$. OS << '$'; -OS << Filename[i]; +OS << NativePath[i]; } } Modified: cfe/trunk/test/Frontend/dependency-gen-escaping.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/dependency-gen-escaping.c?rev=342139&r1=342138&r2=342139&view=diff == --- cfe/trunk/test/Frontend/dependency-gen-escaping.c (original) +++ cfe/trunk/test/Frontend/dependency-gen-escaping.c Thu Sep 13 07:27:32 2018 @@ -21,7 +21,7 @@ // Backslash followed by # or space should escape both characters, because // that's what GNU Make wants. GCC does the right thing with space, but not // #, so Clang does too. (There should be 3 backslashes before the #.) -// SEP2F: a\b\\#c\\\ d.h +// SEP2F: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h // With -fms-compatibility, Backslashes in #include are treated as path separators. // Backslashes are given in the emission for special characters, like 0x20 or 0x23. // SEP5C: a{{[/\\]}}b{{[/\\]}}\#c{{/|}}\ d.h Modified: cfe/trunk/test/Frontend/dependency-gen.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/dependency-gen.c?rev=342139&r1=342138&r2=342139&view=diff == --- cfe/trunk/test/Frontend/dependency-gen.c (original) +++ cfe/trunk/test/Frontend/dependency-gen.c Thu Sep 13 07:27:32 2018 @@ -4,19 +4,19 @@ // RUN: echo > %t.dir/a/b/x.h // RUN: cd %t.dir // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b | FileCheck -check-prefix=CHECK-ONE %s -// CHECK-ONE: {{ }}a/b{{[/\\]}}x.h +// CHECK-ONE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // PR8974 (-include flag) // RUN: %clang -MD -MF - %s -fsyntax-only -include a/b/x.h -DINCLUDE_FLAG_TEST | FileCheck -check-prefix=CHECK-TWO %s -// CHECK-TWO: {{ }}a/b/x.h +// CHECK-TWO: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // rdar://problem/9734352 (paths involving ".") // RUN: %clang -MD -MF - %s -fsyntax-only -I ./a/b | FileCheck -check-prefix=CHECK-THREE %s -// CHECK-THREE: {{ }}a/b{{[/\\]}}x.h +// CHECK-THREE: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // RUN: %clang -MD -MF - %s -fsyntax-only -I .//./a/b/ | FileCheck -check-prefix=CHECK-FOUR %s -// CHECK-FOUR: {{ }}a/b{{[/\\]}}x.h +// CHECK-FOUR: {{ }}a{{[/\\]}}b{{[/\\]}}x.h // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b/. | FileCheck -check-