r320017 - Ignore pointers to incomplete types when diagnosing misaligned addresses
Author: rogfer01 Date: Thu Dec 7 01:23:50 2017 New Revision: 320017 URL: http://llvm.org/viewvc/llvm-project?rev=320017&view=rev Log: Ignore pointers to incomplete types when diagnosing misaligned addresses This is a fix for PR35509 in which we crash because we attempt to compute the alignment of an incomplete type. Differential Revision: https://reviews.llvm.org/D40895 Modified: cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/SemaCXX/address-packed.cpp Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320017&r1=320016&r2=320017&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Dec 7 01:23:50 2017 @@ -12399,8 +12399,9 @@ void Sema::DiscardMisalignedMemberAddres MisalignedMember(Op)); if (MA != MisalignedMembers.end() && (T->isIntegerType() || - (T->isPointerType() && -Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment))) + (T->isPointerType() && (T->getPointeeType()->isIncompleteType() || + Context.getTypeAlignInChars( + T->getPointeeType()) <= MA->Alignment MisalignedMembers.erase(MA); } } Modified: cfe/trunk/test/SemaCXX/address-packed.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/address-packed.cpp?rev=320017&r1=320016&r2=320017&view=diff == --- cfe/trunk/test/SemaCXX/address-packed.cpp (original) +++ cfe/trunk/test/SemaCXX/address-packed.cpp Thu Dec 7 01:23:50 2017 @@ -112,3 +112,12 @@ void g1() { S s3; s3.get(); // expected-note {{in instantiation of member function 'S::get'}} } + +// PR35509 +typedef long L1; +struct Incomplete; +struct S2 { + L1 d; + Incomplete *e() const; +} __attribute__((packed)); +Incomplete *S2::e() const { return (Incomplete *)&d; } // no-warning ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40895: Ignore pointers to incomplete types when diagnosing misaligned addresses
This revision was automatically updated to reflect the committed changes. Closed by commit rL320017: Ignore pointers to incomplete types when diagnosing misaligned addresses (authored by rogfer01). Changed prior to commit: https://reviews.llvm.org/D40895?vs=125718&id=125901#toc Repository: rL LLVM https://reviews.llvm.org/D40895 Files: cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/SemaCXX/address-packed.cpp Index: cfe/trunk/lib/Sema/SemaChecking.cpp === --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -12399,8 +12399,9 @@ MisalignedMember(Op)); if (MA != MisalignedMembers.end() && (T->isIntegerType() || - (T->isPointerType() && -Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment))) + (T->isPointerType() && (T->getPointeeType()->isIncompleteType() || + Context.getTypeAlignInChars( + T->getPointeeType()) <= MA->Alignment MisalignedMembers.erase(MA); } } Index: cfe/trunk/test/SemaCXX/address-packed.cpp === --- cfe/trunk/test/SemaCXX/address-packed.cpp +++ cfe/trunk/test/SemaCXX/address-packed.cpp @@ -112,3 +112,12 @@ S s3; s3.get(); // expected-note {{in instantiation of member function 'S::get'}} } + +// PR35509 +typedef long L1; +struct Incomplete; +struct S2 { + L1 d; + Incomplete *e() const; +} __attribute__((packed)); +Incomplete *S2::e() const { return (Incomplete *)&d; } // no-warning Index: cfe/trunk/lib/Sema/SemaChecking.cpp === --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -12399,8 +12399,9 @@ MisalignedMember(Op)); if (MA != MisalignedMembers.end() && (T->isIntegerType() || - (T->isPointerType() && -Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment))) + (T->isPointerType() && (T->getPointeeType()->isIncompleteType() || + Context.getTypeAlignInChars( + T->getPointeeType()) <= MA->Alignment MisalignedMembers.erase(MA); } } Index: cfe/trunk/test/SemaCXX/address-packed.cpp === --- cfe/trunk/test/SemaCXX/address-packed.cpp +++ cfe/trunk/test/SemaCXX/address-packed.cpp @@ -112,3 +112,12 @@ S s3; s3.get(); // expected-note {{in instantiation of member function 'S::get'}} } + +// PR35509 +typedef long L1; +struct Incomplete; +struct S2 { + L1 d; + Incomplete *e() const; +} __attribute__((packed)); +Incomplete *S2::e() const { return (Incomplete *)&d; } // no-warning ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
ioeric added inline comments. Comment at: clangd/Symbol.h:23 + // The path of the source file where a symbol occurs. + std::string FilePath; + // The offset to the first character of the symbol from the beginning of the Is this relative or absolute? Comment at: clangd/Symbol.h:26 + // source file. + unsigned StartOffset; + // The offset to the last character of the symbol from the beginning of the 0-based or 1-based? Comment at: clangd/Symbol.h:39 + // The symbol identifier, using USR. + std::string Identifier; + // The qualified name of the symbol, e.g. Foo::bar. It might make sense to just call this `USR` to be more explicit. Comment at: clangd/Symbol.h:51 + // * For classes, the canonical location is where they are defined. + SymbolLocation CanonicalLocation; + // Information for code completion. For functions and classes, should we store both declaration and definition locations? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40486: [clangd] Implemented logging using Context
sammccall added inline comments. Comment at: clangd/JSONRPCDispatcher.cpp:23 +namespace { +static Key> TracerKey; +static Key IDKey; sammccall wrote: > RequestTracer? actually, RequestSpan I think - "tracer" is pretty confusing at global scope Comment at: clangd/JSONRPCDispatcher.cpp:70 + + SPAN_ATTACH(*Ctx->getExisting(TracerKey), "Reply", Result); + Ctx->getPtr(OutKey)->writeMessage(json::obj{ Depending on a span tracer existing seems needlessly brittle. What about if (auto *Tracer = Ctx->get(TracerKey)) SPAN_ATTACH(*Tracer, ...) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40489: [clangd] Changed tracing interfaces
sammccall added inline comments. Comment at: clangd/Trace.cpp:138 return; - if (!Args) -Args = llvm::make_unique(); - T->event(Ctx, "E", - Args ? json::obj{{"args", std::move(*Args)}} : json::obj{}); + assert(Args && "Args can't be null at this point"); + T->end_event(Ctx, Name, std::move(*Args)); why not? Comment at: clangd/Trace.h:39 + /// Called when event with \p Name starts. + virtual void begin_event(const ContextData &Ctx, llvm::StringRef Name) = 0; + /// Called when event with \p Name ends. just call this begin? Otherwise style is `beginEvent` I think Comment at: clangd/Trace.h:41 + /// Called when event with \p Name ends. + virtual void end_event(const ContextData &Ctx, llvm::StringRef Name, + json::obj &&Args) = 0; How is identity represented between begin/end event? via Name doesn't seem robust, so why the need to pass it twice? Does providing different contexts for start/end make sense? It might be cleaner/more flexible to have std::function begin() and call the return value to signal end. This seems likely to be pretty easy for different providers to implement, and is easy to use from Span. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40888: [ARM] ACLE parallel arithmetic and DSP style multiplications
This revision was automatically updated to reflect the committed changes. Closed by commit rC320019: [ARM] ACLE parallel arithmetic and DSP style multiplications (authored by SjoerdMeijer). Repository: rC Clang https://reviews.llvm.org/D40888 Files: include/clang/Basic/BuiltinsARM.def lib/Sema/SemaChecking.cpp test/Sema/builtins-arm.c Index: test/Sema/builtins-arm.c === --- test/Sema/builtins-arm.c +++ test/Sema/builtins-arm.c @@ -2,6 +2,8 @@ // RUN: %clang_cc1 -triple armv7 -target-abi apcs-gnu \ // RUN: -fsyntax-only -verify %s +#include + void f(void *a, void *b) { __clear_cache(); // expected-error {{too few arguments to function call, expected 2, have 0}} // expected-note {{'__clear_cache' is a builtin with type 'void (void *, void *)}} __clear_cache(a); // expected-error {{too few arguments to function call, expected 2, have 1}} @@ -136,3 +138,185 @@ __builtin_arm_mrrc2(15, a, 0); // expected-error {{argument to '__builtin_arm_mrrc2' must be a constant integer}} __builtin_arm_mrrc2(15, 0, a); // expected-error {{argument to '__builtin_arm_mrrc2' must be a constant integer}} } + +void test_9_3_multiplications(int a, int b) { + int r; + r = __builtin_arm_smulbb(a, b); + r = __builtin_arm_smulbb(1, -9); + + r = __builtin_arm_smulbt(a, b); + r = __builtin_arm_smulbt(0, b); + + r = __builtin_arm_smultb(a, b); + r = __builtin_arm_smultb(5, b); + + r = __builtin_arm_smultt(a, b); + r = __builtin_arm_smultt(a, -1); + + r = __builtin_arm_smulwb(a, b); + r = __builtin_arm_smulwb(1, 2); + + r = __builtin_arm_smulwt(a, b); + r = __builtin_arm_smulwt(-1, -2); + r = __builtin_arm_smulwt(-1.0f, -2); +} + +void test_9_4_1_width_specified_saturation(int a, int b) { + unsigned u; + int s; + + s = __builtin_arm_ssat(8, 2); + s = __builtin_arm_ssat(a, 1); + s = __builtin_arm_ssat(a, 32); + s = __builtin_arm_ssat(a, 0); // expected-error {{argument should be a value from 1 to 32}} + s = __builtin_arm_ssat(a, 33); // expected-error {{argument should be a value from 1 to 32}} + s = __builtin_arm_ssat(a, b); // expected-error {{argument to '__builtin_arm_ssat' must be a constant integer}} + + u = __builtin_arm_usat(8, 2); + u = __builtin_arm_usat(a, 0); + u = __builtin_arm_usat(a, 31); + u = __builtin_arm_usat(a, 32); // expected-error {{argument should be a value from 0 to 31}} + u = __builtin_arm_usat(a, b); // expected-error {{argument to '__builtin_arm_usat' must be a constant integer}} +} + +void test_9_4_2_saturating_addition_subtraction(int a, int b) { + int s; + s = __builtin_arm_qadd(a, b); + s = __builtin_arm_qadd(-1, 0); + + s = __builtin_arm_qsub(a, b); + s = __builtin_arm_qsub(0, -1); + + s = __builtin_arm_qdbl(a); +} + +void test_9_4_3_accumulating_multiplications(int a, int b, int c) { + int s; + + s = __builtin_arm_smlabb(a, b, c); + s = __builtin_arm_smlabb(1, b, c); + s = __builtin_arm_smlabb(a, 2, c); + s = __builtin_arm_smlabb(a, b, -3); + + s = __builtin_arm_smlabt(a, b, c); + s = __builtin_arm_smlabt(1, b, c); + s = __builtin_arm_smlabt(a, 2, c); + s = __builtin_arm_smlabt(a, b, -3); + + s = __builtin_arm_smlatb(a, b, c); + s = __builtin_arm_smlatt(1, b, c); + s = __builtin_arm_smlawb(a, 2, c); + s = __builtin_arm_smlawt(a, b, -3); +} + +void test_9_5_4_parallel_16bit_saturation(int16x2_t a) { + unsigned u; + int s; + + s = __builtin_arm_ssat16(a, 1); + s = __builtin_arm_ssat16(a, 16); + s = __builtin_arm_ssat16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + s = __builtin_arm_ssat16(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + u = __builtin_arm_usat16(a, 0); + u = __builtin_arm_usat16(a, 15); + u = __builtin_arm_usat16(a, 16); // expected-error {{argument should be a value from 0 to 15}} +} + +void test_9_5_5_packing_and_unpacking(int16x2_t a, int8x4_t b, uint16x2_t c, uint8x4_t d) { + int16x2_t x; + uint16x2_t y; + + x = __builtin_arm_sxtab16(a, b); + x = __builtin_arm_sxtab16(1, -1); + x = __builtin_arm_sxtb16(b); + x = __builtin_arm_sxtb16(-b); + + y = __builtin_arm_uxtab16(c, d); + y = __builtin_arm_uxtab16(-1, -2); + y = __builtin_arm_uxtb16(d); + y = __builtin_arm_uxtb16(-1); +} + +uint8x4_t +test_9_5_6_parallel_selection(uint8x4_t a, uint8x4_t b) { + return __builtin_arm_sel(a, b); +} + +void test_9_5_7_parallel_8bit_addition_substraction(int8x4_t a, int8x4_t b, +uint8x4_t c, uint8x4_t d) { + int8x4_t s; + uint8x4_t u; + + s = __builtin_arm_qadd8(a, b); + s = __builtin_arm_qsub8(a, b); + s = __builtin_arm_sadd8(a, b); + s = __builtin_arm_shadd8(a, b); + s = __builtin_arm_shsub8(a, b); + s = __builtin_arm_ssub8(a, b); + + u = __builtin_arm_uadd8(c, d); + u = __builtin_arm_uhadd8(c, d); + u = __builtin_arm_uhsub8(c, d); + u = __builtin_arm_uqadd8(c, d); + u = __builtin_arm_uqsub8(c, d); + u = __builtin_arm_usub
[PATCH] D40888: [ARM] ACLE parallel arithmetic and DSP style multiplications
This revision was automatically updated to reflect the committed changes. Closed by commit rL320019: [ARM] ACLE parallel arithmetic and DSP style multiplications (authored by SjoerdMeijer). Changed prior to commit: https://reviews.llvm.org/D40888?vs=125705&id=125909#toc Repository: rL LLVM https://reviews.llvm.org/D40888 Files: cfe/trunk/include/clang/Basic/BuiltinsARM.def cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/builtins-arm.c Index: cfe/trunk/include/clang/Basic/BuiltinsARM.def === --- cfe/trunk/include/clang/Basic/BuiltinsARM.def +++ cfe/trunk/include/clang/Basic/BuiltinsARM.def @@ -36,6 +36,7 @@ // Saturating arithmetic BUILTIN(__builtin_arm_qadd, "iii", "nc") BUILTIN(__builtin_arm_qsub, "iii", "nc") +BUILTIN(__builtin_arm_qdbl, "ii", "nc") BUILTIN(__builtin_arm_ssat, "iiUi", "nc") BUILTIN(__builtin_arm_usat, "UiiUi", "nc") Index: cfe/trunk/test/Sema/builtins-arm.c === --- cfe/trunk/test/Sema/builtins-arm.c +++ cfe/trunk/test/Sema/builtins-arm.c @@ -2,6 +2,8 @@ // RUN: %clang_cc1 -triple armv7 -target-abi apcs-gnu \ // RUN: -fsyntax-only -verify %s +#include + void f(void *a, void *b) { __clear_cache(); // expected-error {{too few arguments to function call, expected 2, have 0}} // expected-note {{'__clear_cache' is a builtin with type 'void (void *, void *)}} __clear_cache(a); // expected-error {{too few arguments to function call, expected 2, have 1}} @@ -136,3 +138,185 @@ __builtin_arm_mrrc2(15, a, 0); // expected-error {{argument to '__builtin_arm_mrrc2' must be a constant integer}} __builtin_arm_mrrc2(15, 0, a); // expected-error {{argument to '__builtin_arm_mrrc2' must be a constant integer}} } + +void test_9_3_multiplications(int a, int b) { + int r; + r = __builtin_arm_smulbb(a, b); + r = __builtin_arm_smulbb(1, -9); + + r = __builtin_arm_smulbt(a, b); + r = __builtin_arm_smulbt(0, b); + + r = __builtin_arm_smultb(a, b); + r = __builtin_arm_smultb(5, b); + + r = __builtin_arm_smultt(a, b); + r = __builtin_arm_smultt(a, -1); + + r = __builtin_arm_smulwb(a, b); + r = __builtin_arm_smulwb(1, 2); + + r = __builtin_arm_smulwt(a, b); + r = __builtin_arm_smulwt(-1, -2); + r = __builtin_arm_smulwt(-1.0f, -2); +} + +void test_9_4_1_width_specified_saturation(int a, int b) { + unsigned u; + int s; + + s = __builtin_arm_ssat(8, 2); + s = __builtin_arm_ssat(a, 1); + s = __builtin_arm_ssat(a, 32); + s = __builtin_arm_ssat(a, 0); // expected-error {{argument should be a value from 1 to 32}} + s = __builtin_arm_ssat(a, 33); // expected-error {{argument should be a value from 1 to 32}} + s = __builtin_arm_ssat(a, b); // expected-error {{argument to '__builtin_arm_ssat' must be a constant integer}} + + u = __builtin_arm_usat(8, 2); + u = __builtin_arm_usat(a, 0); + u = __builtin_arm_usat(a, 31); + u = __builtin_arm_usat(a, 32); // expected-error {{argument should be a value from 0 to 31}} + u = __builtin_arm_usat(a, b); // expected-error {{argument to '__builtin_arm_usat' must be a constant integer}} +} + +void test_9_4_2_saturating_addition_subtraction(int a, int b) { + int s; + s = __builtin_arm_qadd(a, b); + s = __builtin_arm_qadd(-1, 0); + + s = __builtin_arm_qsub(a, b); + s = __builtin_arm_qsub(0, -1); + + s = __builtin_arm_qdbl(a); +} + +void test_9_4_3_accumulating_multiplications(int a, int b, int c) { + int s; + + s = __builtin_arm_smlabb(a, b, c); + s = __builtin_arm_smlabb(1, b, c); + s = __builtin_arm_smlabb(a, 2, c); + s = __builtin_arm_smlabb(a, b, -3); + + s = __builtin_arm_smlabt(a, b, c); + s = __builtin_arm_smlabt(1, b, c); + s = __builtin_arm_smlabt(a, 2, c); + s = __builtin_arm_smlabt(a, b, -3); + + s = __builtin_arm_smlatb(a, b, c); + s = __builtin_arm_smlatt(1, b, c); + s = __builtin_arm_smlawb(a, 2, c); + s = __builtin_arm_smlawt(a, b, -3); +} + +void test_9_5_4_parallel_16bit_saturation(int16x2_t a) { + unsigned u; + int s; + + s = __builtin_arm_ssat16(a, 1); + s = __builtin_arm_ssat16(a, 16); + s = __builtin_arm_ssat16(a, 0); // expected-error {{argument should be a value from 1 to 16}} + s = __builtin_arm_ssat16(a, 17); // expected-error {{argument should be a value from 1 to 16}} + + u = __builtin_arm_usat16(a, 0); + u = __builtin_arm_usat16(a, 15); + u = __builtin_arm_usat16(a, 16); // expected-error {{argument should be a value from 0 to 15}} +} + +void test_9_5_5_packing_and_unpacking(int16x2_t a, int8x4_t b, uint16x2_t c, uint8x4_t d) { + int16x2_t x; + uint16x2_t y; + + x = __builtin_arm_sxtab16(a, b); + x = __builtin_arm_sxtab16(1, -1); + x = __builtin_arm_sxtb16(b); + x = __builtin_arm_sxtb16(-b); + + y = __builtin_arm_uxtab16(c, d); + y = __builtin_arm_uxtab16(-1, -2); + y = __builtin_arm_uxtb16(d); + y = __builtin_arm_uxtb16(-1); +} + +uint8x4_t +test_9_5_6_parallel_selection(uint8x4_t a, uint8x4_t b) { + return __buil
r320019 - [ARM] ACLE parallel arithmetic and DSP style multiplications
Author: sjoerdmeijer Date: Thu Dec 7 01:54:39 2017 New Revision: 320019 URL: http://llvm.org/viewvc/llvm-project?rev=320019&view=rev Log: [ARM] ACLE parallel arithmetic and DSP style multiplications This is a follow up of r302131, in which we forgot to add SemaChecking tests. Adding these tests revealed two problems which have been fixed: - added missing intrinsic __qdbl, - properly range checking ssat16 and usat16. Differential Revision: https://reviews.llvm.org/D40888 Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/builtins-arm.c Modified: cfe/trunk/include/clang/Basic/BuiltinsARM.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsARM.def?rev=320019&r1=320018&r2=320019&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsARM.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsARM.def Thu Dec 7 01:54:39 2017 @@ -36,6 +36,7 @@ BUILTIN(__builtin_arm_smulwt, "iii", "nc // Saturating arithmetic BUILTIN(__builtin_arm_qadd, "iii", "nc") BUILTIN(__builtin_arm_qsub, "iii", "nc") +BUILTIN(__builtin_arm_qdbl, "ii", "nc") BUILTIN(__builtin_arm_ssat, "iiUi", "nc") BUILTIN(__builtin_arm_usat, "UiiUi", "nc") Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320019&r1=320018&r2=320019&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Dec 7 01:54:39 2017 @@ -1554,21 +1554,26 @@ bool Sema::CheckARMBuiltinFunctionCall(u // For intrinsics which take an immediate value as part of the instruction, // range check them here. - unsigned i = 0, l = 0, u = 0; + // FIXME: VFP Intrinsics should error if VFP not present. switch (BuiltinID) { default: return false; - case ARM::BI__builtin_arm_ssat: i = 1; l = 1; u = 31; break; - case ARM::BI__builtin_arm_usat: i = 1; u = 31; break; + case ARM::BI__builtin_arm_ssat: +return SemaBuiltinConstantArgRange(TheCall, 1, 1, 32); + case ARM::BI__builtin_arm_usat: +return SemaBuiltinConstantArgRange(TheCall, 1, 0, 31); + case ARM::BI__builtin_arm_ssat16: +return SemaBuiltinConstantArgRange(TheCall, 1, 1, 16); + case ARM::BI__builtin_arm_usat16: +return SemaBuiltinConstantArgRange(TheCall, 1, 0, 15); case ARM::BI__builtin_arm_vcvtr_f: - case ARM::BI__builtin_arm_vcvtr_d: i = 1; u = 1; break; + case ARM::BI__builtin_arm_vcvtr_d: +return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1); case ARM::BI__builtin_arm_dmb: case ARM::BI__builtin_arm_dsb: case ARM::BI__builtin_arm_isb: - case ARM::BI__builtin_arm_dbg: l = 0; u = 15; break; + case ARM::BI__builtin_arm_dbg: +return SemaBuiltinConstantArgRange(TheCall, 0, 0, 15); } - - // FIXME: VFP Intrinsics should error if VFP not present. - return SemaBuiltinConstantArgRange(TheCall, i, l, u + l); } bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, Modified: cfe/trunk/test/Sema/builtins-arm.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins-arm.c?rev=320019&r1=320018&r2=320019&view=diff == --- cfe/trunk/test/Sema/builtins-arm.c (original) +++ cfe/trunk/test/Sema/builtins-arm.c Thu Dec 7 01:54:39 2017 @@ -2,6 +2,8 @@ // RUN: %clang_cc1 -triple armv7 -target-abi apcs-gnu \ // RUN: -fsyntax-only -verify %s +#include + void f(void *a, void *b) { __clear_cache(); // expected-error {{too few arguments to function call, expected 2, have 0}} // expected-note {{'__clear_cache' is a builtin with type 'void (void *, void *)}} __clear_cache(a); // expected-error {{too few arguments to function call, expected 2, have 1}} @@ -136,3 +138,185 @@ void test6(int a, int b, int c) { __builtin_arm_mrrc2(15, a, 0); // expected-error {{argument to '__builtin_arm_mrrc2' must be a constant integer}} __builtin_arm_mrrc2(15, 0, a); // expected-error {{argument to '__builtin_arm_mrrc2' must be a constant integer}} } + +void test_9_3_multiplications(int a, int b) { + int r; + r = __builtin_arm_smulbb(a, b); + r = __builtin_arm_smulbb(1, -9); + + r = __builtin_arm_smulbt(a, b); + r = __builtin_arm_smulbt(0, b); + + r = __builtin_arm_smultb(a, b); + r = __builtin_arm_smultb(5, b); + + r = __builtin_arm_smultt(a, b); + r = __builtin_arm_smultt(a, -1); + + r = __builtin_arm_smulwb(a, b); + r = __builtin_arm_smulwb(1, 2); + + r = __builtin_arm_smulwt(a, b); + r = __builtin_arm_smulwt(-1, -2); + r = __builtin_arm_smulwt(-1.0f, -2); +} + +void test_9_4_1_width_specified_saturation(int a, int b) { + unsigned u; + int s; + + s = __builtin_arm_ssat(8, 2); + s = __builtin_arm_ssat(a, 1); + s = __builtin_arm_ssat(a, 32); + s = __builtin_arm_ssat(a, 0); // exp
[PATCH] D40884: [Index] Add setPreprocessor member to IndexDataConsumer.
ioeric updated this revision to Diff 125913. ioeric marked an inline comment as done. ioeric added a comment. - Removed a redundant #include Repository: rC Clang https://reviews.llvm.org/D40884 Files: include/clang/Index/IndexDataConsumer.h lib/Index/IndexingAction.cpp tools/libclang/CXIndexDataConsumer.h Index: tools/libclang/CXIndexDataConsumer.h === --- tools/libclang/CXIndexDataConsumer.h +++ tools/libclang/CXIndexDataConsumer.h @@ -342,7 +342,7 @@ CXTranslationUnit getCXTU() const { return CXTU; } void setASTContext(ASTContext &ctx); - void setPreprocessor(std::shared_ptr PP); + void setPreprocessor(std::shared_ptr PP) override; bool shouldSuppressRefs() const { return IndexOptions & CXIndexOpt_SuppressRedundantRefs; Index: lib/Index/IndexingAction.cpp === --- lib/Index/IndexingAction.cpp +++ lib/Index/IndexingAction.cpp @@ -8,10 +8,11 @@ //===--===// #include "clang/Index/IndexingAction.h" -#include "clang/Index/IndexDataConsumer.h" #include "IndexingContext.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/MultiplexConsumer.h" +#include "clang/Index/IndexDataConsumer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" @@ -42,16 +43,18 @@ namespace { class IndexASTConsumer : public ASTConsumer { + std::shared_ptr PP; IndexingContext &IndexCtx; public: - IndexASTConsumer(IndexingContext &IndexCtx) -: IndexCtx(IndexCtx) {} + IndexASTConsumer(std::shared_ptr PP, IndexingContext &IndexCtx) + : PP(std::move(PP)), IndexCtx(IndexCtx) {} protected: void Initialize(ASTContext &Context) override { IndexCtx.setASTContext(Context); IndexCtx.getDataConsumer().initialize(Context); +IndexCtx.getDataConsumer().setPreprocessor(PP); } bool HandleTopLevelDecl(DeclGroupRef DG) override { @@ -80,8 +83,10 @@ : DataConsumer(std::move(dataConsumer)), IndexCtx(Opts, *DataConsumer) {} - std::unique_ptr createIndexASTConsumer() { -return llvm::make_unique(IndexCtx); + std::unique_ptr + createIndexASTConsumer(CompilerInstance &CI) { +return llvm::make_unique(CI.getPreprocessorPtr(), + IndexCtx); } void finish() { @@ -98,7 +103,7 @@ protected: std::unique_ptr CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override { -return createIndexASTConsumer(); +return createIndexASTConsumer(CI); } void EndSourceFileAction() override { @@ -142,7 +147,7 @@ std::vector> Consumers; Consumers.push_back(std::move(OtherConsumer)); - Consumers.push_back(createIndexASTConsumer()); + Consumers.push_back(createIndexASTConsumer(CI)); return llvm::make_unique(std::move(Consumers)); } @@ -173,6 +178,7 @@ IndexingContext IndexCtx(Opts, *DataConsumer); IndexCtx.setASTContext(Unit.getASTContext()); DataConsumer->initialize(Unit.getASTContext()); + DataConsumer->setPreprocessor(Unit.getPreprocessorPtr()); indexTranslationUnit(Unit, IndexCtx); DataConsumer->finish(); } @@ -198,7 +204,7 @@ IndexCtx.setASTContext(Ctx); DataConsumer->initialize(Ctx); - for (const Decl *D :Reader.getModuleFileLevelDecls(Mod)) { + for (const Decl *D : Reader.getModuleFileLevelDecls(Mod)) { IndexCtx.indexTopLevelDecl(D); } DataConsumer->finish(); Index: include/clang/Index/IndexDataConsumer.h === --- include/clang/Index/IndexDataConsumer.h +++ include/clang/Index/IndexDataConsumer.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_INDEX_INDEXDATACONSUMER_H #include "clang/Index/IndexSymbol.h" +#include "clang/Lex/Preprocessor.h" namespace clang { class ASTContext; @@ -36,6 +37,8 @@ virtual void initialize(ASTContext &Ctx) {} + virtual void setPreprocessor(std::shared_ptr PP) {} + /// \returns true to continue indexing, or false to abort. virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles, ArrayRef Relations, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
sammccall added a comment. Thanks for putting this together! Have a bit of a braindump here, happy to discuss further either here or offline. Comment at: clangd/Symbol.h:1 +//===--- Symbol.h ---*- C++-*-===// +// I think that: - there's other places in clangd that deal with symbols too, this is specifically for indexing - the index interface belongs alongside Symbol I'd suggest calling this Index.h Comment at: clangd/Symbol.h:1 +//===--- Symbol.h ---*- C++-*-===// +// sammccall wrote: > I think that: > - there's other places in clangd that deal with symbols too, this is > specifically for indexing > - the index interface belongs alongside Symbol > I'd suggest calling this Index.h I don't think having `Symbol`s be completely self-contained objects and passing them around in standard containers like `set`s will prove to be ideal. It means they can't share storage for e.g. location filename, that it's hard for us to arena-allocate them, etc. I think we could use the concept of a set of symbols which share a lifetime. An initial version might just be class SymbolSlab { public: using iterator = DenseSet::iterator; iterator begin(); iterator end(); private: DenseSet Symbols; } But it's easy to add `StringPool` etc there. Then this is the natural unit of granularity of large sets of symbols: a dynamic index that deals with one file at a time would operate on (Filename, SymbolSlab) pairs. SymbolCollector would return a SymbolSlab, etc. Then indexes can be built on top of this using non-owning pointers. Comment at: clangd/Symbol.h:23 + // The path of the source file where a symbol occurs. + std::string FilePath; + // The offset to the first character of the symbol from the beginning of the ioeric wrote: > Is this relative or absolute? Having every symbol own a copy of the filepath seems wasteful. It seems likely that the index will have per-file information too, so this representation should likely be a key to that. Hash of the filepath might work? Comment at: clangd/Symbol.h:32 + +struct CodeCompletionInfo { + // FIXME: add fields here. Let's not add this until we know what's in it. There's gong to be an overlap between information needed for CC and other use cases, so this structure might not help the user navigate. Comment at: clangd/Symbol.h:37 +// The class presents a C++ symbol, e.g. class, function. +struct Symbol { + // The symbol identifier, using USR. hokein wrote: > malaperle wrote: > > I think it would be nice to have methods as an interface to get this data > > instead of storing them directly. So that an index-on-disk could go fetch > > the data. Especially the occurrences which can take a lot of memory (I'm > > working on a branch that does that). But perhaps defining that interface is > > not within the scope of this patch and could be better discussed in D40548 ? > I agree. We can't load all the symbol occurrences into the memory since they > are too large. We need to design interface for the symbol occurrences. > > We could discuss the interface here, but CodeCompletion is the main thing > which this patch focuses on. > We can't load all the symbol occurrences into the memory since they are too > large I've heard this often, but never backed up by data :-) Naively an array of references for a symbol could be doc ID + offset + length, let's say 16 bytes. If a source file consisted entirely of references to 1-character symbols separated by punctuation (1 reference per 2 bytes) then the total size of these references would be 8x the size of the source file - in practice much less. That's not very big. (Maybe there are edge cases with macros/templates, but we can keep them under control) Comment at: clangd/Symbol.h:39 + // The symbol identifier, using USR. + std::string Identifier; + // The qualified name of the symbol, e.g. Foo::bar. ioeric wrote: > It might make sense to just call this `USR` to be more explicit. USRs are large. Can we use a fixed-size hash? Comment at: clangd/Symbol.h:55 + + bool operator<(const Symbol& S) const { +return Identifier < S.Identifier; I'd suggest == and a hash function instead, unless we think this ordering is particularly meaningful? Comment at: clangd/Symbol.h:68 +// changed. +class SymbolCollector : public index::IndexDataConsumer { +public: Please pull this into a separate file. Someone providing e.g. symbols from a YAML file shouldn't need to pull in AST stuff. Mabye `IndexFromAST`, which would sort nicely next to `Index`? Comment at: clangd/Symbol.h:72 + + const std::set &getSymbol
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
krasimir updated this revision to Diff 125915. krasimir added a comment. - Added unsafe canonical delimiter update handling Repository: rC Clang https://reviews.llvm.org/D40909 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestRawStrings.cpp Index: unittests/Format/FormatTestRawStrings.cpp === --- unittests/Format/FormatTestRawStrings.cpp +++ unittests/Format/FormatTestRawStrings.cpp @@ -9,6 +9,8 @@ #include "clang/Format/Format.h" +#include + #include "../Tooling/ReplacementTest.h" #include "FormatTestUtils.h" @@ -19,6 +21,12 @@ #define DEBUG_TYPE "format-test" +#define DBG(A) \ + DEBUG({ \ +llvm::dbgs() << __LINE__ << ":" << __func__ << ":" << #A << " = " << A \ + << "\n"; \ + }); + using clang::tooling::ReplacementTest; using clang::tooling::toReplacements; @@ -65,23 +73,40 @@ FormatStyle getRawStringPbStyleWithColumns(unsigned ColumnLimit) { FormatStyle Style = getLLVMStyle(); Style.ColumnLimit = ColumnLimit; -Style.RawStringFormats = {{/*Delimiter=*/"pb", - /*Kind=*/FormatStyle::LK_TextProto, - /*BasedOnStyle=*/"google"}}; +Style.AdditionalLanguageStyles[FormatStyle::LK_TextProto] = +std::make_shared( +getGoogleStyle(FormatStyle::LK_TextProto)); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_TextProto, + /*Delimiters=*/{"pb"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } - FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringLLVMCppStyleBasedOn(std::string Name) { FormatStyle Style = getLLVMStyle(); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; +DBG(BasedOnStyle.PointerAlignment); return Style; } - FormatStyle getRawStringGoogleCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringGoogleCppStyleBasedOn(std::string Name) { FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } @@ -96,17 +121,19 @@ // llvm style puts '*' on the right. // google style puts '*' on the left. - // Use the llvm style if the raw string style has no BasedOnStyle. - expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", -format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", - getRawStringLLVMCppStyleBasedOn(""))); - - // Use the google style if the raw string style has BasedOnStyle=google. + // Use llvm style outside and the google style inside if the raw string style + // is based on google. expect_eq(R"test(int *i = R"cpp(int* p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", getRawStringLLVMCppStyleBasedOn("google"))); - // Use the llvm style if the raw string style has no BasedOnStyle=llvm. + // Use llvm style if the raw string style has no BasedOnStyle. + expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", +format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", + getRawStringLLVMCppStyleBasedOn(""))); + + // Use google style outside and the llvm style inside if the raw string style + // is based on llvm. expect_eq(R"test(int* i = R"cpp(int *p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = null
r320030 - [Index] Add setPreprocessor member to IndexDataConsumer.
Author: ioeric Date: Thu Dec 7 03:04:24 2017 New Revision: 320030 URL: http://llvm.org/viewvc/llvm-project?rev=320030&view=rev Log: [Index] Add setPreprocessor member to IndexDataConsumer. Summary: This enables us to use information in Preprocessor when handling symbol occurrences. Reviewers: arphaman, hokein Reviewed By: hokein Subscribers: malaperle, cfe-commits Differential Revision: https://reviews.llvm.org/D40884 Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h cfe/trunk/lib/Index/IndexingAction.cpp cfe/trunk/tools/libclang/CXIndexDataConsumer.h Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexDataConsumer.h?rev=320030&r1=320029&r2=320030&view=diff == --- cfe/trunk/include/clang/Index/IndexDataConsumer.h (original) +++ cfe/trunk/include/clang/Index/IndexDataConsumer.h Thu Dec 7 03:04:24 2017 @@ -11,6 +11,7 @@ #define LLVM_CLANG_INDEX_INDEXDATACONSUMER_H #include "clang/Index/IndexSymbol.h" +#include "clang/Lex/Preprocessor.h" namespace clang { class ASTContext; @@ -36,6 +37,8 @@ public: virtual void initialize(ASTContext &Ctx) {} + virtual void setPreprocessor(std::shared_ptr PP) {} + /// \returns true to continue indexing, or false to abort. virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles, ArrayRef Relations, Modified: cfe/trunk/lib/Index/IndexingAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingAction.cpp?rev=320030&r1=320029&r2=320030&view=diff == --- cfe/trunk/lib/Index/IndexingAction.cpp (original) +++ cfe/trunk/lib/Index/IndexingAction.cpp Thu Dec 7 03:04:24 2017 @@ -8,10 +8,11 @@ //===--===// #include "clang/Index/IndexingAction.h" -#include "clang/Index/IndexDataConsumer.h" #include "IndexingContext.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/MultiplexConsumer.h" +#include "clang/Index/IndexDataConsumer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" @@ -42,16 +43,18 @@ bool IndexDataConsumer::handleModuleOccu namespace { class IndexASTConsumer : public ASTConsumer { + std::shared_ptr PP; IndexingContext &IndexCtx; public: - IndexASTConsumer(IndexingContext &IndexCtx) -: IndexCtx(IndexCtx) {} + IndexASTConsumer(std::shared_ptr PP, IndexingContext &IndexCtx) + : PP(std::move(PP)), IndexCtx(IndexCtx) {} protected: void Initialize(ASTContext &Context) override { IndexCtx.setASTContext(Context); IndexCtx.getDataConsumer().initialize(Context); +IndexCtx.getDataConsumer().setPreprocessor(PP); } bool HandleTopLevelDecl(DeclGroupRef DG) override { @@ -80,8 +83,10 @@ protected: : DataConsumer(std::move(dataConsumer)), IndexCtx(Opts, *DataConsumer) {} - std::unique_ptr createIndexASTConsumer() { -return llvm::make_unique(IndexCtx); + std::unique_ptr + createIndexASTConsumer(CompilerInstance &CI) { +return llvm::make_unique(CI.getPreprocessorPtr(), + IndexCtx); } void finish() { @@ -98,7 +103,7 @@ public: protected: std::unique_ptr CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override { -return createIndexASTConsumer(); +return createIndexASTConsumer(CI); } void EndSourceFileAction() override { @@ -142,7 +147,7 @@ WrappingIndexAction::CreateASTConsumer(C std::vector> Consumers; Consumers.push_back(std::move(OtherConsumer)); - Consumers.push_back(createIndexASTConsumer()); + Consumers.push_back(createIndexASTConsumer(CI)); return llvm::make_unique(std::move(Consumers)); } @@ -173,6 +178,7 @@ void index::indexASTUnit(ASTUnit &Unit, IndexingContext IndexCtx(Opts, *DataConsumer); IndexCtx.setASTContext(Unit.getASTContext()); DataConsumer->initialize(Unit.getASTContext()); + DataConsumer->setPreprocessor(Unit.getPreprocessorPtr()); indexTranslationUnit(Unit, IndexCtx); DataConsumer->finish(); } @@ -198,7 +204,7 @@ void index::indexModuleFile(serializatio IndexCtx.setASTContext(Ctx); DataConsumer->initialize(Ctx); - for (const Decl *D :Reader.getModuleFileLevelDecls(Mod)) { + for (const Decl *D : Reader.getModuleFileLevelDecls(Mod)) { IndexCtx.indexTopLevelDecl(D); } DataConsumer->finish(); Modified: cfe/trunk/tools/libclang/CXIndexDataConsumer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXIndexDataConsumer.h?rev=320030&r1=320029&r2=320030&view=diff == --- cfe/trunk/tools/libclang/CXInde
[PATCH] D40884: [Index] Add setPreprocessor member to IndexDataConsumer.
This revision was automatically updated to reflect the committed changes. Closed by commit rL320030: [Index] Add setPreprocessor member to IndexDataConsumer. (authored by ioeric). Repository: rL LLVM https://reviews.llvm.org/D40884 Files: cfe/trunk/include/clang/Index/IndexDataConsumer.h cfe/trunk/lib/Index/IndexingAction.cpp cfe/trunk/tools/libclang/CXIndexDataConsumer.h Index: cfe/trunk/include/clang/Index/IndexDataConsumer.h === --- cfe/trunk/include/clang/Index/IndexDataConsumer.h +++ cfe/trunk/include/clang/Index/IndexDataConsumer.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_INDEX_INDEXDATACONSUMER_H #include "clang/Index/IndexSymbol.h" +#include "clang/Lex/Preprocessor.h" namespace clang { class ASTContext; @@ -36,6 +37,8 @@ virtual void initialize(ASTContext &Ctx) {} + virtual void setPreprocessor(std::shared_ptr PP) {} + /// \returns true to continue indexing, or false to abort. virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles, ArrayRef Relations, Index: cfe/trunk/lib/Index/IndexingAction.cpp === --- cfe/trunk/lib/Index/IndexingAction.cpp +++ cfe/trunk/lib/Index/IndexingAction.cpp @@ -8,10 +8,11 @@ //===--===// #include "clang/Index/IndexingAction.h" -#include "clang/Index/IndexDataConsumer.h" #include "IndexingContext.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/MultiplexConsumer.h" +#include "clang/Index/IndexDataConsumer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" @@ -42,16 +43,18 @@ namespace { class IndexASTConsumer : public ASTConsumer { + std::shared_ptr PP; IndexingContext &IndexCtx; public: - IndexASTConsumer(IndexingContext &IndexCtx) -: IndexCtx(IndexCtx) {} + IndexASTConsumer(std::shared_ptr PP, IndexingContext &IndexCtx) + : PP(std::move(PP)), IndexCtx(IndexCtx) {} protected: void Initialize(ASTContext &Context) override { IndexCtx.setASTContext(Context); IndexCtx.getDataConsumer().initialize(Context); +IndexCtx.getDataConsumer().setPreprocessor(PP); } bool HandleTopLevelDecl(DeclGroupRef DG) override { @@ -80,8 +83,10 @@ : DataConsumer(std::move(dataConsumer)), IndexCtx(Opts, *DataConsumer) {} - std::unique_ptr createIndexASTConsumer() { -return llvm::make_unique(IndexCtx); + std::unique_ptr + createIndexASTConsumer(CompilerInstance &CI) { +return llvm::make_unique(CI.getPreprocessorPtr(), + IndexCtx); } void finish() { @@ -98,7 +103,7 @@ protected: std::unique_ptr CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override { -return createIndexASTConsumer(); +return createIndexASTConsumer(CI); } void EndSourceFileAction() override { @@ -142,7 +147,7 @@ std::vector> Consumers; Consumers.push_back(std::move(OtherConsumer)); - Consumers.push_back(createIndexASTConsumer()); + Consumers.push_back(createIndexASTConsumer(CI)); return llvm::make_unique(std::move(Consumers)); } @@ -173,6 +178,7 @@ IndexingContext IndexCtx(Opts, *DataConsumer); IndexCtx.setASTContext(Unit.getASTContext()); DataConsumer->initialize(Unit.getASTContext()); + DataConsumer->setPreprocessor(Unit.getPreprocessorPtr()); indexTranslationUnit(Unit, IndexCtx); DataConsumer->finish(); } @@ -198,7 +204,7 @@ IndexCtx.setASTContext(Ctx); DataConsumer->initialize(Ctx); - for (const Decl *D :Reader.getModuleFileLevelDecls(Mod)) { + for (const Decl *D : Reader.getModuleFileLevelDecls(Mod)) { IndexCtx.indexTopLevelDecl(D); } DataConsumer->finish(); Index: cfe/trunk/tools/libclang/CXIndexDataConsumer.h === --- cfe/trunk/tools/libclang/CXIndexDataConsumer.h +++ cfe/trunk/tools/libclang/CXIndexDataConsumer.h @@ -342,7 +342,7 @@ CXTranslationUnit getCXTU() const { return CXTU; } void setASTContext(ASTContext &ctx); - void setPreprocessor(std::shared_ptr PP); + void setPreprocessor(std::shared_ptr PP) override; bool shouldSuppressRefs() const { return IndexOptions & CXIndexOpt_SuppressRedundantRefs; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40884: [Index] Add setPreprocessor member to IndexDataConsumer.
This revision was automatically updated to reflect the committed changes. Closed by commit rC320030: [Index] Add setPreprocessor member to IndexDataConsumer. (authored by ioeric). Changed prior to commit: https://reviews.llvm.org/D40884?vs=125913&id=125920#toc Repository: rC Clang https://reviews.llvm.org/D40884 Files: include/clang/Index/IndexDataConsumer.h lib/Index/IndexingAction.cpp tools/libclang/CXIndexDataConsumer.h Index: tools/libclang/CXIndexDataConsumer.h === --- tools/libclang/CXIndexDataConsumer.h +++ tools/libclang/CXIndexDataConsumer.h @@ -342,7 +342,7 @@ CXTranslationUnit getCXTU() const { return CXTU; } void setASTContext(ASTContext &ctx); - void setPreprocessor(std::shared_ptr PP); + void setPreprocessor(std::shared_ptr PP) override; bool shouldSuppressRefs() const { return IndexOptions & CXIndexOpt_SuppressRedundantRefs; Index: include/clang/Index/IndexDataConsumer.h === --- include/clang/Index/IndexDataConsumer.h +++ include/clang/Index/IndexDataConsumer.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_INDEX_INDEXDATACONSUMER_H #include "clang/Index/IndexSymbol.h" +#include "clang/Lex/Preprocessor.h" namespace clang { class ASTContext; @@ -36,6 +37,8 @@ virtual void initialize(ASTContext &Ctx) {} + virtual void setPreprocessor(std::shared_ptr PP) {} + /// \returns true to continue indexing, or false to abort. virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles, ArrayRef Relations, Index: lib/Index/IndexingAction.cpp === --- lib/Index/IndexingAction.cpp +++ lib/Index/IndexingAction.cpp @@ -8,10 +8,11 @@ //===--===// #include "clang/Index/IndexingAction.h" -#include "clang/Index/IndexDataConsumer.h" #include "IndexingContext.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/MultiplexConsumer.h" +#include "clang/Index/IndexDataConsumer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" @@ -42,16 +43,18 @@ namespace { class IndexASTConsumer : public ASTConsumer { + std::shared_ptr PP; IndexingContext &IndexCtx; public: - IndexASTConsumer(IndexingContext &IndexCtx) -: IndexCtx(IndexCtx) {} + IndexASTConsumer(std::shared_ptr PP, IndexingContext &IndexCtx) + : PP(std::move(PP)), IndexCtx(IndexCtx) {} protected: void Initialize(ASTContext &Context) override { IndexCtx.setASTContext(Context); IndexCtx.getDataConsumer().initialize(Context); +IndexCtx.getDataConsumer().setPreprocessor(PP); } bool HandleTopLevelDecl(DeclGroupRef DG) override { @@ -80,8 +83,10 @@ : DataConsumer(std::move(dataConsumer)), IndexCtx(Opts, *DataConsumer) {} - std::unique_ptr createIndexASTConsumer() { -return llvm::make_unique(IndexCtx); + std::unique_ptr + createIndexASTConsumer(CompilerInstance &CI) { +return llvm::make_unique(CI.getPreprocessorPtr(), + IndexCtx); } void finish() { @@ -98,7 +103,7 @@ protected: std::unique_ptr CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override { -return createIndexASTConsumer(); +return createIndexASTConsumer(CI); } void EndSourceFileAction() override { @@ -142,7 +147,7 @@ std::vector> Consumers; Consumers.push_back(std::move(OtherConsumer)); - Consumers.push_back(createIndexASTConsumer()); + Consumers.push_back(createIndexASTConsumer(CI)); return llvm::make_unique(std::move(Consumers)); } @@ -173,6 +178,7 @@ IndexingContext IndexCtx(Opts, *DataConsumer); IndexCtx.setASTContext(Unit.getASTContext()); DataConsumer->initialize(Unit.getASTContext()); + DataConsumer->setPreprocessor(Unit.getPreprocessorPtr()); indexTranslationUnit(Unit, IndexCtx); DataConsumer->finish(); } @@ -198,7 +204,7 @@ IndexCtx.setASTContext(Ctx); DataConsumer->initialize(Ctx); - for (const Decl *D :Reader.getModuleFileLevelDecls(Mod)) { + for (const Decl *D : Reader.getModuleFileLevelDecls(Mod)) { IndexCtx.indexTopLevelDecl(D); } DataConsumer->finish(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
krasimir updated this revision to Diff 125921. krasimir added a comment. - Added support for enclosing function names Repository: rC Clang https://reviews.llvm.org/D40909 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestRawStrings.cpp Index: unittests/Format/FormatTestRawStrings.cpp === --- unittests/Format/FormatTestRawStrings.cpp +++ unittests/Format/FormatTestRawStrings.cpp @@ -9,6 +9,8 @@ #include "clang/Format/Format.h" +#include + #include "../Tooling/ReplacementTest.h" #include "FormatTestUtils.h" @@ -19,6 +21,12 @@ #define DEBUG_TYPE "format-test" +#define DBG(A) \ + DEBUG({ \ +llvm::dbgs() << __LINE__ << ":" << __func__ << ":" << #A << " = " << A \ + << "\n"; \ + }); + using clang::tooling::ReplacementTest; using clang::tooling::toReplacements; @@ -65,23 +73,40 @@ FormatStyle getRawStringPbStyleWithColumns(unsigned ColumnLimit) { FormatStyle Style = getLLVMStyle(); Style.ColumnLimit = ColumnLimit; -Style.RawStringFormats = {{/*Delimiter=*/"pb", - /*Kind=*/FormatStyle::LK_TextProto, - /*BasedOnStyle=*/"google"}}; +Style.AdditionalLanguageStyles[FormatStyle::LK_TextProto] = +std::make_shared( +getGoogleStyle(FormatStyle::LK_TextProto)); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_TextProto, + /*Delimiters=*/{"pb"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } - FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringLLVMCppStyleBasedOn(std::string Name) { FormatStyle Style = getLLVMStyle(); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; +DBG(BasedOnStyle.PointerAlignment); return Style; } - FormatStyle getRawStringGoogleCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringGoogleCppStyleBasedOn(std::string Name) { FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } @@ -96,17 +121,19 @@ // llvm style puts '*' on the right. // google style puts '*' on the left. - // Use the llvm style if the raw string style has no BasedOnStyle. - expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", -format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", - getRawStringLLVMCppStyleBasedOn(""))); - - // Use the google style if the raw string style has BasedOnStyle=google. + // Use llvm style outside and the google style inside if the raw string style + // is based on google. expect_eq(R"test(int *i = R"cpp(int* p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", getRawStringLLVMCppStyleBasedOn("google"))); - // Use the llvm style if the raw string style has no BasedOnStyle=llvm. + // Use llvm style if the raw string style has no BasedOnStyle. + expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", +format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", + getRawStringLLVMCppStyleBasedOn(""))); + + // Use google style outside and the llvm style inside if the raw string style + // is based on llvm. expect_eq(R"test(int* i = R"cpp(int *p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)c
[PATCH] D40948: Switch Clang's default C++ language target to C++14
t.p.northover created this revision. Herald added a subscriber: mcrosier. Hi all, So, I've finally managed to run all the tests I wanted and get this out for review. Sorry it's taken so long. This patch switches Clang's default C++ target to C++14 across all platforms and updates the test-suite to pass. I've managed to test Linux, macOS and iOS on both the regression tests and the test-suite (minus externals on Linux because it's my home machine). There's a separate patch for the test-suite which I'll attach to a follow-up message here. I think most of the test changes are pretty straightforward and fall into a few broad categories: - Changes to how the operands for "operator new" are promoted and otherwise wrangled in C++14. Slightly different CodeGen but essentially fine. - Some tests explicitly testing C++ mode differences left the C++98 line without a -std=... argument, I added it. - OpenMP CodeGen changes. As far as I can tell both are correct, but the tests are essentially unreadable so I gave up trying to adapt the output and just set it to C++98 mode. I'm happy to rework anything people think needs changing. Repository: rC Clang https://reviews.llvm.org/D40948 Files: clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGenCXX/new-overflow.cpp clang/test/CodeGenCXX/new.cpp clang/test/CodeGenCXX/vtable-available-externally.cpp clang/test/Lexer/cxx-features.cpp clang/test/Lexer/half-literal.cpp clang/test/OpenMP/taskloop_reduction_codegen.cpp clang/test/OpenMP/taskloop_simd_reduction_codegen.cpp clang/test/Parser/cxx1z-nested-namespace-definition.cpp clang/test/SemaCXX/new-array-size-conv.cpp clang/test/SemaCXX/new-delete.cpp clang/test/SemaCXX/unknown-type-name.cpp clang/test/SemaTemplate/class-template-decl.cpp clang/test/SemaTemplate/explicit-instantiation.cpp Index: clang/test/SemaTemplate/explicit-instantiation.cpp === --- clang/test/SemaTemplate/explicit-instantiation.cpp +++ clang/test/SemaTemplate/explicit-instantiation.cpp @@ -124,10 +124,10 @@ namespace undefined_static_data_member { template struct A { static int a; // expected-note {{here}} -template static int b; // expected-note {{here}} expected-warning {{extension}} +template static int b; // expected-note {{here}} expected-warning 0+ {{extension}} }; struct B { -template static int c; // expected-note {{here}} expected-warning {{extension}} +template static int c; // expected-note {{here}} expected-warning 0+ {{extension}} }; template int A::a; // expected-error {{explicit instantiation of undefined static data member 'a' of class template 'undefined_static_data_member::A'}} @@ -137,14 +137,14 @@ template struct C { static int a; -template static int b; // expected-warning {{extension}} +template static int b; // expected-warning 0+ {{extension}} }; struct D { -template static int c; // expected-warning {{extension}} +template static int c; // expected-warning 0+ {{extension}} }; template int C::a; - template template int C::b; // expected-warning {{extension}} - template int D::c; // expected-warning {{extension}} + template template int C::b; // expected-warning 0+ {{extension}} + template int D::c; // expected-warning 0+ {{extension}} template int C::a; template int C::b; Index: clang/test/SemaTemplate/class-template-decl.cpp === --- clang/test/SemaTemplate/class-template-decl.cpp +++ clang/test/SemaTemplate/class-template-decl.cpp @@ -57,8 +57,7 @@ template class X; // expected-error{{expression}} } -template class X1 var; // expected-warning{{variable templates are a C++14 extension}} \ - // expected-error {{variable has incomplete type 'class X1'}} \ +template class X1 var; // expected-error {{variable has incomplete type 'class X1'}} \ // expected-note {{forward declaration of 'X1'}} namespace M { Index: clang/test/SemaCXX/unknown-type-name.cpp === --- clang/test/SemaCXX/unknown-type-name.cpp +++ clang/test/SemaCXX/unknown-type-name.cpp @@ -95,7 +95,10 @@ template int h(T::type, int); // expected-error{{missing 'typename'}} template int h(T::type x, char); // expected-error{{missing 'typename'}} -template int junk1(T::junk); // expected-warning{{variable templates are a C++14 extension}} +template int junk1(T::junk); +#if __cplusplus <= 201103L +// expected-warning@-2 {{variable templates are a C++14 extension}} +#endif template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}} template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}} #if __cplusplus <= 199711L @@ -106,7 +109,11 @@ // FIXME: We can tell this was intended to be a function because it does not //have a
Re: [PATCH] D40948: Switch Clang's default C++ language target to C++14
Here's the test-suite diff. It's really just a few ancient code-bases that don't compile with C++14, no runtime issues I noticed. Tim. commit a52b065052bfefaac17e7096fd2c911aac62e9da Author: Tim Northover Date: Thu Dec 7 09:16:34 2017 + Support C++14 as a default in Clang diff --git a/External/SPEC/CFP2006/447.dealII/CMakeLists.txt b/External/SPEC/CFP2006/447.dealII/CMakeLists.txt index 4c3388f4..00bd711b 100644 --- a/External/SPEC/CFP2006/447.dealII/CMakeLists.txt +++ b/External/SPEC/CFP2006/447.dealII/CMakeLists.txt @@ -1,6 +1,7 @@ include_directories(${BENCHMARK_DIR}/src/include) add_definitions(-DBOOST_DISABLE_THREADS -Ddeal_II_dimension=3) list(APPEND LDFLAGS -lm) +list(APPEND CXXFLAGS -std=gnu++98) macro(verify_n run_type dir n) # Note that the official SPEC fp tolarence is only "-a .001", however this diff --git a/External/SPEC/CFP2006/447.dealII/Makefile b/External/SPEC/CFP2006/447.dealII/Makefile index 32d6d55d..ac433b55 100644 --- a/External/SPEC/CFP2006/447.dealII/Makefile +++ b/External/SPEC/CFP2006/447.dealII/Makefile @@ -16,7 +16,7 @@ CPPFLAGS += \ -Ddeal_II_dimension=3 \ -DBOOST_DISABLE_THREADS \ -I$(SPEC_BENCH_DIR)/src/include -CXXFLAGS += -stdlib=libstdc++ +CXXFLAGS += -stdlib=libstdc++ -std=gnu++98 STDOUT_FILENAME := log diff --git a/External/SPEC/CFP2006/450.soplex/CMakeLists.txt b/External/SPEC/CFP2006/450.soplex/CMakeLists.txt index f572c6bf..ffd415a5 100644 --- a/External/SPEC/CFP2006/450.soplex/CMakeLists.txt +++ b/External/SPEC/CFP2006/450.soplex/CMakeLists.txt @@ -1,4 +1,5 @@ list(APPEND LDFLAGS -lm) +list(APPEND CXXFLAGS -std=gnu++98) macro(test_input run_type input outname stdout_reltol info_reltol) llvm_test_run(RUN_TYPE ${run_type} diff --git a/External/SPEC/CFP2006/450.soplex/Makefile b/External/SPEC/CFP2006/450.soplex/Makefile index dcb457d4..b7de08ec 100644 --- a/External/SPEC/CFP2006/450.soplex/Makefile +++ b/External/SPEC/CFP2006/450.soplex/Makefile @@ -9,6 +9,7 @@ LEVEL = ../../../.. FP_ABSTOLERANCE = 1.0e-5 CPPFLAGS += -DNDEBUG +CXXFLAGS += -std=gnu++98 LDFLAGS = -lstdc++ -lm LIBS= -lstdc++ -lm diff --git a/External/SPEC/CINT2006/483.xalancbmk/CMakeLists.txt b/External/SPEC/CINT2006/483.xalancbmk/CMakeLists.txt index caf5ab8c..712cefbd 100644 --- a/External/SPEC/CINT2006/483.xalancbmk/CMakeLists.txt +++ b/External/SPEC/CINT2006/483.xalancbmk/CMakeLists.txt @@ -11,6 +11,9 @@ add_definitions( -DXML_USE_NATIVE_TRANSCODER -DXML_USE_INMEM_MESSAGELOADER ) + +list(APPEND CXXFLAGS -std=gnu++98) + include_directories( ${CMAKE_CURRENT_SOURCE_DIR} ${BENCHMARK_DIR}/src diff --git a/External/SPEC/CINT2006/483.xalancbmk/Makefile b/External/SPEC/CINT2006/483.xalancbmk/Makefile index 780ea3cf..2e366415 100644 --- a/External/SPEC/CINT2006/483.xalancbmk/Makefile +++ b/External/SPEC/CINT2006/483.xalancbmk/Makefile @@ -20,6 +20,8 @@ CPPFLAGS += -DNDEBUG -DAPP_NO_THREADS -DXALAN_INMEM_MSG_LOADER\ -I$(SPEC_BENCH_DIR)/src/xercesc/util/Transcoders/Iconv\ -I$(SPEC_BENCH_DIR)/src/xalanc/include +CXXFLAGS += -std=gnu++98 + ifeq ($(TARGET_OS),Darwin) CPPFLAGS += -DSPEC_CPU_MACOSX endif diff --git a/MultiSource/Benchmarks/7zip/CMakeLists.txt b/MultiSource/Benchmarks/7zip/CMakeLists.txt index ee0a9ff9..9cba36f9 100644 --- a/MultiSource/Benchmarks/7zip/CMakeLists.txt +++ b/MultiSource/Benchmarks/7zip/CMakeLists.txt @@ -1,7 +1,7 @@ set(PROG 7zip-benchmark) set(RUN_OPTIONS b) list(APPEND CFLAGS -DBREAK_HANDLER -DUNICODE -D_UNICODE -I${CMAKE_CURRENT_SOURCE_DIR}/C -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/myWindows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/include_windows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP -I. -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DNDEBUG -D_REENTRANT -DENV_UNIX -D_7ZIP_LARGE_PAGES -pthread) -list(APPEND CXXFLAGS -DBREAK_HANDLER -DUNICODE -D_UNICODE -I${CMAKE_CURRENT_SOURCE_DIR}/C -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/myWindows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/include_windows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP -I. -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DNDEBUG -D_REENTRANT -DENV_UNIX -D_7ZIP_LARGE_PAGES -pthread) +list(APPEND CXXFLAGS -Wno-error=c++11-narrowing -DBREAK_HANDLER -DUNICODE -D_UNICODE -I${CMAKE_CURRENT_SOURCE_DIR}/C -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/myWindows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/include_windows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP -I. -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DNDEBUG -D_REENTRANT -DENV_UNIX -D_7ZIP_LARGE_PAGES -pthread) list(APPEND LDFLAGS -lstdc++ -pthread) set(Source CPP/myWindows/myGetTickCount.cpp CPP/myWindows/wine_date_and_time.cpp CPP/myWindows/myAddExeFlag.cpp CPP/myWindows/mySplitCommandLine.cpp CPP/7zip/UI/Console/BenchCon.cpp CPP/7zip/UI/Console/ConsoleClose.cpp CPP/7zip/UI/Console/ExtractCallbackConsole.cpp CPP/7zip/UI/Console/List.cpp CPP/7zip/UI/Console/Main.cpp CPP/7zip/UI/Console/MainAr.cpp CPP/7zip/UI/Console/OpenCallbackConsole.cpp CPP/7zip/UI/Console/PercentPri
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
krasimir updated this revision to Diff 125927. krasimir added a comment. - Updated documentation Repository: rC Clang https://reviews.llvm.org/D40909 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestRawStrings.cpp Index: unittests/Format/FormatTestRawStrings.cpp === --- unittests/Format/FormatTestRawStrings.cpp +++ unittests/Format/FormatTestRawStrings.cpp @@ -65,23 +65,39 @@ FormatStyle getRawStringPbStyleWithColumns(unsigned ColumnLimit) { FormatStyle Style = getLLVMStyle(); Style.ColumnLimit = ColumnLimit; -Style.RawStringFormats = {{/*Delimiter=*/"pb", - /*Kind=*/FormatStyle::LK_TextProto, - /*BasedOnStyle=*/"google"}}; +Style.AdditionalLanguageStyles[FormatStyle::LK_TextProto] = +std::make_shared( +getGoogleStyle(FormatStyle::LK_TextProto)); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_TextProto, + /*Delimiters=*/{"pb"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } - FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringLLVMCppStyleBasedOn(std::string Name) { FormatStyle Style = getLLVMStyle(); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } - FormatStyle getRawStringGoogleCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringGoogleCppStyleBasedOn(std::string Name) { FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } @@ -96,17 +112,19 @@ // llvm style puts '*' on the right. // google style puts '*' on the left. - // Use the llvm style if the raw string style has no BasedOnStyle. - expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", -format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", - getRawStringLLVMCppStyleBasedOn(""))); - - // Use the google style if the raw string style has BasedOnStyle=google. + // Use llvm style outside and the google style inside if the raw string style + // is based on google. expect_eq(R"test(int *i = R"cpp(int* p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", getRawStringLLVMCppStyleBasedOn("google"))); - // Use the llvm style if the raw string style has no BasedOnStyle=llvm. + // Use llvm style if the raw string style has no BasedOnStyle. + expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", +format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", + getRawStringLLVMCppStyleBasedOn(""))); + + // Use google style outside and the llvm style inside if the raw string style + // is based on llvm. expect_eq(R"test(int* i = R"cpp(int *p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", getRawStringGoogleCppStyleBasedOn("llvm"))); @@ -121,29 +139,6 @@ s = R"PB(item:1)PB"; t = R"pb(item:1)pb";)test", getRawStringPbStyleWithColumns(40))); - - FormatStyle MixedStyle = getLLVMStyle(); - MixedStyle.RawStringFormats = { - {/*Delimiter=*/"cpp", /*Kind=*/FormatStyle::LK_Cpp, - /*BasedOnStyle=*/"llvm"}, - {/*Delimiter=*/"CPP", /*Kind=*/FormatStyle::LK_Cpp, - /*BasedOnStyle=*/"google"}}; - - // Format the 'cpp' raw string with '*' on the right. - // Format the 'CPP' raw string with '*' on the left. - // Do not format the 'Cpp' raw string. - // Do
[PATCH] D40548: [clangd] Prototyping index support and naive index-based global code completion. WIP
ioeric updated this revision to Diff 125928. ioeric added a comment. - More cleanups and merged with https://reviews.llvm.org/D40897 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/ClangdIndex.cpp clangd/ClangdIndex.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Symbol.cpp clangd/Symbol.h clangd/SymbolCompletionInfo.cpp clangd/SymbolCompletionInfo.h clangd/SymbolIndex.h clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -90,6 +90,15 @@ "Trace internal events and timestamps in chrome://tracing JSON format"), llvm::cl::init(""), llvm::cl::Hidden); +static llvm::cl::opt EnableIndexBasedCompletion( +"enable-index-based-completion", +llvm::cl::desc( +"Enable index-based global code completion (experimental). Clangd will " +"use symbols built from ASTs of opened files and additional indexes " +"(e.g. offline built codebase-wide symbol table) to complete partial " +"symbols."), +llvm::cl::init(false)); + int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); @@ -170,10 +179,14 @@ clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = EnableSnippets; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; + if (EnableIndexBasedCompletion) +// Disable sema code completion for qualified code completion and use global +// symbol index instead. +CCOpts.IncludeGlobals = false; // Initialize and run ClangdLSPServer. ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory, -CCOpts, ResourceDirRef, -CompileCommandsDirPath); +CCOpts, ResourceDirRef, CompileCommandsDirPath, +EnableIndexBasedCompletion, {}); constexpr int NoShutdownRequestErrorCode = 1; llvm::set_thread_name("clangd.main"); return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode; Index: clangd/SymbolIndex.h === --- /dev/null +++ clangd/SymbolIndex.h @@ -0,0 +1,60 @@ +//===--- CompletionIndex.h - Index for code completion ---*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLINDEX_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLINDEX_H + +#include "SymbolCompletionInfo.h" +#include "llvm/Support/Error.h" + +namespace clang { +namespace clangd { + +struct CompletionRequest { + std::string Query; + size_t MaxCandidateCount = UINT_MAX; +}; + +/// \brief Signals for scoring completion candidates. +struct ScoreSignals { + // FIXME: add score signals like cross-reference count. +}; + +struct CompletionSymbol { + ScoreSignals Signals; + float SymbolScore; + + std::string USR; + index::SymbolKind Kind; + std::string QualifiedName; + + SymbolCompletionInfo CompletionInfo; +}; + +struct CompletionResult { + std::vector Symbols; + bool AllMatched; +}; + +class SymbolIndex { +public: + virtual ~SymbolIndex() = default; + + virtual llvm::Expected + complete(const CompletionRequest &Req) const = 0; + + // FIXME: add interfaces for more index use cases: + // - Symbol getSymbolInfo(llvm::StringRef USR); + // - getAllOccurrences(llvm::StringRef USR); +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLINDEX_H Index: clangd/SymbolCompletionInfo.h === --- /dev/null +++ clangd/SymbolCompletionInfo.h @@ -0,0 +1,31 @@ +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLCOMPLETIONINFO_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLCOMPLETIONINFO_H + +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Index/IndexSymbol.h" +#include "clang/Lex/Preprocessor.h" + +namespace clang { +namespace clangd { + +struct SymbolCompletionInfo { + static SymbolCompletionInfo create(ASTContext &Ctx, Preprocessor &PP, + const NamedDecl *ND); + + SymbolCompletionInfo() = default; + /// Label that can be be displayed in the completion list. + std::string Label; + /// Symbol annotation and/or comment for the symbol declaration. + std::string Documentation; + /// Detail about the symbol like result type. + std::
[PATCH] D40937: [clang-tidy] Infinite loop checker
martong added inline comments. Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:105 + return llvm::make_unique( + *(new ExprSequence(TheCFG.get(), &ASTCtx))); +} `make_unique` is a forwarding function, therefore there is no need to create an object and then move it. Instead you can simply forward the arguments to the constructor: `return llvm::make_unique(TheCFG.get(), &ASTCtx)` https://reviews.llvm.org/D40937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
djasper added inline comments. Comment at: include/clang/Format/Format.h:1216 +LK_TextProto, +/// Do not use. Keep at last position. +LK_End, Lets find a way to implement without this in the public header file. Comment at: include/clang/Format/Format.h:1375 +std::vector EnclosingFunctionNames; +/// \brief The canonical delimiter for this language. +std::string CanonicalDelimiter; Can you pull apart this patch? In my view, it has three parts that have an ordering, but are actually fairly independent: 1. Propagate all configured languages to the formatting library. First patch to land, should not affect the visible behavior. 2. Restructure RawStringFormat to be centered around each language. This is a restructuring to make things easier and use #1. 3. Add a CanonicalDelimiter and make clang-format canonicalize it. I'll focus my comments on what's required for #1 for now as that is already complicated (IMO). Comment at: lib/Format/Format.cpp:919 + } + if (LanguageFound) { +for (int i = Styles.size() - 1; i >= 0; --i) { Prefer early exit, i.e. if (!LanguageFound) return make_error_code(ParseError::Unsuitable); ... Comment at: lib/Format/Format.cpp:920 + if (LanguageFound) { +for (int i = Styles.size() - 1; i >= 0; --i) { + if (Styles[i].Language == FormatStyle::LK_None) { I think this is getting a bit convoluted and I don't even understand whether we are doing what is document (even before this patch). So in lines 892-905, we verify that: - Only the first Style in the file is allowed be LK_None. - No language is duplicated. That seems good. According to the documentation: "The first section may have no language set, it will set the default style options for all lanugages.". Does the latter part actually happen? Seems to me that we are just setting "Style" to the style configured for a specific language, completely ignoring values that might have been set in the LK_None style. Or is that somehow happening when reading the JSON? Independent of that, I think we should use this structure more explicitly. I think we should create an additional class (FormatStyles or FormatStyleSet or something) that is returned by this function and handed to the formatting library. This function then doesn't need to look at the language anymore. That class should then have a function getFormatStyle(LanguageKind Language); that returns the style for a particular language (doing the default logic, etc.). Internally, it can likely just have a map and I don't think you need to pre-fill that for all language kinds. If a language kind is not in the map, you can just return what's stored for LK_None. WDYT? Repository: rC Clang https://reviews.llvm.org/D40909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
krasimir added inline comments. Comment at: include/clang/Format/Format.h:1375 +std::vector EnclosingFunctionNames; +/// \brief The canonical delimiter for this language. +std::string CanonicalDelimiter; djasper wrote: > Can you pull apart this patch? > > In my view, it has three parts that have an ordering, but are actually fairly > independent: > > 1. Propagate all configured languages to the formatting library. First patch > to land, should not affect the visible behavior. > 2. Restructure RawStringFormat to be centered around each language. This is a > restructuring to make things easier and use #1. > 3. Add a CanonicalDelimiter and make clang-format canonicalize it. > > I'll focus my comments on what's required for #1 for now as that is already > complicated (IMO). I believe these should all go together: the reason that we propagate all configured languages to the formatting library is to be able to use them as a replacement for the BasedOnStyle in RawStringFormat. To make this possible, we need to update the internal structure of RawStringFormat itself to base it around each language. The canonical delimiter part is just a convenience for this I guess, which could be split. My biggest concern with (1) is that since it has no visible behavior and no other uses except for the adaptation of (2), it is not testable by itself and it's not evident that a patch doing just (1) would handle the things correctly. Repository: rC Clang https://reviews.llvm.org/D40909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.
ilya-biryukov requested changes to this revision. ilya-biryukov added inline comments. This revision now requires changes to proceed. Comment at: lib/Frontend/PrecompiledPreamble.cpp:242 std::shared_ptr PCHContainerOps, bool StoreInMemory, -PreambleCallbacks &Callbacks) { +PreambleCallbacks &Callbacks, std::unique_ptr PPCallbacks) { assert(VFS && "VFS is null"); Could we add a method to `PreambleCallbacks` to create `PPCallbacks` instead? Otherwise we have both `MacroDefined` in `PreambleCallbacks` and a separate set of PPCallbacks, so we have two ways of doing the same thing. ``` class PreambleCallbacks { public: // ... /// Remove this. virtual void HandleMacroDefined(...); // Add this instead. virtual std::unique_ptr createPPCallbacks(); } ``` Alternatively, follow the current pattern in `PreambleCallbacks` and add some extra functions from the `PPCallbacks` interface to it. This would not require changes to the existing usages of `PrecompiledPreamble` in `ASTUnit`. Albeit, I'd prefer the first option. ``` class PreambleCallbacks { public: // ... // Keep this virtual void HandleMacroDefined(...); // Add the ones you need, e.g. virtual void InclusionDirective(...); virtual void FileChanged(...); }; ``` Repository: rC Clang https://reviews.llvm.org/D39375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
krasimir added inline comments. Comment at: lib/Format/Format.cpp:920 + if (LanguageFound) { +for (int i = Styles.size() - 1; i >= 0; --i) { + if (Styles[i].Language == FormatStyle::LK_None) { djasper wrote: > I think this is getting a bit convoluted and I don't even understand whether > we are doing what is document (even before this patch). > > So in lines 892-905, we verify that: > - Only the first Style in the file is allowed be LK_None. > - No language is duplicated. > > That seems good. > > According to the documentation: "The first section may have no language set, > it will set the default style options for all lanugages.". Does the latter > part actually happen? Seems to me that we are just setting "Style" to the > style configured for a specific language, completely ignoring values that > might have been set in the LK_None style. Or is that somehow happening when > reading the JSON? > > Independent of that, I think we should use this structure more explicitly. I > think we should create an additional class (FormatStyles or FormatStyleSet or > something) that is returned by this function and handed to the formatting > library. This function then doesn't need to look at the language anymore. > > That class should then have a function getFormatStyle(LanguageKind Language); > that returns the style for a particular language (doing the default logic, > etc.). Internally, it can likely just have a map and I don't think > you need to pre-fill that for all language kinds. If a language kind is not > in the map, you can just return what's stored for LK_None. WDYT? Yes, defaulting to the None for missing language specifications is handled at line 912: ``` if (!LanguageFound && (Styles[i].Language == Language || Styles[i].Language == FormatStyle::LK_None ``` I was thinking of the FormatStyleSet approach but the problem is that this has repercussions all over the library. We could indeed update this specific function that way, but fundamentally the additional language styles are part of the FormatStyle and need to somehow be recorded inside there. That's why I went with KISS and directly made this function handle that case. Repository: rC Clang https://reviews.llvm.org/D40909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40909: [clang-format] Reorganize raw string delimiters
krasimir updated this revision to Diff 125939. krasimir edited the summary of this revision. krasimir added a comment. - Address review comments Repository: rC Clang https://reviews.llvm.org/D40909 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestRawStrings.cpp Index: unittests/Format/FormatTestRawStrings.cpp === --- unittests/Format/FormatTestRawStrings.cpp +++ unittests/Format/FormatTestRawStrings.cpp @@ -65,23 +65,39 @@ FormatStyle getRawStringPbStyleWithColumns(unsigned ColumnLimit) { FormatStyle Style = getLLVMStyle(); Style.ColumnLimit = ColumnLimit; -Style.RawStringFormats = {{/*Delimiter=*/"pb", - /*Kind=*/FormatStyle::LK_TextProto, - /*BasedOnStyle=*/"google"}}; +Style.AdditionalLanguageStyles[FormatStyle::LK_TextProto] = +std::make_shared( +getGoogleStyle(FormatStyle::LK_TextProto)); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_TextProto, + /*Delimiters=*/{"pb"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } - FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringLLVMCppStyleBasedOn(std::string Name) { FormatStyle Style = getLLVMStyle(); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } - FormatStyle getRawStringGoogleCppStyleBasedOn(std::string BasedOnStyle) { + FormatStyle getRawStringGoogleCppStyleBasedOn(std::string Name) { FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp); -Style.RawStringFormats = {{/*Delimiter=*/"cpp", - /*Kind=*/FormatStyle::LK_Cpp, BasedOnStyle}}; +FormatStyle BasedOnStyle = getLLVMStyle(); +getPredefinedStyle(Name, FormatStyle::LK_Cpp, &BasedOnStyle); +Style.AdditionalLanguageStyles[FormatStyle::LK_Cpp] = +std::make_shared(BasedOnStyle); +Style.RawStringFormats = {{/*Language=*/FormatStyle::LK_Cpp, + /*Delimiters=*/{"cpp"}, + /*EnclosingFunctionNames=*/{}, + /*CanonicalDelimiter=*/""}}; return Style; } @@ -96,17 +112,19 @@ // llvm style puts '*' on the right. // google style puts '*' on the left. - // Use the llvm style if the raw string style has no BasedOnStyle. - expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", -format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", - getRawStringLLVMCppStyleBasedOn(""))); - - // Use the google style if the raw string style has BasedOnStyle=google. + // Use llvm style outside and the google style inside if the raw string style + // is based on google. expect_eq(R"test(int *i = R"cpp(int* p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", getRawStringLLVMCppStyleBasedOn("google"))); - // Use the llvm style if the raw string style has no BasedOnStyle=llvm. + // Use llvm style if the raw string style has no BasedOnStyle. + expect_eq(R"test(int *i = R"cpp(int *p = nullptr;)cpp")test", +format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", + getRawStringLLVMCppStyleBasedOn(""))); + + // Use google style outside and the llvm style inside if the raw string style + // is based on llvm. expect_eq(R"test(int* i = R"cpp(int *p = nullptr;)cpp")test", format(R"test(int * i = R"cpp(int * p = nullptr;)cpp")test", getRawStringGoogleCppStyleBasedOn("llvm"))); @@ -121,29 +139,6 @@ s = R"PB(item:1)PB"; t = R"pb(item:1)pb";)test", getRawStringPbStyleWithColumns(40))); - - FormatStyle MixedStyle = getLLVMStyle(); - MixedStyle.RawStringFormats = { - {/*Delimiter=*/"cpp", /*Kind=*/FormatStyle::LK_Cpp, - /*BasedOnStyle=*/"llvm"}, - {/*Delimiter=*/"CPP", /*Kind=*/FormatStyle::LK_Cpp, - /*BasedOnStyle=*/"google"}}; - - // Format the 'cpp' raw string with '*' on the right. - // Format the 'CPP' raw string with '*' on the left. -
[PATCH] D40485: [clangd] Introduced a Context that stores implicit data
ilya-biryukov added inline comments. Comment at: clangd/Context.h:25 + +class ContextData { +public: sammccall wrote: > IIUC, the only reason we expose separate `Context` and `ContextData` types is > to give things like Span a stable reference to hold onto (`ContextData*`). > > Could we use `Context` instead (a copy)? Then we incur `shared_ptr` overhead > for span creation, but greatly simplify the interface. > If we want to avoid the overhead, the Span could maybe grab whatever it needs > out of the context at construction time, for use in the destructor. > > Either way, we should make sure `Context` is front-and-center in this header, > and I hope we can hide ContextData entirely. I have considered making `Context` the only used interface, but I hated the fact that each `Span` constructor would copy a `shared_ptr`. (And `Span`s can be pretty ubiquitous). On the other hand, that should not happen when tracing is off, so we won't be adding overhead when `Span` are not used. Will try moving `ContextData` into `.cpp` file, this should certainly be an implementation detail. And we can expose it later if we'll actually have a use-case for that (i.e. not copying `shared_ptr`s). Comment at: clangd/Context.h:63 +/// used as parents for some other Contexts. +class Context { +public: ioeric wrote: > sammccall wrote: > > I think we should strongly consider calling the class Ctx over Context. > > It's going to appear in many function signatures, and I'm not sure the > > extra characters buy us anything considering the abbreviation is pretty > > unambiguous, and the full version isn't very explicit. > I have no opinion on the names... Just wondering: if the class is called > `Ctx`, what name do you suggest to use for variables? With `Context`, you can > probably use `Ctx`. But... LLVM variable naming is sad... :( I'd also go with `Context Ctx` rather than `Ctx C`. I'd say `Context` is quite a small and concise name. It even reads better than `Ctx` to me. But I'm fine either way, so will change the name to `Ctx`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC
sammccall created this revision. Herald added subscribers: cfe-commits, ilya-biryukov, klimek. This improves readability of tests and error messages. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40952 Files: test/clangd/completion-items-kinds.test test/clangd/completion-priorities.test test/clangd/completion-qualifiers.test test/clangd/completion-snippet.test unittests/clangd/CodeCompleteTests.cpp unittests/clangd/Matchers.h Index: unittests/clangd/Matchers.h === --- /dev/null +++ unittests/clangd/Matchers.h @@ -0,0 +1,103 @@ +//===-- Matchers.h --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// GMock matchers that aren't specific to particular tests. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H +#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H +#include "gmock/gmock.h" + +namespace clang { +namespace clangd { +using ::testing::Matcher; + +// EXPECT_IFF expects matcher if condition is true, and Not(matcher) if false. +// This is hard to write as a function, because matchers may be polymorphic. +#define EXPECT_IFF(condition, value, matcher) \ + do { \ +if (condition) \ + EXPECT_THAT(value, matcher); \ +else \ + EXPECT_THAT(value, ::testing::Not(matcher)); \ + } while (0) + +// HasSubsequence(m1, m2, ...) matches a vector containing elements that match +// m1, m2 ... in that order. +template +class SubsequenceMatcher +: public ::testing::MatcherInterface &> { + std::vector> Matchers; + +public: + SubsequenceMatcher(std::vector> M) : Matchers(M) {} + + void DescribeTo(std::ostream *OS) const { +*OS << "Contains the subsequence ["; +const char *Sep = ""; +for (const auto &M : Matchers) { + *OS << Sep; + M.DescribeTo(OS); + Sep = ", "; +} +*OS << "]"; + } + + bool MatchAndExplain(const std::vector &V, + ::testing::MatchResultListener *L) const { +std::vector Matches(Matchers.size()); +size_t I = 0; +for (size_t J = 0; I < Matchers.size() && J < V.size(); ++J) + if (Matchers[I].Matches(V[J])) +Matches[I++] = J; +if (I == Matchers.size()) // We exhausted all matchers. + return true; +if (L->IsInterested()) { + *L << "\n Matched:"; + for (size_t K = 0; K < I; ++K) { +*L << "\n\t"; +Matchers[K].DescribeTo(L->stream()); +*L << " ==> " << ::testing::PrintToString(V[Matches[K]]); + } + *L << "\n\t"; + Matchers[I].DescribeTo(L->stream()); + *L << " ==> no subsequent match"; +} +return false; + } +}; + +template class PolySubsequenceMatcher { + std::tuple Matchers; + +public: + PolySubsequenceMatcher(M &&... Args) + : Matchers(std::make_tuple(std::forward(Args)...)) {} + + template operator Matcher &>() const { +return ::testing::MakeMatcher(new SubsequenceMatcher( +TypedMatchers(llvm::index_sequence_for{}))); + } + +private: + template + std::vector> TypedMatchers(llvm::index_sequence) const { +return {std::get(Matchers)...}; + } +}; + +template +PolySubsequenceMatcher HasSubsequence(Args &&... M) { + return PolySubsequenceMatcher(std::forward(M)...); +} + +} // namespace clangd +} // namespace clang +#endif Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -8,6 +8,7 @@ //===--===// #include "ClangdServer.h" #include "Compiler.h" +#include "Matchers.h" #include "Protocol.h" #include "TestFS.h" #include "gmock/gmock.h" @@ -18,15 +19,23 @@ // Let GMock print completion items. void PrintTo(const CompletionItem &I, std::ostream *O) { llvm::raw_os_ostream OS(*O); - OS << toJSON(I); + OS << I.label << " - " << toJSON(I); +} +void PrintTo(const std::vector &V, std::ostream *O) { + *O << "{\n"; + for (const auto &I : V) { +*O << "\t"; +PrintTo(I, O); +*O << "\n"; + } + *O << "}"; } namespace { using namespace llvm; using ::testing::AllOf; using ::testing::Contains; using ::testing::ElementsAre; -using ::testing::Matcher; using ::testing::Not; class IgnoreDiagnos
[PATCH] D40948: Switch Clang's default C++ language target to C++14
Hahnfeld added a reviewer: ABataev. Hahnfeld added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1732 case InputKind::ObjCXX: // The PS4 uses C++11 as the default C++ standard. + LangStd = LangStandard::lang_gnucxx14; This comment should be removed. Repository: rC Clang https://reviews.llvm.org/D40948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.
schroedersi added a comment. Ping https://reviews.llvm.org/D30946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
alexfh added a comment. Sorry for the delay. I missed this revision somehow. Please add cfe-commits to the subscribers list so that others can chime in. https://reviews.llvm.org/D39363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
alexfh added a comment. One nit. Otherwise seems fine. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:502 -if (!Type.isNull() && Type.isLocalConstQualified() && +if (!Type.isNull() && Type.isConstQualified() && Decl->isStaticDataMember() && NamingStyles[SK_ClassConstant]) Can we check for `!Type.isNull() && Type.isConstQualified()` once here? if (!Type.isNull() && Type.isConstQualified()) { if (Decl->isStaticDataMember() && NamingStyles[SK_ClassConstant]) return SK_ClassConstant; if (Decl->isFileVarDecl() && NamingStyles[SK_GlobalConstant]) return SK_GlobalConstant; ... } https://reviews.llvm.org/D39363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40956: [AMDGPU] Switch to the new addr space mapping by default for clang
yaxunl created this revision. Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, kzhuravl. Will clean up the old addr space mapping in separate patch. https://reviews.llvm.org/D40956 Files: lib/Basic/Targets/AMDGPU.cpp lib/Basic/Targets/AMDGPU.h test/CodeGen/address-space.c test/CodeGen/default-address-space.c test/CodeGen/target-data.c test/CodeGenOpenCL/address-spaces.cl test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl test/CodeGenOpenCL/amdgpu-alignment.cl test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl test/CodeGenOpenCL/amdgpu-env-amdgiz.cl test/CodeGenOpenCL/amdgpu-nullptr.cl test/CodeGenOpenCL/blocks.cl test/CodeGenOpenCL/byval.cl test/CodeGenOpenCL/opencl_types.cl test/CodeGenOpenCL/size_t.cl Index: test/CodeGenOpenCL/size_t.cl === --- test/CodeGenOpenCL/size_t.cl +++ test/CodeGenOpenCL/size_t.cl @@ -1,12 +1,14 @@ // RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple spir-unknown-unknown -o - | FileCheck --check-prefix=SZ32 %s // RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple spir64-unknown-unknown -o - | FileCheck --check-prefix=SZ64 --check-prefix=SZ64ONLY %s -// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple amdgcn -o - | FileCheck --check-prefix=SZ64 --check-prefix=AMDONLY %s -// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple amdgcn---opencl -o - | FileCheck --check-prefix=SZ64 --check-prefix=AMDONLY %s +// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple amdgcn -o - | FileCheck --check-prefix=SZ64 --check-prefix=AMDGCN %s +// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -emit-llvm -O0 -triple amdgcn---opencl -o - | FileCheck --check-prefix=SZ64 --check-prefix=AMDGCN %s //SZ32: define{{.*}} i32 @test_ptrtoint_private(i8* %x) //SZ32: ptrtoint i8* %{{.*}} to i32 -//SZ64: define{{.*}} i64 @test_ptrtoint_private(i8* %x) -//SZ64: ptrtoint i8* %{{.*}} to i64 +//SZ64ONLY: define{{.*}} i64 @test_ptrtoint_private(i8* %x) +//SZ64ONLY: ptrtoint i8* %{{.*}} to i64 +//AMDGCN: define{{.*}} i64 @test_ptrtoint_private(i8 addrspace(5)* %x) +//AMDGCN: ptrtoint i8 addrspace(5)* %{{.*}} to i64 size_t test_ptrtoint_private(private char* x) { return (size_t)x; } @@ -37,18 +39,21 @@ //SZ32: define{{.*}} i32 @test_ptrtoint_generic(i8 addrspace(4)* %x) //SZ32: ptrtoint i8 addrspace(4)* %{{.*}} to i32 -//SZ64: define{{.*}} i64 @test_ptrtoint_generic(i8 addrspace(4)* %x) -//SZ64: ptrtoint i8 addrspace(4)* %{{.*}} to i64 +//SZ64ONLY: define{{.*}} i64 @test_ptrtoint_generic(i8 addrspace(4)* %x) +//SZ64ONLY: ptrtoint i8 addrspace(4)* %{{.*}} to i64 +//AMDGCN: define{{.*}} i64 @test_ptrtoint_generic(i8* %x) +//AMDGCN: ptrtoint i8* %{{.*}} to i64 size_t test_ptrtoint_generic(generic char* x) { return (size_t)x; } //SZ32: define{{.*}} i8* @test_inttoptr_private(i32 %x) //SZ32: inttoptr i32 %{{.*}} to i8* -//SZ64: define{{.*}} i8* @test_inttoptr_private(i64 %x) -//AMDONLY: trunc i64 %{{.*}} to i32 -//AMDONLY: inttoptr i32 %{{.*}} to i8* +//SZ64ONLY: define{{.*}} i8* @test_inttoptr_private(i64 %x) //SZ64ONLY: inttoptr i64 %{{.*}} to i8* +//AMDGCN: define{{.*}} i8 addrspace(5)* @test_inttoptr_private(i64 %x) +//AMDGCN: trunc i64 %{{.*}} to i32 +//AMDGCN: inttoptr i32 %{{.*}} to i8 addrspace(5)* private char* test_inttoptr_private(size_t x) { return (private char*)x; } @@ -64,8 +69,8 @@ //SZ32: define{{.*}} i8 addrspace(3)* @test_add_local(i8 addrspace(3)* %x, i32 %y) //SZ32: getelementptr inbounds i8, i8 addrspace(3)* %{{.*}}, i32 //SZ64: define{{.*}} i8 addrspace(3)* @test_add_local(i8 addrspace(3)* %x, i64 %y) -//AMDONLY: trunc i64 %{{.*}} to i32 -//AMDONLY: getelementptr inbounds i8, i8 addrspace(3)* %{{.*}}, i32 +//AMDGCN: trunc i64 %{{.*}} to i32 +//AMDGCN: getelementptr inbounds i8, i8 addrspace(3)* %{{.*}}, i32 //SZ64ONLY: getelementptr inbounds i8, i8 addrspace(3)* %{{.*}}, i64 local char* test_add_local(local char* x, ptrdiff_t y) { return x + y; @@ -92,9 +97,12 @@ //SZ32: define{{.*}} i32 @test_sub_private(i8* %x, i8* %y) //SZ32: ptrtoint i8* %{{.*}} to i32 //SZ32: ptrtoint i8* %{{.*}} to i32 -//SZ64: define{{.*}} i64 @test_sub_private(i8* %x, i8* %y) -//SZ64: ptrtoint i8* %{{.*}} to i64 -//SZ64: ptrtoint i8* %{{.*}} to i64 +//SZ64ONLY: define{{.*}} i64 @test_sub_private(i8* %x, i8* %y) +//SZ64ONLY: ptrtoint i8* %{{.*}} to i64 +//SZ64ONLY: ptrtoint i8* %{{.*}} to i64 +//AMDGCN: define{{.*}} i64 @test_sub_private(i8 addrspace(5)* %x, i8 addrspace(5)* %y) +//AMDGCN: ptrtoint i8 addrspace(5)* %{{.*}} to i64 +//AMDGCN: ptrtoint i8 addrspace(5)* %{{.*}} to i64 ptrdiff_t test_sub_private(private char* x, private char *y) { return x - y; } @@ -102,9 +110,12 @@ //SZ32: define{{.*}} i32 @test_sub_mix(i8* %x, i8 addrspace(4)* %y) //SZ32: ptrto
[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.
ioeric updated this revision to Diff 125960. ioeric added a comment. - Use IncludeNamespaceLevelDecls option; fix some broken tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/ClangdIndex.cpp clangd/ClangdIndex.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Symbol.cpp clangd/Symbol.h clangd/SymbolCompletionInfo.cpp clangd/SymbolCompletionInfo.h clangd/SymbolIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/CMakeLists.txt unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp === --- /dev/null +++ unittests/clangd/SymbolCollectorTests.cpp @@ -0,0 +1,110 @@ +//===-- SymbolCollectorTests.cpp ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "Symbol.h" +#include "clang/Index/IndexingAction.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "gtest/gtest.h" +#include "gmock/gmock.h" + +#include +#include + +using testing::ElementsAre; +using testing::Eq; +using testing::Field; + +namespace clang { +namespace clangd { + +namespace { +class SymbolIndexActionFactory : public tooling::FrontendActionFactory { + public: + SymbolIndexActionFactory() = default; + + clang::FrontendAction *create() override { +index::IndexingOptions IndexOpts; +IndexOpts.SystemSymbolFilter = +index::IndexingOptions::SystemSymbolFilterKind::All; +IndexOpts.IndexFunctionLocals = false; +Collector = std::make_shared(); +FrontendAction *Action = +index::createIndexingAction(Collector, IndexOpts, nullptr).release(); +return Action; + } + + std::shared_ptr Collector; +}; + +class SymbolCollectorTest : public ::testing::Test { +public: + bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) { +llvm::IntrusiveRefCntPtr InMemoryFileSystem( +new vfs::InMemoryFileSystem); +llvm::IntrusiveRefCntPtr Files( +new FileManager(FileSystemOptions(), InMemoryFileSystem)); + +const std::string FileName = "symbol.cc"; +const std::string HeaderName = "symbols.h"; +auto Factory = llvm::make_unique(); + +tooling::ToolInvocation Invocation( +{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, +Factory->create(), Files.get(), +std::make_shared()); + +InMemoryFileSystem->addFile(HeaderName, 0, +llvm::MemoryBuffer::getMemBuffer(HeaderCode)); + +std::string Content = "#include\"" + std::string(HeaderName) + "\""; +Content += "\n" + MainCode.str(); +InMemoryFileSystem->addFile(FileName, 0, +llvm::MemoryBuffer::getMemBuffer(Content)); +Invocation.run(); +Symbols = Factory->Collector->getSymbols(); +return true; + } + +protected: + std::set Symbols; +}; + +TEST_F(SymbolCollectorTest, CollectSymbol) { + const std::string Header = R"( +class Foo { + void f(); +}; +void f1(); +inline void f2() {} + )"; + const std::string Main = R"( +namespace { +void ff() {} // ignore +} +void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(Field(&Symbol::QualifiedName, Eq("Foo")), + Field(&Symbol::QualifiedName, Eq("Foo::f")), + Field(&Symbol::QualifiedName, Eq("f1")), + Field(&Symbol::QualifiedName, Eq("f2"; +} + +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt === --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -15,6 +15,7 @@ JSONExprTests.cpp TestFS.cpp TraceTests.cpp + SymbolCollectorTests.cpp ) target_link_libraries(ClangdTests Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -90,6 +90,15 @@ "Trace internal events and timestamps in chrome://tracing JSON format"), llvm::cl::init(""),
[PATCH] D40939: [analyzer] Avoid element regions of void type.
xazax.hun added a comment. I like the idea of adding those assertions but a bit worried about the other changes. Basically (if I get this right), we are masking the issues here and I am a bit afraid that they will get forgotten. I think it would be nice to at least add a FIXME that this path should never be triggered. Repository: rC Clang https://reviews.llvm.org/D40939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.
ioeric updated this revision to Diff 125962. ioeric added a comment. Diff on https://reviews.llvm.org/D40897 instead origin/master! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/ClangdIndex.cpp clangd/ClangdIndex.h clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.cpp clangd/ClangdUnitStore.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Symbol.cpp clangd/Symbol.h clangd/SymbolCompletionInfo.cpp clangd/SymbolCompletionInfo.h clangd/SymbolIndex.h clangd/tool/ClangdMain.cpp unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp === --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -36,15 +36,15 @@ public: SymbolIndexActionFactory() = default; - clang::ASTFrontendAction *create() override { + clang::FrontendAction *create() override { index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; Collector = std::make_shared(); FrontendAction *Action = index::createIndexingAction(Collector, IndexOpts, nullptr).release(); -return llvm::cast(Action); +return Action; } std::shared_ptr Collector; Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -90,6 +90,15 @@ "Trace internal events and timestamps in chrome://tracing JSON format"), llvm::cl::init(""), llvm::cl::Hidden); +static llvm::cl::opt EnableIndexBasedCompletion( +"enable-index-based-completion", +llvm::cl::desc( +"Enable index-based global code completion (experimental). Clangd will " +"use symbols built from ASTs of opened files and additional indexes " +"(e.g. offline built codebase-wide symbol table) to complete partial " +"symbols."), +llvm::cl::init(false)); + int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); @@ -170,10 +179,15 @@ clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = EnableSnippets; CCOpts.IncludeIneligibleResults = IncludeIneligibleResults; + if (EnableIndexBasedCompletion) { +// Disable sema code completion for qualified code completion and use global +// symbol index instead. +CCOpts.IncludeNamespaceLevelDecls = false; + } // Initialize and run ClangdLSPServer. ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory, -CCOpts, ResourceDirRef, -CompileCommandsDirPath); +CCOpts, ResourceDirRef, CompileCommandsDirPath, +EnableIndexBasedCompletion, {}); constexpr int NoShutdownRequestErrorCode = 1; llvm::set_thread_name("clangd.main"); return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCode; Index: clangd/SymbolIndex.h === --- /dev/null +++ clangd/SymbolIndex.h @@ -0,0 +1,60 @@ +//===--- CompletionIndex.h - Index for code completion ---*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLINDEX_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLINDEX_H + +#include "SymbolCompletionInfo.h" +#include "llvm/Support/Error.h" + +namespace clang { +namespace clangd { + +struct CompletionRequest { + std::string Query; + size_t MaxCandidateCount = UINT_MAX; +}; + +/// \brief Signals for scoring completion candidates. +struct ScoreSignals { + // FIXME: add score signals like cross-reference count. +}; + +struct CompletionSymbol { + ScoreSignals Signals; + float SymbolScore; + + std::string USR; + index::SymbolKind Kind; + std::string QualifiedName; + + SymbolCompletionInfo CompletionInfo; +}; + +struct CompletionResult { + std::vector Symbols; + bool AllMatched; +}; + +class SymbolIndex { +public: + virtual ~SymbolIndex() = default; + + virtual llvm::Expected + complete(const CompletionRequest &Req) const = 0; + + // FIXME: add interfaces for more index use cases: + // - Symbol getSymbolInfo(llvm::StringRef USR); + // - getAllOccurrences(llvm::StringRef USR); +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOLINDEX_H Index: clangd/SymbolCompletionInfo.h ===
[PATCH] D40937: [clang-tidy] Infinite loop checker
szepet updated this revision to Diff 125965. szepet marked 9 inline comments as done. szepet added a comment. Updates based on comments. https://reviews.llvm.org/D40937 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/InfiniteLoopCheck.cpp clang-tidy/misc/InfiniteLoopCheck.h clang-tidy/misc/MiscTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/misc-infinite-loop.rst test/clang-tidy/misc-infinite-loop.cpp Index: test/clang-tidy/misc-infinite-loop.cpp === --- /dev/null +++ test/clang-tidy/misc-infinite-loop.cpp @@ -0,0 +1,114 @@ +// RUN: %check_clang_tidy %s misc-infinite-loop %t + +void simple_infinite_loop1() { + int i = 0; + int j = 0; + while (i < 10) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: The condition variable (i) is not updated in the loop body [misc-infinite-loop] +j++; + } + + do { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: The condition variable (i) is not updated in the loop body +j++; + } while (i < 10); + + for (i = 0; i < 10; ++j) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: The condition variable (i) is not updated in the loop body + } +} + +void simple_infinite_loop2() { + int i = 0; + int j = 0; + int Limit = 10; + while (i < Limit) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: None of the condition variables (i, Limit) are updated in the loop body [misc-infinite-loop] +j++; + } + + do { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: None of the condition variables (i, Limit) are updated in the loop body +j++; + } while (i < Limit); + + for (i = 0; i < Limit; ++j) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: None of the condition variables (i, Limit) are updated in the loop body + } +} + +void simple_not_infinite() { + int i = 0; + int Limit = 100; + while (i < Limit) { // Not an error since 'Limit' is updated +Limit--; + } + do { +Limit--; + } while (i < Limit); + + for (i = 0; i < Limit; Limit--) { + } +} + +void escape_before1() { + int i = 0; + int Limit = 100; + int *p = &i; + while (i < Limit) { // Not an error, since p is alias of i. +*++p; + } + + do { +*++p; + } while (i < Limit); + + for (i = 0; i < Limit; *++p) { + } +} + +void escape_before2() { + int i = 0; + int Limit = 100; + int *p = &i; + while (i < Limit) { // We do not warn since the var 'i' is escaped but it is + // an actual error, since the pointer 'p' is increased. +*(p++); + } +} + +void escape_after() { + int i = 0; + int j = 0; + int Limit = 10; + + while (i < Limit) { +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: None of the condition variables (i, Limit) are updated in the loop body + } + int *p = &i; +} + +int glob; +void glob_var(int &x) { + int i = 0; + int Limit = 100; + while (x < Limit) { // Not an error since 'x' can be an alias of glob. +glob++; + } +} + +void glob_var2() { + int i = 0, Limit = 100; + while (glob < Limit) { // Since 'glob' is declared out of the function we do not warn. +i++; + } +} + +bool foo(int n); +void fun_call() { + int i = 0; + int Limit = 100; + while (foo(i)) { // Do not warn, since foo can have state. +Limit--; + } +} Index: docs/clang-tidy/checks/misc-infinite-loop.rst === --- /dev/null +++ docs/clang-tidy/checks/misc-infinite-loop.rst @@ -0,0 +1,30 @@ +.. title:: clang-tidy - misc-infinite-loop + +misc-infinite-loop +== + +The check finds loops where none of the condition variables are updated in the +body. This performs a very conservative check in order to avoid false positives +and work only on integer types at the moment. + +Examples: + +.. code-block:: c++ + + void simple_infinite_loop() { +int i = 0; +int j = 0; +int Limit = 10; +while (i < Limit) { // Error, since none of the variables are updated. + j++; +} + } + + void escape_before() { +int i = 0; +int Limit = 100; +int *p = &i; +while (i < Limit) { // Not an error, since p is alias of i. + *++p; +} + } Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -120,6 +120,7 @@ misc-definitions-in-headers misc-forwarding-reference-overload misc-incorrect-roundings + misc-infinite-loop misc-lambda-function-name misc-macro-parentheses misc-macro-repeated-side-effects Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -158,6 +158,11 @@ Finds uses of bitwise operations on signed integer types, which may lead to undefined or implementation defined behaviour. +- New `misc-infinite-loop +
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
george.karpenkov added inline comments. Comment at: tools/scan-build-py/libscanbuild/analyze.py:44 +CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt' +CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps' gerazo wrote: > george.karpenkov wrote: > > What would happen when multiple analyzer runs are launched concurrently in > > the same directory? Would they race on this file? > Yes and no. The 1st, collect part of CTU creates this file by aggregating all > data from the build system, while the 2nd part which does the analysis itself > only reads it. Multiple analysis can use this file simultaneously without > problem. However, multiple collect phases launched on a system does not make > sense. In this case, the later one would write over the previous results with > the same data. > However, multiple collect phases launched on a system does not make sense Why not? What about a system with multiple users, where code is in the shared directory? Or even simpler: I've launched a background process, forgot about it, and then launched it again? > In this case, the later one would write over the previous results with the > same data. That is probably fine, I am more worried about racing, where process B would be reading a partially overriden file (not completely sure whether it is possible) Comment at: tools/scan-build-py/libscanbuild/analyze.py:103 +def prefix_with(constant, pieces): +""" From a sequence create another sequence where every second element gerazo wrote: > george.karpenkov wrote: > > Actually I would prefer a separate NFC PR just moving this function. This > > PR is already too complicated as it is. > Yes. The only reason was for the move to make it testable. However, we need > more tests as you wrote down below. Sure, but that separate PR can also include tests. Comment at: tools/scan-build-py/libscanbuild/analyze.py:120 + func_map_cmd=args.func_map_cmd) +if hasattr(args, 'ctu_phases') and hasattr(args.ctu_phases, 'dir') +else CtuConfig(collect=False, analyze=False, dir='', func_map_cmd='')) gerazo wrote: > george.karpenkov wrote: > > Extensive `hasattr` usage is often a codesmell, and it hinders > > understanding. > > Firstly, shouldn't `args.ctu_phases.dir` be always available, judging from > > the code in `arguments.py`? > > Secondly, in what cases `args.ctu_phases` is not available when we are > > already going down the CTU code path? Shouldn't we throw an error at that > > point instead of creating a default configuration? > > (default-configurations-instead-of-crashing is also another way to > > introduce very subtle and hard-to-catch error modes) > It definitely needs more comments, so thanks, I will put them here. > There are two separate possibilities here for this code part: > 1. The user does not want CTU at all. In this case, no CTU phases are > switched on (collect and analyze), so no CTU code will run. This is why dir > has no importance in this case. > 2. CTU capabilities were not even detected, so the help and available > arguments about CTU are also missing. In this case we create a dummy config > telling that everything is switched off. > > The reason for using hasattr was that not having CTU capabilities is not an > exceptional condition, rather a matter of configuration. I felt that handling > this with an exception would be misleading. Right, but instead of doing (1) and (2) can we have a separate (maybe hidden from user) param called e.g. `ctu_enabled` which would explicitly communicate that? Comment at: tools/scan-build-py/libscanbuild/analyze.py:144 +if len(ast_files) == 1: +mangled_ast_pairs.append((mangled_name, ast_files.pop())) + gerazo wrote: > george.karpenkov wrote: > > Firstly, no need to modify the set in order to get the first element, just > > use `next(iter(ast_files))`. > > Secondly, when exactly do conflicting names happen? Is it a bug? Shouldn't > > the tool log such cases? > Nice catch, thanks. > For your second question: Unfortunately, it is not a bug, rather a misfeature > which we can't handle yet. There can be tricky build-systems where a single > file with different configurations is built multiple times, so the same > function signature will be present in multiple link units. The other option > is when two separate compile units have an exact same function signature, but > they are never linked together, so it is not a problem in the build itself. > In this case, the cross compilation unit functionality cannot tell exactly > which compilation unit to turn to for the function, because there are > multiple valid choices. In this case, we deliberately leave such functions > out to avoid potential problems. It deserves a comment I think. > In this case, we deliberately leave such functions out to avoid potential > problems W
[PATCH] D40937: [clang-tidy] Infinite loop checker
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/misc-infinite-loop.rst:6 + +The check finds loops where none of the condition variables are updated in the +body. This performs a very conservative check in order to avoid false positives Please synchronize with release notes. https://reviews.llvm.org/D40937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40937: [clang-tidy] Infinite loop checker
szepet added a comment. In https://reviews.llvm.org/D40937#947760, @JVApen wrote: > How does this check deal with atomic members? > ... This patch only works on integer types. So, since the atomic is a non-supported type the check will skip that `while` loop. https://reviews.llvm.org/D40937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40937: [clang-tidy] Infinite loop checker
xazax.hun added a comment. This does not support memberExprs as condition variables right now. What happens if you have something like this: struct X { void f(int a) { while(a < i) { --i; } } int i; }; I think you could extend the test cases with some classes. Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:28 +void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) { + const auto loopCondition = []() { +return allOf(hasCondition(expr().bind("condition")), Do you need this to be a lambda? Can't you just use a local variable? Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:31 + anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")), + hasAncestor(functionDecl().bind("containing-func"))), + unless(hasBody(hasDescendant(loopEndingStmt(); Maybe this is not too important, but you might also want to check for blocks here. Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:48 +declRefExpr(to(varDecl(VarNodeMatcher)), + binaryOperator(anyOf(hasOperatorName("="), hasOperatorName("+="), + hasOperatorName("/="), hasOperatorName("*="), This can be greatly simplified once https://reviews.llvm.org/D38921 is accepted. Maybe you could use that as a dependent revision? It should be close to be accepted. https://reviews.llvm.org/D40937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40937: [clang-tidy] Infinite loop checker
xazax.hun added inline comments. Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:121 + + Stmt *FunctionBody = nullptr; + if (ContainingLambda) This could be pointer to const, right? https://reviews.llvm.org/D40937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40939: [analyzer] Avoid element regions of void type.
NoQ added a comment. I think the new behavior is correct in the sense that in our region hierarchy byte offsets (such as arithmetic on void pointers) are normally represented as `char`-type element regions. For instance, we have a similar mechanism is implemented in pointer casts case, when the byte offset of the pointer is not divisible by the casted object size: we just add a character element region to account for the remainder of the offset. Like, it's not the situation when we couldn't figure out the type - it would have been null in that case. Here we know exactly that the type is void. And when you add N to a void pointer, it changes by N bytes, which is what we should, in my opinion, try to represent somehow. So i believe that the newly constructed `SVal`s are correctly representing their respective expressions or arithmetic results, and i'm not immediately seeing a better behavior. Repository: rC Clang https://reviews.llvm.org/D40939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40939: [analyzer] Avoid element regions of void type.
xazax.hun added a comment. In https://reviews.llvm.org/D40939#948252, @NoQ wrote: > Like, it's not the situation when we couldn't figure out the type - it would > have been null in that case. Here we know exactly that the type is void. Oh, thank you for the clarification! Repository: rC Clang https://reviews.llvm.org/D40939 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40562: [Sema] Ignore decls in namespaces when global decls are not wanted.
ioeric added a comment. Ping. (fyi, you could find use of the new option in https://reviews.llvm.org/D40548) Repository: rC Clang https://reviews.llvm.org/D40562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40563: [SemaCodeComplete] Allow passing out scope specifiers in qualified-id completions via completion context.
ioeric added a comment. Fyi, you could find use of the new API in https://reviews.llvm.org/D40548 I'd like to land this patch if there is no objection. https://reviews.llvm.org/D40563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40939: [analyzer] Avoid element regions of void type.
NoQ updated this revision to Diff 125972. NoQ added a comment. Rebase on top of https://reviews.llvm.org/D40584. Stronger assertion for `CXXThisRegion` type. Uhm, forgot to mention that i also added a similar assertion to `CXXThisRegion` (which didn't find any bugs yet). Other typed value regions don't store types directly. https://reviews.llvm.org/D40939 Files: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -988,6 +988,12 @@ elementType = resultTy->getPointeeType(); } +// Represent arithmetic on void pointers as arithmetic on char pointers. +// It is fine when a TypedValueRegion of char value type represents +// a void pointer. +if (elementType->isVoidType()) + elementType = getContext().CharTy; + if (Optional indexV = index.getAs()) { return loc::MemRegionVal(MemMgr.getElementRegion(elementType, *indexV, superR, getContext())); Index: lib/StaticAnalyzer/Core/ExprEngine.cpp === --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2175,9 +2175,16 @@ ProgramStateRef state = Node->getState(); if (IsGLValueLike) { - SVal V = state->getLValue(A->getType(), - state->getSVal(Idx, LCtx), - state->getSVal(Base, LCtx)); + QualType T = A->getType(); + + // One of the forbidden LValue types! We still need to have sensible + // symbolic lvalues to represent this stuff. + if (T->isVoidType()) +T = getContext().CharTy; + + SVal V = state->getLValue(T, +state->getSVal(Idx, LCtx), +state->getSVal(Base, LCtx)); Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr, ProgramPoint::PostLValueKind); } else if (IsVectorType) { Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h === --- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -961,7 +961,10 @@ CXXThisRegion(const PointerType *thisPointerTy, const StackArgumentsSpaceRegion *sReg) : TypedValueRegion(sReg, CXXThisRegionKind), -ThisPointerTy(thisPointerTy) {} +ThisPointerTy(thisPointerTy) { +assert(ThisPointerTy->getPointeeType()->getAsCXXRecordDecl() && + "Invalid region type!"); + } static void ProfileRegion(llvm::FoldingSetNodeID &ID, const PointerType *PT, @@ -1075,6 +1078,8 @@ assert((!Idx.getAs() || Idx.castAs().getValue().isSigned()) && "The index must be signed"); +assert(!elementType.isNull() && !elementType->isVoidType() && + "Invalid region type!"); } static void ProfileRegion(llvm::FoldingSetNodeID& ID, QualType elementType, Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp === --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -988,6 +988,12 @@ elementType = resultTy->getPointeeType(); } +// Represent arithmetic on void pointers as arithmetic on char pointers. +// It is fine when a TypedValueRegion of char value type represents +// a void pointer. +if (elementType->isVoidType()) + elementType = getContext().CharTy; + if (Optional indexV = index.getAs()) { return loc::MemRegionVal(MemMgr.getElementRegion(elementType, *indexV, superR, getContext())); Index: lib/StaticAnalyzer/Core/ExprEngine.cpp === --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2175,9 +2175,16 @@ ProgramStateRef state = Node->getState(); if (IsGLValueLike) { - SVal V = state->getLValue(A->getType(), - state->getSVal(Idx, LCtx), - state->getSVal(Base, LCtx)); + QualType T = A->getType(); + + // One of the forbidden LValue types! We still need to have sensible + // symbolic lvalues to represent this stuff. + if (T->isVoidType()) +T = getContext().CharTy; + + SVal V = state->getLValue(T, +state->getSVal(Idx, LCtx), +state->getSVal(Base, LCtx)); Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr, ProgramPoint::PostLValueKin
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
malaperle added a comment. In https://reviews.llvm.org/D40897#946911, @hokein wrote: > Our rough plan would be > > 1. Define the Symbol structure. > 2. Design the interfaces of SymbolIndex, ASTIndex. > 3. Combine 1) and 2) together to make global code completion work (we'd use > YAML dataset for LLVM project, note that this is not a final solution, it > would be hidden in an `--experimental` flag). > 4. Switch to use the dataset from index-while-building when it is ready. Thanks for the explanation. On my end, the plan is not quite sequential. The branch I am developing has interfaces for querying, building and a dataset format, let's call it ClangdIndexDataStorage, which is different from index-while-building (libIndexStore). I also have a version that uses libIndexStore through the same interfaces. So with that in mind, there are too main activities: 1. Work towards the interfaces for using the index (this patch, and Eric's). From my perspective, I will make sure that it can be as compatible as possible with reading the index from disk and the features we want to develop. One important aspect is to have a good balance between memory and performance. In Eclipse CDT and also the branch I work on using ClangdIndexDataStorage, the emphasis was to minimize memory consumption and have a configurable cache size. But different choices could be made here, perhaps I can start a discussion about that separately. 2. Work on index-while-building or the other format getting adopted in Clangd. The index-while-building (libIndexStore) is promising but also has a few missing pieces. We need a mapping solution (LMDB equivalent). We also need to make sure it's fast enough and contain enough information for the features we will need, etc. Comment at: clangd/Symbol.h:23 + // The path of the source file where a symbol occurs. + std::string FilePath; + // The offset to the first character of the symbol from the beginning of the sammccall wrote: > ioeric wrote: > > Is this relative or absolute? > Having every symbol own a copy of the filepath seems wasteful. > It seems likely that the index will have per-file information too, so this > representation should likely be a key to that. Hash of the filepath might > work? How we model it is that a symbol doesn't have a "location", but its occurrence do. One could consider the location of a symbol to be either its declaration occurrence (SymbolRole::Declaration) or its definition (SymbolRole::Definition). What we do to get the location path is each occurrence has a pointer (a "database" pointer, but it doesn't matter) to a file entry and then we get the path from the entry. So conceptually, it works a bit like this (although it fetches information on disk). ``` class IndexOccurrence { IndexOccurrence *FilePtr; std::string Occurrence::getPath() { return FilePtr->getPath(); } }; ``` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D40948: Switch Clang's default C++ language target to C++14
> + LangStd = LangStandard::lang_gnucxx14; > > This comment should be removed. Opps, yep. Fixed on my local branch. I won't upload a new diff just yet though. Thanks. Tim. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.
sammccall added inline comments. Comment at: clangd/ClangdIndex.h:24 +/// (re-)scoring symbols from different indexes. +class CombinedSymbolIndex : public SymbolIndex { +public: This seems a little premature to me - it's hard to know if this idea makes sense without actually having multiple implementations to mix together. My gut feeling is that sorting by (index weight, symbol score) puts too much emphasis on the weight. Can we just have a single optional index for now? Comment at: clangd/ClangdIndex.h:36 + + void addSymbolIndex(llvm::StringRef Label, WeightedIndex Index); + If you do keep this class, move the label inside the struct? Avoids a lot of std::pair Comment at: clangd/ClangdIndex.h:50 +// relatively small symbol table built offline. +class InMemoryIndexSourcer { +public: As discussed offline - the lifetime of the contained symbols and the operations on the index derived from it need to be carefully considered. Involving inheritance here seems likely to make these tricky interactions even trickier. Would suggest: - a concrete class (`SymbolCache`? `FileSymbols`?) to hold a collection of symbols from multiple files, with the ability to iterate over them and replace all the symbols from one file at once - a concrete class `MemIndex` that can be constructed from a sequence of symbols and implements `Index` You probably want to make them both immutable with snapshot semantics, or have a reader-writer lock that spans both. Comment at: clangd/ClangdIndex.h:72 + /// collected. + void update(PathRef Path, ASTContext &Context, + std::shared_ptr PP, The AST -> symbol conversion doesn't seem to have much to do with the rest of the class - we can move this to a free function I think, which would give the class a narrower responsibility. Comment at: clangd/ClangdIndex.h:1 +//===--- ClangdIndex.h - Symbol indexes for clangd.---*- C++-*-===// +// nit: `Clangd` prefix doesn't do much. I'd suggest `Index.h` for the interface (including Symbol), and `MemIndex.h` for the in-memory implementations? Comment at: clangd/ClangdLSPServer.h:37 + llvm::Optional CompileCommandsDir, + std::vector< + std::pair> ilya-biryukov wrote: > Maybe pass a parameter of type `SymbolIndex&` instead of a vector, which is > used to create `CombinedSymbolIndex` later? > It seems that `ClangdServer` does not really care about the specific index > implementation that's used (nor should it!) > > We could have helper methods to conveniently create `CombinedSymbolIndex` > from a list of indexes, or even create the default index for clangd. I think the issue is that ClangdServer is going to create and maintain the dynamic index, which should then be merged as a peer with a set of other indexes. Can we just pass in a single SymbolIndex* StaticIdx, and hard-code the assumption about static index + dynamic index being what we're using? That way we can also hard-code the heuristics for merging them together, instead of data-driving everything. Comment at: clangd/ClangdLSPServer.h:38 + llvm::Optional CompileCommandsDir, + bool EnableIndexBasedCodeCompletion, + std::vector< this is kind of two options in one - "should we build the index" and "should we query the index". That seems OK, but I'd consider renaming this to BuildDynamicSymbolIndex or something to make it clear that building is optional, querying it isn't provided as an option Comment at: clangd/CodeComplete.cpp:378 +// FIXME: output a warning to logger if there are results from sema. +return qualifiedIdCompletionWithIndex(*Index, S, **OptSS, &Items); + } I don't like the idea of doing index lookups from the middle of a sema callback! Can we just record the CCContext in the Collector instead, and do this work afterwards? Comment at: clangd/CodeComplete.h:71 /// Get code completions at a specified \p Pos in \p FileName. +/// If `Index` is not nullptr, this will use the index for global code +/// completion; otherwise, use sema code completion by default. I'm not sure you want to spell out this behavior - merging vs replacing is more of an implementation detail Comment at: clangd/SymbolIndex.h:45 + +class SymbolIndex { +public: Interfaces need doc :-) Comment at: clangd/SymbolIndex.h:49 + + virtual llvm::Expected + complete(const CompletionRequest &Req) const = 0; As discussed, we may want a callback-based API that makes it easier for an implementation to manage the lifetime of symbols without copying them. Also is there anything to do with errors other than log and treat as no
[PATCH] D40752: [OpenMP] Fix assert fail after target implicit map checks
jdenny added a comment. Alexey: I see that you committed the error message change, so I think this issue is done. Is Abandon Revision correct in this scenario? Sorry, I'm new here. https://reviews.llvm.org/D40752 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40752: [OpenMP] Fix assert fail after target implicit map checks
ABataev added a comment. In https://reviews.llvm.org/D40752#948399, @jdenny wrote: > Alexey: I see that you committed the error message change, so I think this > issue is done. Is Abandon Revision correct in this scenario? Sorry, I'm new > here. Yes, it is correct https://reviews.llvm.org/D40752 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40897: [clangd] Introduce a "Symbol" class.
malaperle added inline comments. Comment at: clangd/Symbol.h:37 +// The class presents a C++ symbol, e.g. class, function. +struct Symbol { + // The symbol identifier, using USR. sammccall wrote: > hokein wrote: > > malaperle wrote: > > > I think it would be nice to have methods as an interface to get this data > > > instead of storing them directly. So that an index-on-disk could go fetch > > > the data. Especially the occurrences which can take a lot of memory (I'm > > > working on a branch that does that). But perhaps defining that interface > > > is not within the scope of this patch and could be better discussed in > > > D40548 ? > > I agree. We can't load all the symbol occurrences into the memory since > > they are too large. We need to design interface for the symbol occurrences. > > > > We could discuss the interface here, but CodeCompletion is the main thing > > which this patch focuses on. > > We can't load all the symbol occurrences into the memory since they are too > > large > > I've heard this often, but never backed up by data :-) > > Naively an array of references for a symbol could be doc ID + offset + > length, let's say 16 bytes. > > If a source file consisted entirely of references to 1-character symbols > separated by punctuation (1 reference per 2 bytes) then the total size of > these references would be 8x the size of the source file - in practice much > less. That's not very big. > > (Maybe there are edge cases with macros/templates, but we can keep them under > control) I'd have to break down how much memory it used by what, I'll come back to you on that. Indexing llvm with ClangdIndexDataStorage, which is pretty packed is about 200MB. That's already a lot considering we want to index code bases many times bigger. But I'll try to come up with more precise numbers. I'm open to different strategies. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40968: [OpenMP] Diagnose function name on the link clause
kkwli0 created this revision. This patch is to add diagnose when a function name is specified on the link clause. According to the OpenMP spec, only the list items that exclude the function name are allowed on the link clause. void foo() {} #pragma omp declare target link(foo) d2.c:2:33: error: function name is not allowed in 'link' clause #pragma omp declare target link(foo) ^ d2.c:1:6: note: 'foo' defined here void foo() {} ^ 1 error generated. https://reviews.llvm.org/D40968 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaOpenMP.cpp test/OpenMP/declare_target_ast_print.cpp test/OpenMP/declare_target_messages.cpp Index: test/OpenMP/declare_target_messages.cpp === --- test/OpenMP/declare_target_messages.cpp +++ test/OpenMP/declare_target_messages.cpp @@ -19,6 +19,10 @@ void c(); // expected-warning {{declaration is not declared in any declare target region}} +void func() {} // expected-note {{'func' defined here}} + +#pragma omp declare target link(func) // expected-error {{function name is not allowed in 'link' clause}} + extern int b; struct NonT { Index: test/OpenMP/declare_target_ast_print.cpp === --- test/OpenMP/declare_target_ast_print.cpp +++ test/OpenMP/declare_target_ast_print.cpp @@ -108,9 +108,7 @@ // CHECK: #pragma omp end declare target int c1, c2, c3; -void f3() { -} -#pragma omp declare target link(c1) link(c2), link(c3, f3) +#pragma omp declare target link(c1) link(c2), link(c3) // CHECK: #pragma omp declare target link // CHECK: int c1; // CHECK: #pragma omp end declare target @@ -120,9 +118,6 @@ // CHECK: #pragma omp declare target link // CHECK: int c3; // CHECK: #pragma omp end declare target -// CHECK: #pragma omp declare target link -// CHECK: void f3() -// CHECK: #pragma omp end declare target struct SSSt { #pragma omp declare target Index: lib/Sema/SemaOpenMP.cpp === --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -12573,6 +12573,11 @@ if (!SameDirectiveDecls.insert(cast(ND->getCanonicalDecl( Diag(Id.getLoc(), diag::err_omp_declare_target_multiple) << Id.getName(); +if (MT == OMPDeclareTargetDeclAttr::MT_Link && isa(ND)) { + Diag(Id.getLoc(), diag::err_omp_function_in_link_clause); + Diag(ND->getLocation(), diag::note_defined_here) << ND; +} + if (!ND->hasAttr()) { Attr *A = OMPDeclareTargetDeclAttr::CreateImplicit(Context, MT); ND->addAttr(A); Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -8709,6 +8709,8 @@ def warn_omp_not_in_target_context : Warning< "declaration is not declared in any declare target region">, InGroup; +def err_omp_function_in_link_clause : Error< + "function name is not allowed in 'link' clause">; def err_omp_aligned_expected_array_or_ptr : Error< "argument of aligned clause should be array" "%select{ or pointer|, pointer, reference to array or reference to pointer}1" Index: test/OpenMP/declare_target_messages.cpp === --- test/OpenMP/declare_target_messages.cpp +++ test/OpenMP/declare_target_messages.cpp @@ -19,6 +19,10 @@ void c(); // expected-warning {{declaration is not declared in any declare target region}} +void func() {} // expected-note {{'func' defined here}} + +#pragma omp declare target link(func) // expected-error {{function name is not allowed in 'link' clause}} + extern int b; struct NonT { Index: test/OpenMP/declare_target_ast_print.cpp === --- test/OpenMP/declare_target_ast_print.cpp +++ test/OpenMP/declare_target_ast_print.cpp @@ -108,9 +108,7 @@ // CHECK: #pragma omp end declare target int c1, c2, c3; -void f3() { -} -#pragma omp declare target link(c1) link(c2), link(c3, f3) +#pragma omp declare target link(c1) link(c2), link(c3) // CHECK: #pragma omp declare target link // CHECK: int c1; // CHECK: #pragma omp end declare target @@ -120,9 +118,6 @@ // CHECK: #pragma omp declare target link // CHECK: int c3; // CHECK: #pragma omp end declare target -// CHECK: #pragma omp declare target link -// CHECK: void f3() -// CHECK: #pragma omp end declare target struct SSSt { #pragma omp declare target Index: lib/Sema/SemaOpenMP.cpp === --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -12573,6 +12573,11 @@ if (!SameDirectiveDecls.insert(cast(ND->getCanonicalDecl( Diag(Id.getLoc(), diag::err_omp_declare_target_multiple) << Id.getName(); +if (MT == OMPDeclareTarge
[PATCH] D39050: Add index-while-building support to Clang
malaperle added inline comments. Comment at: include/indexstore/IndexStoreCXX.h:84 + + std::pair getLineCol() { +unsigned line, col; ioeric wrote: > nathawes wrote: > > malaperle wrote: > > > From what I understand, this returns the beginning of the occurrence. It > > > would be useful to also have the end of the occurrence. From what I > > > tested in Xcode, when you do "Find Selected Symbol in Workspace", it > > > highlights the symbol name in yellow in the list of matches, so it mush > > > use that LineCol then highlight the matching name. This is works in many > > > situations but others occurrences won't have the name of the symbol. For > > > example: > > > "MyClass o1, o2;" > > > If I use "Find Selected Symbol in Workspace" on MyClass constructor, if > > > won't be able to highlight o1 and o2 > > > Do you think it would be possible to add that (EndLineCol)? If not, how > > > would one go about extending libindexstore in order to add additional > > > information per occurrence? It is not obvious to me so far. > > > We also need other things, for example in the case of definitions, we > > > need the full range of the definition so that users can "peek" at > > > definitions in a pop-up. Without storing the end of the definition in the > > > index, we would need to reparse the file. > > > > > Our approach to related locations (e.g. name, signature, body, and doc > > comment start/end locs) has been to not include them in the index and > > derive them from the start location later. There's less data to collect and > > write out during the build that way, and deriving the other locations isn't > > that costly usually, as in most cases you 1) don't need to type check or > > even preprocess to get the related locations, as with finding the end of o1 > > and o2 in your example, and 2) usually only need to derive locations for a > > single or small number of occurrences, like when 'peeking' at a definition. > > > > Are there cases where you think this approach won't work/perform well > > enough for the indexer-backed queries clangd needs to support? > > > I agree that we should try to keep the serialized symbol size minimal, but I > think it's worthwhile to also store end locations for occurrences because 1) > it's cheap, 2) it's not necessary easy to compute without parsing for > occurrences like `a::b::c` or `a::b`, 3) it would be useful for many LSP > use cases. There's a few reason I think it's better to store the end loc. - When doing "find references", computing the end loc of each occurrence will be costly. Imagine having thousands of occurrences and for each of them having to run logic to find the end of the occurrence. - The AST and preprocessor are the best tools I know to figure out the proper end loc. Not using them means having to write a mini-preprocessor with some knowledge about the language semantics to cover some cases. ``` MyClass |o1, o2; ``` Here, I have to stop at the comma. So it's basically take any alpha-numerical character, right? ``` bool operator<(const Foo&, const Foo&) Ret operator()(Params ...params) ``` No, in those cases, we have to take < and the first (). - In the case of body start/end locations, similarly, it can be non-trivial. ``` void foo() { if (0) { } } ``` We have to count the balanced { } until we finish the body. ``` #define FUNC_BODY {\ \ } void foo() FUNC_BODY ``` Oops, where's the body? We need another special logic for this, etc. I think overall, it puts a lot of burden on the client of libIndexStore, burden that would be much more work and more inaccurate than using the AST/Preprocessor while indexing. Comment at: include/indexstore/IndexStoreCXX.h:374 + + enum class DependencyKind { +Unit, nathawes wrote: > malaperle wrote: > > As part of this dependency tracking mechanism, I haven't found that it > > could provide information about about the files including a specific > > header. So given a header (or source file in odd cases), I do not see a way > > to get all the files that would need to be reindexed if that header > > changed. Is that something you achieve outside the index? Or perhaps this > > is something I missed in the code. > The unit files store the path of the header/source files they depend on as > 'File' dependencies. So any unit file with 'File' dependency on header/source > file that was modified may need to be re-indexed. > > To support finding which specific files include or are included by a given > header (rather than which units somehow transitively include it) we also > store the file-to-file inclusions in the unit file (retrieved via > IndexUnitReader's foreachInclude method below). Thanks! I'll play around with this a bit more with this new information. Comment at: include/indexstore/IndexStoreCXX.h:377 +Record, +File, + }; nathawes wrote: > malaperle wrote: >
[PATCH] D40968: [OpenMP] Diagnose function name on the link clause
ABataev added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:12576 +if (MT == OMPDeclareTargetDeclAttr::MT_Link && isa(ND)) { + Diag(Id.getLoc(), diag::err_omp_function_in_link_clause); I would like to see this some in `check` functions, like `checkDeclIsAllowedInOpenMPTarget` rather than here https://reviews.llvm.org/D40968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39050: Add index-while-building support to Clang
akyrtzi added a comment. @malaperle, to clarify we are not suggesting that you write your own parser, the suggestion is to use clang in 'fast-scan' mode to get the structure of the declarations of a single file, see `CXTranslationUnit_SingleFileParse` (along with enabling skipping of bodies). We have found clang is super fast when you only try to get the structure of a file like this. We can make convenient APIs to provide the syntactic structure of declarations based on their location. But let's say we added the end-loc, is it enough ? If you want to implement the 'peek the definition' like Eclipse, then it is not enough, you also need to figure out if there are documentation comments associated with the declaration and also show those. Also what if you want to highlight the type signature of a function, then just storing the location of the closing brace of its body is not enough. There can be any arbitrary things you may want to get from the structure of the declaration (e.g. the parameter ranges), but we could provide an API to gather any syntactic structure info you may want. I would encourage you to try `CXTranslationUnit_SingleFileParse|CXTranslationUnit_SkipFunctionBodies`, you will be pleasantly surprised with how fast this mode is. The `c-index-test` option is `-single-file-parse`. https://reviews.llvm.org/D39050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320073 - [driver] Set the 'simulator' environment for Darwin when compiling for
Author: arphaman Date: Thu Dec 7 11:04:10 2017 New Revision: 320073 URL: http://llvm.org/viewvc/llvm-project?rev=320073&view=rev Log: [driver] Set the 'simulator' environment for Darwin when compiling for iOS/tvOS/watchOS simulator rdar://35135215 Differential Revision: https://reviews.llvm.org/D40682 Modified: cfe/trunk/include/clang/Driver/ToolChain.h cfe/trunk/lib/CodeGen/CGObjCMac.cpp cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/lib/Driver/ToolChains/Darwin.cpp cfe/trunk/lib/Driver/ToolChains/Darwin.h cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/darwin-sdkroot.c cfe/trunk/test/Driver/darwin-simulator-macro.c cfe/trunk/test/Driver/darwin-version.c cfe/trunk/test/Driver/fsanitize.c Modified: cfe/trunk/include/clang/Driver/ToolChain.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=320073&r1=320072&r2=320073&view=diff == --- cfe/trunk/include/clang/Driver/ToolChain.h (original) +++ cfe/trunk/include/clang/Driver/ToolChain.h Thu Dec 7 11:04:10 2017 @@ -93,7 +93,7 @@ public: private: const Driver &D; - const llvm::Triple Triple; + llvm::Triple Triple; const llvm::opt::ArgList &Args; // We need to initialize CachedRTTIArg before CachedRTTIMode const llvm::opt::Arg *const CachedRTTIArg; @@ -136,6 +136,8 @@ protected: ToolChain(const Driver &D, const llvm::Triple &T, const llvm::opt::ArgList &Args); + void setTripleEnvironment(llvm::Triple::EnvironmentType Env); + virtual Tool *buildAssembler() const; virtual Tool *buildLinker() const; virtual Tool *getTool(Action::ActionClass AC) const; Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=320073&r1=320072&r2=320073&view=diff == --- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original) +++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Thu Dec 7 11:04:10 2017 @@ -4885,10 +4885,7 @@ void CGObjCCommonMac::EmitImageInfo() { } // Indicate whether we're compiling this to run on a simulator. - const llvm::Triple &Triple = CGM.getTarget().getTriple(); - if ((Triple.isiOS() || Triple.isWatchOS()) && - (Triple.getArch() == llvm::Triple::x86 || - Triple.getArch() == llvm::Triple::x86_64)) + if (CGM.getTarget().getTriple().isSimulatorEnvironment()) Mod.addModuleFlag(llvm::Module::Error, "Objective-C Is Simulated", eImageInfo_ImageIsSimulated); Modified: cfe/trunk/lib/Driver/ToolChain.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=320073&r1=320072&r2=320073&view=diff == --- cfe/trunk/lib/Driver/ToolChain.cpp (original) +++ cfe/trunk/lib/Driver/ToolChain.cpp Thu Dec 7 11:04:10 2017 @@ -81,6 +81,12 @@ ToolChain::ToolChain(const Driver &D, co getFilePaths().push_back(CandidateLibPath); } +void ToolChain::setTripleEnvironment(llvm::Triple::EnvironmentType Env) { + Triple.setEnvironment(Env); + if (EffectiveTriple != llvm::Triple()) +EffectiveTriple.setEnvironment(Env); +} + ToolChain::~ToolChain() { } Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=320073&r1=320072&r2=320073&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Thu Dec 7 11:04:10 2017 @@ -1214,14 +1214,6 @@ void Darwin::AddDeploymentTarget(Derived if (iOSVersion) ExplicitIOSDeploymentTargetStr = iOSVersion->getAsString(Args); - // Add a macro to differentiate between m(iphone|tv|watch)os-version-min=X.Y and - // -m(iphone|tv|watch)simulator-version-min=X.Y. - if (Args.hasArg(options::OPT_mios_simulator_version_min_EQ) || - Args.hasArg(options::OPT_mtvos_simulator_version_min_EQ) || - Args.hasArg(options::OPT_mwatchos_simulator_version_min_EQ)) -Args.append(Args.MakeSeparateArg(nullptr, Opts.getOption(options::OPT_D), - " __APPLE_EMBEDDED_SIMULATOR__=1")); - if (OSXVersion && (iOSVersion || TvOSVersion || WatchOSVersion)) { getDriver().Diag(diag::err_drv_argument_not_allowed_with) << OSXVersion->getAsString(Args) Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.h?rev=320073&r1=320072&r2=320073&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Darwin.h (original) +++ cfe/trunk/lib/Driver/ToolChains/Darwin.h Thu Dec 7 11:04:10 2017 @@ -329,6 +329,9 @@ protected: TargetInitialized = true; Target
[clang-tools-extra] r320074 - [clangd-fuzzer] Update contruction of LSPServer.
Author: morehouse Date: Thu Dec 7 11:04:27 2017 New Revision: 320074 URL: http://llvm.org/viewvc/llvm-project?rev=320074&view=rev Log: [clangd-fuzzer] Update contruction of LSPServer. The constructor for ClangdLSPServer changed in r318412 and r318925, breaking the clangd-fuzzer build. Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp?rev=320074&r1=320073&r2=320074&view=diff == --- clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp (original) +++ clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Thu Dec 7 11:04:27 2017 @@ -13,16 +13,19 @@ /// //===--===// +#include "CodeComplete.h" #include "ClangdLSPServer.h" #include extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) { clang::clangd::JSONOutput Out(llvm::nulls(), llvm::nulls(), nullptr); + clang::clangd::CodeCompleteOptions CCOpts; + CCOpts.EnableSnippets = false; // Initialize and run ClangdLSPServer. clang::clangd::ClangdLSPServer LSPServer( Out, clang::clangd::getDefaultAsyncThreadsCount(), - /*EnableSnippets=*/false, llvm::None, llvm::None); + /*StorePreamblesInMemory=*/false, CCOpts, llvm::None, llvm::None); std::istringstream In(std::string(reinterpret_cast(data), size)); LSPServer.run(In); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40938: update hwasan docs
kcc updated this revision to Diff 126001. kcc added a comment. mention https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt Repository: rC Clang https://reviews.llvm.org/D40938 Files: docs/HardwareAssistedAddressSanitizerDesign.rst Index: docs/HardwareAssistedAddressSanitizerDesign.rst === --- docs/HardwareAssistedAddressSanitizerDesign.rst +++ docs/HardwareAssistedAddressSanitizerDesign.rst @@ -1,9 +1,9 @@ -= -HardwareAssistedAddressSanitizer Design Documentation -= +=== +Hardware-assisted AddressSanitizer Design Documentation +=== This page is a design document for -**HardwareAssistedAddressSanitizer** (or HWASAN) +**hardware-assisted AddressSanitizer** (or **HWASAN**) a tool similar to :doc:`AddressSanitizer`, but based on partial hardware assistance. @@ -23,7 +23,7 @@ AArch64 has the `Address Tagging`_, a hardware feature that allows software to use 8 most significant bits of a 64-bit pointer as -a tag. HardwareAssistedAddressSanitizer uses `Address Tagging`_ +a tag. HWASAN uses `Address Tagging`_ to implement a memory safety tool, similar to :doc:`AddressSanitizer`, but with smaller memory overhead and slightly different (mostly better) accuracy guarantees. @@ -77,11 +77,26 @@ Errors are generated by `__builtin_trap` and are handled by a signal handler. +Attribute +- + +HWASAN uses it's own LLVM IR Attribute `sanitize_hwaddress` and a matching +C function attribute. An alternative would be to re-use ASAN's attribute +`sanitize_address`. The reasons to use a separate attribute are: + + * Users may need to disable ASAN but not HWASAN, or vise versa, +because the tools have different trade-offs and compatibility issues. + * LLVM (ideally) does not use flags to decide which pass is being used, +ASAN or HWASAN are being applied, based on the function attributes. + +This does mean that users of HWASAN may need to add the new attribute +to the code that already uses the old attribute. + Comparison with AddressSanitizer -HardwareAssistedAddressSanitizer: +HWASAN: * Is less portable than :doc:`AddressSanitizer` as it relies on hardware `Address Tagging`_ (AArch64). Address Tagging can be emulated with compiler instrumentation, @@ -91,15 +106,16 @@ * May have compatibility problems if the target code uses higher pointer bits for other purposes. * May require changes in the OS kernels (e.g. Linux seems to dislike -tagged pointers passed from address space). +tagged pointers passed from address space: +https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt). * **Does not require redzones to detect buffer overflows**, but the buffer overflow detection is probabilistic, with roughly `(2**K-1)/(2**K)` probability of catching a bug. * **Does not require quarantine to detect heap-use-after-free, or stack-use-after-return**. The detection is similarly probabilistic. -The memory overhead of HardwareAssistedAddressSanitizer is expected to be much smaller +The memory overhead of HWASAN is expected to be much smaller than that of AddressSanitizer: `1/N` extra memory for the shadow and some overhead due to `N`-aligning all objects. Index: docs/HardwareAssistedAddressSanitizerDesign.rst === --- docs/HardwareAssistedAddressSanitizerDesign.rst +++ docs/HardwareAssistedAddressSanitizerDesign.rst @@ -1,9 +1,9 @@ -= -HardwareAssistedAddressSanitizer Design Documentation -= +=== +Hardware-assisted AddressSanitizer Design Documentation +=== This page is a design document for -**HardwareAssistedAddressSanitizer** (or HWASAN) +**hardware-assisted AddressSanitizer** (or **HWASAN**) a tool similar to :doc:`AddressSanitizer`, but based on partial hardware assistance. @@ -23,7 +23,7 @@ AArch64 has the `Address Tagging`_, a hardware feature that allows software to use 8 most significant bits of a 64-bit pointer as -a tag. HardwareAssistedAddressSanitizer uses `Address Tagging`_ +a tag. HWASAN uses `Address Tagging`_ to implement a memory safety tool, similar to :doc:`AddressSanitizer`, but with smaller memory overhead and slightly different (mostly better) accuracy guarantees. @@ -77,11 +77,26 @@ Errors are generated by `__builtin_trap` and are handled by a signal handler. +Attribute +- + +HWASAN uses it's own LLVM IR Attribute `sanitize_hwaddress` and a matching +C function attribute. An alternativ
[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used
This revision was automatically updated to reflect the committed changes. Closed by commit rC320073: [driver] Set the 'simulator' environment for Darwin when compiling for (authored by arphaman). Repository: rC Clang https://reviews.llvm.org/D40682 Files: include/clang/Driver/ToolChain.h lib/CodeGen/CGObjCMac.cpp lib/Driver/ToolChain.cpp lib/Driver/ToolChains/Darwin.cpp lib/Driver/ToolChains/Darwin.h lib/Frontend/InitPreprocessor.cpp test/Driver/darwin-sdkroot.c test/Driver/darwin-simulator-macro.c test/Driver/darwin-version.c test/Driver/fsanitize.c Index: include/clang/Driver/ToolChain.h === --- include/clang/Driver/ToolChain.h +++ include/clang/Driver/ToolChain.h @@ -93,7 +93,7 @@ private: const Driver &D; - const llvm::Triple Triple; + llvm::Triple Triple; const llvm::opt::ArgList &Args; // We need to initialize CachedRTTIArg before CachedRTTIMode const llvm::opt::Arg *const CachedRTTIArg; @@ -136,6 +136,8 @@ ToolChain(const Driver &D, const llvm::Triple &T, const llvm::opt::ArgList &Args); + void setTripleEnvironment(llvm::Triple::EnvironmentType Env); + virtual Tool *buildAssembler() const; virtual Tool *buildLinker() const; virtual Tool *getTool(Action::ActionClass AC) const; Index: test/Driver/darwin-simulator-macro.c === --- test/Driver/darwin-simulator-macro.c +++ test/Driver/darwin-simulator-macro.c @@ -4,9 +4,8 @@ // RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mappletvsimulator-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=SIM %s // RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mwatchos-simulator-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=SIM %s // RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mwatchsimulator-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=SIM %s -// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mios-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=DEV %s -// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mtvos-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=DEV %s -// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mwatchos-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=DEV %s +// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mios-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=SIM %s +// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mtvos-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=SIM %s +// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -mwatchos-version-min=6.0.0 -E -dM %s | FileCheck -check-prefix=SIM %s // SIM: #define __APPLE_EMBEDDED_SIMULATOR__ 1 -// DEV-NOT: __APPLE_EMBEDDED_SIMULATOR__ Index: test/Driver/fsanitize.c === --- test/Driver/fsanitize.c +++ test/Driver/fsanitize.c @@ -334,10 +334,10 @@ // CHECK-TSAN-ARM-IOS: unsupported option '-fsanitize=thread' for target 'arm-apple-ios' // RUN: %clang -target i386-apple-iossimulator -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-I386-IOSSIMULATOR -// CHECK-TSAN-I386-IOSSIMULATOR: unsupported option '-fsanitize=thread' for target 'i386-apple-iossimulator' +// CHECK-TSAN-I386-IOSSIMULATOR: unsupported option '-fsanitize=thread' for target 'i386-apple-iossimulator-simulator' // RUN: %clang -target i386-apple-tvossimulator -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-I386-TVOSSIMULATOR -// CHECK-TSAN-I386-TVOSSIMULATOR: unsupported option '-fsanitize=thread' for target 'i386-apple-tvossimulator' +// CHECK-TSAN-I386-TVOSSIMULATOR: unsupported option '-fsanitize=thread' for target 'i386-apple-tvossimulator-simulator' // RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-memory-access %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-MEMORY-ACCESS // CHECK-TSAN-MEMORY-ACCESS-NOT: -cc1{{.*}}tsan-instrument-memory-accesses=0 @@ -423,11 +423,11 @@ // RUN: %clang -target i386-apple-iossimulator -fsanitize=efficiency-cache-frag %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ESAN-I386-IOSSIMULATOR // RUN: %clang -target i386-apple-iossimulator -fsanitize=efficiency-working-set %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ESAN-I386-IOSSIMULATOR -// CHECK-ESAN-I386-IOSSIMULATOR: unsupported option '-fsanitize=efficiency-{{.*}}' for target 'i386-apple-iossimulator' +// CHECK-ESAN-I386-IOSSIMULATOR: unsupported option '-fsanitize=efficiency-{{.*}}' for target 'i386-apple-iossimulator-simulator' // RUN: %clang -target i386-apple-tvossimulator -fsanitize=efficiency-cache-frag %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ESAN-I386-TVOSSIMULATOR // RUN: %clang -target i386-apple-tvossimulator -fsanitize=efficiency-working-set %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ESAN-I386-TVOSSIMULATOR -// CHECK-ESAN-I386-TVOSSIMUL
[PATCH] D40936: Hardware-assisted AddressSanitizer (clang part).
alekseyshl accepted this revision. alekseyshl added inline comments. Comment at: clang/include/clang/Basic/Sanitizers.def:47 +SANITIZER("hwaddress", HWAddress) + Why didn't we follow KASan style and name it "hw-address"? Comment at: clang/lib/Driver/SanitizerArgs.cpp:383 + std::make_pair(Efficiency, Address | HWAddress | Leak | Thread | Memory | + KernelAddress), + std::make_pair(Scudo, Address | HWAddress | Leak | Thread | Memory | Nit: 4 less spaces here and for the next pair. https://reviews.llvm.org/D40936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320075 - update hwasan docs
Author: kcc Date: Thu Dec 7 11:21:30 2017 New Revision: 320075 URL: http://llvm.org/viewvc/llvm-project?rev=320075&view=rev Log: update hwasan docs Summary: * use more readable name * document the hwasan attribute Reviewers: eugenis Reviewed By: eugenis Subscribers: llvm-commits, cfe-commits Differential Revision: https://reviews.llvm.org/D40938 Modified: cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst Modified: cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst?rev=320075&r1=320074&r2=320075&view=diff == --- cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst (original) +++ cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst Thu Dec 7 11:21:30 2017 @@ -1,9 +1,9 @@ -= -HardwareAssistedAddressSanitizer Design Documentation -= +=== +Hardware-assisted AddressSanitizer Design Documentation +=== This page is a design document for -**HardwareAssistedAddressSanitizer** (or HWASAN) +**hardware-assisted AddressSanitizer** (or **HWASAN**) a tool similar to :doc:`AddressSanitizer`, but based on partial hardware assistance. @@ -23,7 +23,7 @@ See the `AddressSanitizer paper`_ for de AArch64 has the `Address Tagging`_, a hardware feature that allows software to use 8 most significant bits of a 64-bit pointer as -a tag. HardwareAssistedAddressSanitizer uses `Address Tagging`_ +a tag. HWASAN uses `Address Tagging`_ to implement a memory safety tool, similar to :doc:`AddressSanitizer`, but with smaller memory overhead and slightly different (mostly better) accuracy guarantees. @@ -77,11 +77,26 @@ Error reporting Errors are generated by `__builtin_trap` and are handled by a signal handler. +Attribute +- + +HWASAN uses it's own LLVM IR Attribute `sanitize_hwaddress` and a matching +C function attribute. An alternative would be to re-use ASAN's attribute +`sanitize_address`. The reasons to use a separate attribute are: + + * Users may need to disable ASAN but not HWASAN, or vise versa, +because the tools have different trade-offs and compatibility issues. + * LLVM (ideally) does not use flags to decide which pass is being used, +ASAN or HWASAN are being applied, based on the function attributes. + +This does mean that users of HWASAN may need to add the new attribute +to the code that already uses the old attribute. + Comparison with AddressSanitizer -HardwareAssistedAddressSanitizer: +HWASAN: * Is less portable than :doc:`AddressSanitizer` as it relies on hardware `Address Tagging`_ (AArch64). Address Tagging can be emulated with compiler instrumentation, @@ -91,7 +106,8 @@ HardwareAssistedAddressSanitizer: * May have compatibility problems if the target code uses higher pointer bits for other purposes. * May require changes in the OS kernels (e.g. Linux seems to dislike -tagged pointers passed from address space). +tagged pointers passed from address space: +https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt). * **Does not require redzones to detect buffer overflows**, but the buffer overflow detection is probabilistic, with roughly `(2**K-1)/(2**K)` probability of catching a bug. @@ -99,7 +115,7 @@ HardwareAssistedAddressSanitizer: or stack-use-after-return**. The detection is similarly probabilistic. -The memory overhead of HardwareAssistedAddressSanitizer is expected to be much smaller +The memory overhead of HWASAN is expected to be much smaller than that of AddressSanitizer: `1/N` extra memory for the shadow and some overhead due to `N`-aligning all objects. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40938: update hwasan docs
This revision was automatically updated to reflect the committed changes. Closed by commit rC320075: update hwasan docs (authored by kcc). Changed prior to commit: https://reviews.llvm.org/D40938?vs=126001&id=126005#toc Repository: rC Clang https://reviews.llvm.org/D40938 Files: docs/HardwareAssistedAddressSanitizerDesign.rst Index: docs/HardwareAssistedAddressSanitizerDesign.rst === --- docs/HardwareAssistedAddressSanitizerDesign.rst +++ docs/HardwareAssistedAddressSanitizerDesign.rst @@ -1,9 +1,9 @@ -= -HardwareAssistedAddressSanitizer Design Documentation -= +=== +Hardware-assisted AddressSanitizer Design Documentation +=== This page is a design document for -**HardwareAssistedAddressSanitizer** (or HWASAN) +**hardware-assisted AddressSanitizer** (or **HWASAN**) a tool similar to :doc:`AddressSanitizer`, but based on partial hardware assistance. @@ -23,7 +23,7 @@ AArch64 has the `Address Tagging`_, a hardware feature that allows software to use 8 most significant bits of a 64-bit pointer as -a tag. HardwareAssistedAddressSanitizer uses `Address Tagging`_ +a tag. HWASAN uses `Address Tagging`_ to implement a memory safety tool, similar to :doc:`AddressSanitizer`, but with smaller memory overhead and slightly different (mostly better) accuracy guarantees. @@ -77,11 +77,26 @@ Errors are generated by `__builtin_trap` and are handled by a signal handler. +Attribute +- + +HWASAN uses it's own LLVM IR Attribute `sanitize_hwaddress` and a matching +C function attribute. An alternative would be to re-use ASAN's attribute +`sanitize_address`. The reasons to use a separate attribute are: + + * Users may need to disable ASAN but not HWASAN, or vise versa, +because the tools have different trade-offs and compatibility issues. + * LLVM (ideally) does not use flags to decide which pass is being used, +ASAN or HWASAN are being applied, based on the function attributes. + +This does mean that users of HWASAN may need to add the new attribute +to the code that already uses the old attribute. + Comparison with AddressSanitizer -HardwareAssistedAddressSanitizer: +HWASAN: * Is less portable than :doc:`AddressSanitizer` as it relies on hardware `Address Tagging`_ (AArch64). Address Tagging can be emulated with compiler instrumentation, @@ -91,15 +106,16 @@ * May have compatibility problems if the target code uses higher pointer bits for other purposes. * May require changes in the OS kernels (e.g. Linux seems to dislike -tagged pointers passed from address space). +tagged pointers passed from address space: +https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt). * **Does not require redzones to detect buffer overflows**, but the buffer overflow detection is probabilistic, with roughly `(2**K-1)/(2**K)` probability of catching a bug. * **Does not require quarantine to detect heap-use-after-free, or stack-use-after-return**. The detection is similarly probabilistic. -The memory overhead of HardwareAssistedAddressSanitizer is expected to be much smaller +The memory overhead of HWASAN is expected to be much smaller than that of AddressSanitizer: `1/N` extra memory for the shadow and some overhead due to `N`-aligning all objects. Index: docs/HardwareAssistedAddressSanitizerDesign.rst === --- docs/HardwareAssistedAddressSanitizerDesign.rst +++ docs/HardwareAssistedAddressSanitizerDesign.rst @@ -1,9 +1,9 @@ -= -HardwareAssistedAddressSanitizer Design Documentation -= +=== +Hardware-assisted AddressSanitizer Design Documentation +=== This page is a design document for -**HardwareAssistedAddressSanitizer** (or HWASAN) +**hardware-assisted AddressSanitizer** (or **HWASAN**) a tool similar to :doc:`AddressSanitizer`, but based on partial hardware assistance. @@ -23,7 +23,7 @@ AArch64 has the `Address Tagging`_, a hardware feature that allows software to use 8 most significant bits of a 64-bit pointer as -a tag. HardwareAssistedAddressSanitizer uses `Address Tagging`_ +a tag. HWASAN uses `Address Tagging`_ to implement a memory safety tool, similar to :doc:`AddressSanitizer`, but with smaller memory overhead and slightly different (mostly better) accuracy guarantees. @@ -77,11 +77,26 @@ Errors are generated by `__builtin_trap` and are handled by a signal handler. +Attribute +- + +HWASAN uses it's own LLVM IR
[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.
Nebiroth added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:242 std::shared_ptr PCHContainerOps, bool StoreInMemory, -PreambleCallbacks &Callbacks) { +PreambleCallbacks &Callbacks, std::unique_ptr PPCallbacks) { assert(VFS && "VFS is null"); ilya-biryukov wrote: > Could we add a method to `PreambleCallbacks` to create `PPCallbacks` instead? > Otherwise we have both `MacroDefined` in `PreambleCallbacks` and a separate > set of PPCallbacks, so we have two ways of doing the same thing. > > ``` > class PreambleCallbacks { > public: > // ... > > /// Remove this. > virtual void HandleMacroDefined(...); > > // Add this instead. > virtual std::unique_ptr createPPCallbacks(); > > } > ``` > > Alternatively, follow the current pattern in `PreambleCallbacks` and add some > extra functions from the `PPCallbacks` interface to it. This would not > require changes to the existing usages of `PrecompiledPreamble` in `ASTUnit`. > Albeit, I'd prefer the first option. > ``` > class PreambleCallbacks { > public: > // ... > > // Keep this > virtual void HandleMacroDefined(...); > // Add the ones you need, e.g. > virtual void InclusionDirective(...); > virtual void FileChanged(...); > }; > ``` If we were to do that, one would then be required to define a wrapper class for PPCallbacks and create an instance of it inside createPPCallbacks() for the purpose of creating a unique_ptr? Then that unique_ptr would be sent from within the PreambleCallbacks parameter in the Build function? Repository: rC Clang https://reviews.llvm.org/D39375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40936: Hardware-assisted AddressSanitizer (clang part).
eugenis added inline comments. Comment at: clang/include/clang/Basic/Sanitizers.def:47 +SANITIZER("hwaddress", HWAddress) + alekseyshl wrote: > Why didn't we follow KASan style and name it "hw-address"? A matter of taste, I guess. I don't want the attribute to be sanitize_hw_address, too many underscores. Also, kernel is a word and hw is just to letters. Comment at: clang/lib/Driver/SanitizerArgs.cpp:383 + std::make_pair(Efficiency, Address | HWAddress | Leak | Thread | Memory | + KernelAddress), + std::make_pair(Scudo, Address | HWAddress | Leak | Thread | Memory | alekseyshl wrote: > Nit: 4 less spaces here and for the next pair. https://bugs.llvm.org/show_bug.cgi?id=35563 https://reviews.llvm.org/D40936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40977: [OpenMP] NVPTX: Set default/minimum compute capability to sm_35
grokos created this revision. grokos added a project: OpenMP. Herald added a subscriber: mgorny. The current implementation of the nvptx runtime (to be upstreamed shortly) uses the `atomicMax` operation on 64-bit integers. This is only supported in compute capabilities 3.5 and later. I've changed the clang default to `sm_35`. Repository: rC Clang https://reviews.llvm.org/D40977 Files: CMakeLists.txt Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -241,14 +241,15 @@ set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING "Default OpenMP runtime used by -fopenmp.") -# OpenMP offloading requires at least sm_30 because we use shuffle instructions -# to generate efficient code for reductions. -set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +# OpenMP offloading requires at least sm_35 because we use shuffle instructions +# to generate efficient code for reductions and the atomicMax instruction on +# 64-bit integers in the implementation of conditional lastprivate. +set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs.") string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}") -if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 30) - message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_30") - set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35) + message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_35") + set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE) endif() Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -241,14 +241,15 @@ set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING "Default OpenMP runtime used by -fopenmp.") -# OpenMP offloading requires at least sm_30 because we use shuffle instructions -# to generate efficient code for reductions. -set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +# OpenMP offloading requires at least sm_35 because we use shuffle instructions +# to generate efficient code for reductions and the atomicMax instruction on +# 64-bit integers in the implementation of conditional lastprivate. +set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs.") string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}") -if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 30) - message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_30") - set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35) + message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_35") + set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40948: Switch Clang's default C++ language target to C++14
probinson added a reviewer: rjmccall. probinson added a comment. +rjmccall for the codegen bits. Comment at: clang/test/SemaCXX/new-array-size-conv.cpp:1 -// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s +// RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=gnu++98 %s // RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=c++98 %s I think the intent here was "whatever the current default is" and not specifically gnu++98, because the next line explicitly specifies c++98. So, unless this test fails miserably for C++14 (which I wouldn't expect) I think it should be adapted to whatever gets reported for that dialect. Repository: rC Clang https://reviews.llvm.org/D40948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320078 - [OPENMP] Do not capture private variables in the target regions.
Author: abataev Date: Thu Dec 7 11:49:28 2017 New Revision: 320078 URL: http://llvm.org/viewvc/llvm-project?rev=320078&view=rev Log: [OPENMP] Do not capture private variables in the target regions. Private variables are completely redefined in the outlined regions, so we don't need to capture them. Patch adds this behavior to the target-based regions. Modified: cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/OpenMP/teams_distribute_parallel_for_private_codegen.cpp cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_private_codegen.cpp cfe/trunk/test/OpenMP/teams_distribute_private_codegen.cpp cfe/trunk/test/OpenMP/teams_distribute_simd_private_codegen.cpp cfe/trunk/test/OpenMP/teams_private_codegen.cpp Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=320078&r1=320077&r2=320078&view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Dec 7 11:49:28 2017 @@ -14619,14 +14619,16 @@ bool Sema::tryCaptureVariable( // just break here. Similarly, global variables that are captured in a // target region should not be captured outside the scope of the region. if (RSI->CapRegionKind == CR_OpenMP) { - auto IsTargetCap = isOpenMPTargetCapturedDecl(Var, RSI->OpenMPLevel); + bool IsOpenMPPrivateDecl = isOpenMPPrivateDecl(Var, RSI->OpenMPLevel); + auto IsTargetCap = !IsOpenMPPrivateDecl && + isOpenMPTargetCapturedDecl(Var, RSI->OpenMPLevel); // When we detect target captures we are looking from inside the // target region, therefore we need to propagate the capture from the // enclosing region. Therefore, the capture is not initially nested. if (IsTargetCap) FunctionScopesIndex--; - if (IsTargetCap || isOpenMPPrivateDecl(Var, RSI->OpenMPLevel)) { + if (IsTargetCap || IsOpenMPPrivateDecl) { Nested = !IsTargetCap; DeclRefType = DeclRefType.getUnqualifiedType(); CaptureType = Context.getLValueReferenceType(DeclRefType); Modified: cfe/trunk/test/OpenMP/teams_distribute_parallel_for_private_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/teams_distribute_parallel_for_private_codegen.cpp?rev=320078&r1=320077&r2=320078&view=diff == --- cfe/trunk/test/OpenMP/teams_distribute_parallel_for_private_codegen.cpp (original) +++ cfe/trunk/test/OpenMP/teams_distribute_parallel_for_private_codegen.cpp Thu Dec 7 11:49:28 2017 @@ -72,13 +72,13 @@ int main() { // LAMBDA: call void [[OUTER_LAMBDA:@.+]]( [&]() { // LAMBDA: define{{.*}} internal{{.*}} void [[OUTER_LAMBDA]]( -// LAMBDA: call i32 @__tgt_target_teams(i64 -1, i8* @{{[^,]+}}, i32 3, i8** %{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i64* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32 0, i32 0) +// LAMBDA: call i32 @__tgt_target_teams(i64 -1, i8* @{{[^,]+}}, i32 1, i8** %{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i64* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32 0, i32 0) // LAMBDA: call void @[[LOFFL1:.+]]( // LAMBDA: ret #pragma omp target #pragma omp teams distribute parallel for private(g, g1, sivar) for (int i = 0; i < 2; ++i) { -// LAMBDA: define{{.*}} internal{{.*}} void @[[LOFFL1]](i{{64|32}} {{%.+}}, i{{64|32}} {{%.+}}) +// LAMBDA: define{{.*}} internal{{.*}} void @[[LOFFL1]](i{{64|32}} {{%.+}}) // LAMBDA: call void {{.+}} @__kmpc_fork_teams({{.+}}, i32 0, {{.+}} @[[LOUTL1:.+]] to {{.+}}) // LAMBDA: ret void @@ -164,12 +164,12 @@ int main() { } // CHECK: define {{.*}}i{{[0-9]+}} @main() -// CHECK: call i32 @__tgt_target_teams(i64 -1, i8* @{{[^,]+}}, i32 5, i8** %{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i64* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32 0, i32 0) -// CHECK: call void @[[OFFL1:.+]](i{{64|32}} %{{.+}}) +// CHECK: call i32 @__tgt_target_teams(i64 -1, i8* @{{[^,]+}}, i32 0, i8** null, i8** null, i{{64|32}}* null, i64* null, i32 0, i32 0) +// CHECK: call void @[[OFFL1:.+]]() // CHECK: {{%.+}} = call{{.*}} i32 @[[TMAIN_INT:.+]]() // CHECK: ret -// CHECK: define{{.*}} void @[[OFFL1]]({{.+}}) +// CHECK: define{{.*}} void @[[OFFL1]]() // CHECK: call void {{.+}} @__kmpc_fork_teams({{.+}}, i32 0, {{.+}} @[[OUTL1:.+]] to {{.+}}) // CHECK: ret void Modified: cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_private_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_private_codegen.cpp?rev=320078&r1=320077&r2=320078&view=diff == --- cfe/trunk/test/OpenMP/teams_distribu
[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks
george.burgess.iv added a comment. Thanks for this! It's interesting to me that these array-bound checks don't seem to use `@llvm.objectsize` in some form already. I can't find any notes about it grepping through git/source, so I'm happy with it. Comment at: lib/CodeGen/CGExpr.cpp:819 + QualType EltTy) { + auto *DRE = dyn_cast(E->IgnoreImpCasts()); + if (!DRE) nit: would `IgnoreParenImpCasts` be better? Comment at: lib/CodeGen/CGExpr.cpp:828 + // Find the implicit size parameter. + auto SizeDeclIt = SizeArguments.find(PVD); + if (SizeDeclIt == SizeArguments.end()) We should probably only do this if the argument to `pass_object_size` is `0` or `1`. `2` and `3` give lower-bounds on size (and a default value of 0), which could result in false-positives. (And please add a test that we don't do this for 2 or 3.) https://reviews.llvm.org/D40940 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40625: Harmonizing attribute GNU/C++ spellings
aaron.ballman marked 4 inline comments as done. aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1218-1228 def NeonPolyVectorType : TypeAttr { - let Spellings = [GNU<"neon_polyvector_type">]; + let Spellings = [Clang<"neon_polyvector_type">]; let Args = [IntArgument<"NumElements">]; let Documentation = [Undocumented]; } def NeonVectorType : TypeAttr { sbaranga wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > rsmith wrote: > > > > I *think* these are a Clang invention rather than part of the ARM NEON > > > > intrinsics specification, but perhaps you could ask someone from ARM to > > > > confirm that. > > > @sbaranga or @jmolloy -- do you happen to know the answer to this, or > > > know someone who does? > > Pinging this comment. > Yes, these should be internal to clang and used to implement the ACLE > specification (which defines the NEON intrinsics). The ACLE doesn't define > these. > > Here is a link to the latest spec: > https://developer.arm.com/products/software-development-tools/compilers/arm-compiler-5/docs/101028/latest/1-preface Perfect, thank you @sbaranga! https://reviews.llvm.org/D40625 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40625: Harmonizing attribute GNU/C++ spellings
aaron.ballman updated this revision to Diff 126015. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Updated based on review feedback. This patch leaves off attributes with custom parsing, as well as `analyzer_noreturn`, so that we can handle them as special cases. https://reviews.llvm.org/D40625 Files: include/clang/Basic/Attr.td Index: include/clang/Basic/Attr.td === --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -513,7 +513,7 @@ } def AddressSpace : TypeAttr { - let Spellings = [GNU<"address_space">]; + let Spellings = [Clang<"address_space">]; let Args = [IntArgument<"AddressSpace">]; let Documentation = [Undocumented]; } @@ -598,7 +598,7 @@ } def Annotate : InheritableParamAttr { - let Spellings = [GNU<"annotate">]; + let Spellings = [Clang<"annotate">]; let Args = [StringArgument<"Annotation">]; // Ensure that the annotate attribute can be used with // '#pragma clang attribute' even though it has no subject list. @@ -700,7 +700,7 @@ } def Blocks : InheritableAttr { - let Spellings = [GNU<"blocks">]; + let Spellings = [Clang<"blocks">]; let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>]; let Documentation = [Undocumented]; } @@ -727,34 +727,34 @@ // cf_returns_retained attributes. It is generally applied by // '#pragma clang arc_cf_code_audited' rather than explicitly. def CFAuditedTransfer : InheritableAttr { - let Spellings = [GNU<"cf_audited_transfer">]; + let Spellings = [Clang<"cf_audited_transfer">]; let Subjects = SubjectList<[Function], ErrorDiag>; let Documentation = [Undocumented]; } // cf_unknown_transfer is an explicit opt-out of cf_audited_transfer. // It indicates that the function has unknown or unautomatable // transfer semantics. def CFUnknownTransfer : InheritableAttr { - let Spellings = [GNU<"cf_unknown_transfer">]; + let Spellings = [Clang<"cf_unknown_transfer">]; let Subjects = SubjectList<[Function], ErrorDiag>; let Documentation = [Undocumented]; } def CFReturnsRetained : InheritableAttr { - let Spellings = [GNU<"cf_returns_retained">]; + let Spellings = [Clang<"cf_returns_retained">]; // let Subjects = SubjectList<[ObjCMethod, ObjCProperty, Function]>; let Documentation = [Undocumented]; } def CFReturnsNotRetained : InheritableAttr { - let Spellings = [GNU<"cf_returns_not_retained">]; + let Spellings = [Clang<"cf_returns_not_retained">]; // let Subjects = SubjectList<[ObjCMethod, ObjCProperty, Function]>; let Documentation = [Undocumented]; } def CFConsumed : InheritableParamAttr { - let Spellings = [GNU<"cf_consumed">]; + let Spellings = [Clang<"cf_consumed">]; let Subjects = SubjectList<[ParmVar]>; let Documentation = [Undocumented]; } @@ -875,7 +875,7 @@ } def CXX11NoReturn : InheritableAttr { - let Spellings = [CXX11<"","noreturn", 200809>]; + let Spellings = [CXX11<"", "noreturn", 200809>]; let Subjects = SubjectList<[Function], ErrorDiag>; let Documentation = [CXX11NoReturnDocs]; } @@ -1030,13 +1030,13 @@ } def MinSize : InheritableAttr { - let Spellings = [GNU<"minsize">]; + let Spellings = [Clang<"minsize">]; let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>; let Documentation = [Undocumented]; } def FlagEnum : InheritableAttr { - let Spellings = [GNU<"flag_enum">]; + let Spellings = [Clang<"flag_enum">]; let Subjects = SubjectList<[Enum]>; let Documentation = [FlagEnumDocs]; } @@ -1085,22 +1085,22 @@ } def IBAction : InheritableAttr { - let Spellings = [GNU<"ibaction">]; + let Spellings = [Clang<"ibaction">]; let Subjects = SubjectList<[ObjCInstanceMethod]>; // An AST node is created for this attribute, but is not used by other parts // of the compiler. However, this node needs to exist in the AST because // external tools rely on it. let Documentation = [Undocumented]; } def IBOutlet : InheritableAttr { - let Spellings = [GNU<"iboutlet">]; + let Spellings = [Clang<"iboutlet">]; // let Subjects = [ObjCIvar, ObjCProperty]; let Documentation = [Undocumented]; } def IBOutletCollection : InheritableAttr { - let Spellings = [GNU<"iboutletcollection">]; + let Spellings = [Clang<"iboutletcollection">]; let Args = [TypeArgument<"Interface", 1>]; // let Subjects = [ObjCIvar, ObjCProperty]; let Documentation = [Undocumented]; @@ -1210,13 +1210,13 @@ } def NeonPolyVectorType : TypeAttr { - let Spellings = [GNU<"neon_polyvector_type">]; + let Spellings = [Clang<"neon_polyvector_type">]; let Args = [IntArgument<"NumElements">]; let Documentation = [Undocumented]; } def NeonVectorType : TypeAttr { - let Spellings = [GNU<"neon_vector_type">]; + let Spellings = [Clang<"neon_vector_type">]; let Args = [IntArgument<"NumElements">]; let Documentation = [Undocumented]; } @@ -1299,28 +1299,28 @@ // this should be rejected on non-k
[PATCH] D40225: Add -std=c17 as a flag
aaron.ballman added a comment. In https://reviews.llvm.org/D40225#944113, @aaron.ballman wrote: > Rebased on ToT and given more context. Ping. I'd like to get this in before the 6.0 branch. https://reviews.llvm.org/D40225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40448: Add a printing policy for AST dumping
aaron.ballman updated this revision to Diff 126016. aaron.ballman added a comment. Ping. Added more context to the patch. https://reviews.llvm.org/D40448 Files: include/clang/AST/Type.h lib/AST/ASTDumper.cpp lib/AST/TypePrinter.cpp lib/StaticAnalyzer/Checkers/PaddingChecker.cpp test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-examples.cpp test/Frontend/float16.cpp test/Misc/ast-dump-attr.cpp test/Misc/ast-dump-decl.cpp test/Misc/ast-dump-invalid.cpp test/OpenMP/dump.cpp test/Parser/objc-default-ctor-init.mm test/SemaCXX/compound-literal.cpp test/SemaCXX/sourceranges.cpp test/SemaCXX/warn-redundant-move.cpp test/SemaObjCXX/block-cleanup.mm test/SemaTemplate/default-expr-arguments-2.cpp test/SemaTemplate/default-expr-arguments-3.cpp Index: test/SemaTemplate/default-expr-arguments-3.cpp === --- test/SemaTemplate/default-expr-arguments-3.cpp +++ test/SemaTemplate/default-expr-arguments-3.cpp @@ -1,11 +1,11 @@ // RUN: %clang_cc1 -std=c++14 -verify -ast-dump %s | FileCheck %s // expected-no-diagnostics -// CHECK: FunctionDecl {{.*}} used func 'void (void)' +// CHECK: FunctionDecl {{.*}} used func 'void ()' // CHECK-NEXT: TemplateArgument type 'int' -// CHECK: LambdaExpr {{.*}} 'class (lambda at -// CHECK: ParmVarDecl {{.*}} used f 'enum foo' cinit -// CHECK-NEXT: DeclRefExpr {{.*}} 'enum foo' EnumConstant {{.*}} 'a' 'enum foo' +// CHECK: LambdaExpr {{.*}} '(lambda at +// CHECK: ParmVarDecl {{.*}} used f 'foo' cinit +// CHECK-NEXT: DeclRefExpr {{.*}} 'foo' EnumConstant {{.*}} 'a' 'foo' namespace PR28795 { template @@ -22,9 +22,9 @@ // CHECK: ClassTemplateSpecializationDecl {{.*}} struct class2 definition // CHECK: TemplateArgument type 'int' -// CHECK: LambdaExpr {{.*}} 'class (lambda at -// CHECK: ParmVarDecl {{.*}} used f 'enum foo' cinit -// CHECK-NEXT: DeclRefExpr {{.*}} 'enum foo' EnumConstant {{.*}} 'a' 'enum foo' +// CHECK: LambdaExpr {{.*}} '(lambda at +// CHECK: ParmVarDecl {{.*}} used f 'foo' cinit +// CHECK-NEXT: DeclRefExpr {{.*}} 'foo' EnumConstant {{.*}} 'a' 'foo' // Template struct case: template struct class2 { @@ -38,11 +38,11 @@ // CHECK: FunctionTemplateDecl {{.*}} f1 // CHECK-NEXT: TemplateTypeParmDecl {{.*}} typename depth 0 index 0 T -// CHECK-NEXT: FunctionDecl {{.*}} f1 'void (void)' -// CHECK: FunctionDecl {{.*}} f1 'void (void)' +// CHECK-NEXT: FunctionDecl {{.*}} f1 'void ()' +// CHECK: FunctionDecl {{.*}} f1 'void ()' // CHECK-NEXT: TemplateArgument type 'int' -// CHECK: ParmVarDecl {{.*}} n 'enum foo' cinit -// CHECK-NEXT: DeclRefExpr {{.*}} 'enum foo' EnumConstant {{.*}} 'a' 'enum foo' +// CHECK: ParmVarDecl {{.*}} n 'foo' cinit +// CHECK-NEXT: DeclRefExpr {{.*}} 'foo' EnumConstant {{.*}} 'a' 'foo' template void f1() { Index: test/SemaTemplate/default-expr-arguments-2.cpp === --- test/SemaTemplate/default-expr-arguments-2.cpp +++ test/SemaTemplate/default-expr-arguments-2.cpp @@ -9,8 +9,8 @@ public: enum { kSomeConst = 128 }; bar(int x = kSomeConst) {} }; - - // CHECK: FunctionDecl{{.*}}f 'void (void)' + + // CHECK: FunctionDecl{{.*}}f 'void ()' void f() { // CHECK: VarDecl{{.*}}tmp 'bar' // CHECK: CXXDefaultArgExpr{{.*}}'int' Index: test/SemaObjCXX/block-cleanup.mm === --- test/SemaObjCXX/block-cleanup.mm +++ test/SemaObjCXX/block-cleanup.mm @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -std=gnu++11 -o /dev/null -x objective-c++ -fblocks -ast-dump %s 2>&1 | FileCheck %s -// CHECK: -FunctionDecl {{.*}} test 'id (void)' +// CHECK: -FunctionDecl {{.*}} test 'id ()' // CHECK-NEXT: -CompoundStmt // CHECK-NEXT: -ReturnStmt // CHECK-NEXT: -ExprWithCleanups Index: test/SemaCXX/warn-redundant-move.cpp === --- test/SemaCXX/warn-redundant-move.cpp +++ test/SemaCXX/warn-redundant-move.cpp @@ -75,7 +75,7 @@ return d; // Verify the implicit move from the AST dump // CHECK-AST: ReturnStmt{{.*}}line:[[@LINE-2]] - // CHECK-AST-NEXT: CXXConstructExpr{{.*}}struct D{{.*}}void (struct D &&) + // CHECK-AST-NEXT: CXXConstructExpr{{.*}}D{{.*}}void (D &&) // CHECK-AST-NEXT: ImplicitCastExpr // CHECK-AST-NEXT: DeclRefExpr{{.*}}ParmVar{{.*}}'d' Index: test/SemaCXX/sourceranges.cpp === --- test/SemaCXX/sourceranges.cpp +++ test/SemaCXX/sourceranges.cpp @@ -46,7 +46,7 @@ void construct() { using namespace foo; A a = A(12); - // CHECK: CXXConstructExpr {{0x[0-9a-fA-F]+}} 'class foo::A' 'void (int){{( __attribute__\(\(thiscall\)\))?}}' + // CHECK: CXXConstructExpr {{0x[0-9a-fA-F]+}} 'foo::A' 'void (int){{( __attribute__\(\(thiscall\)\))?}}' D d = D(12); - // CHECK: CXXConstructExpr {{0x[0-9a-fA-F]+}} 'struct D' 'void (int){{(
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
berenm updated this revision to Diff 126017. berenm added a comment. Herald added a subscriber: klimek. Factor !Type.isNull() && Type.isConstQualified() condition Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D39363 Files: clang-tidy/readability/IdentifierNamingCheck.cpp test/clang-tidy/readability-identifier-naming.cpp Index: test/clang-tidy/readability-identifier-naming.cpp === --- test/clang-tidy/readability-identifier-naming.cpp +++ test/clang-tidy/readability-identifier-naming.cpp @@ -405,6 +405,38 @@ using ::FOO_NS::InlineNamespace::CE_function; // CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}} + + unsigned MY_LOCAL_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for local variable 'MY_LOCAL_array' +// CHECK-FIXES: {{^}} unsigned my_local_array[] = {1,2,3};{{$}} + + unsigned const MyConstLocal_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'MyConstLocal_array' +// CHECK-FIXES: {{^}} unsigned const kMyConstLocalArray[] = {1,2,3};{{$}} + + static unsigned MY_STATIC_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for static variable 'MY_STATIC_array' +// CHECK-FIXES: {{^}} static unsigned s_myStaticArray[] = {1,2,3};{{$}} + + static unsigned const MyConstStatic_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: invalid case style for static constant 'MyConstStatic_array' +// CHECK-FIXES: {{^}} static unsigned const MY_CONST_STATIC_ARRAY[] = {1,2,3};{{$}} + + char MY_LOCAL_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'MY_LOCAL_string' +// CHECK-FIXES: {{^}} char my_local_string[] = "123";{{$}} + + char const MyConstLocal_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant 'MyConstLocal_string' +// CHECK-FIXES: {{^}} char const kMyConstLocalString[] = "123";{{$}} + + static char MY_STATIC_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for static variable 'MY_STATIC_string' +// CHECK-FIXES: {{^}} static char s_myStaticString[] = "123";{{$}} + + static char const MyConstStatic_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for static constant 'MyConstStatic_string' +// CHECK-FIXES: {{^}} static char const MY_CONST_STATIC_STRING[] = "123";{{$}} } #define MY_TEST_Macro(X) X() @@ -418,6 +450,27 @@ template struct a { typename t_t::template b<> c; + + char const MY_ConstMember_string[4] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'MY_ConstMember_string' +// CHECK-FIXES: {{^}} char const my_const_member_string[4] = "123";{{$}} + + static char const MyConstClass_string[]; +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for class constant 'MyConstClass_string' +// CHECK-FIXES: {{^}} static char const kMyConstClassString[];{{$}} }; +template +char const a::MyConstClass_string[] = "123"; +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for class constant 'MyConstClass_string' +// CHECK-FIXES: {{^}}char const a::kMyConstClassString[] = "123";{{$}} + template class A> struct b { A c; }; + +unsigned MY_GLOBAL_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'MY_GLOBAL_array' +// CHECK-FIXES: {{^}}unsigned g_my_global_array[] = {1,2,3};{{$}} + +unsigned const MyConstGlobal_array[] = {1,2,3}; +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global constant 'MyConstGlobal_array' +// CHECK-FIXES: {{^}}unsigned const MY_CONST_GLOBAL_ARRAY[] = {1,2,3};{{$}} Index: clang-tidy/readability/IdentifierNamingCheck.cpp === --- clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tidy/readability/IdentifierNamingCheck.cpp @@ -449,13 +449,13 @@ if (const auto *Decl = dyn_cast(D)) { QualType Type = Decl->getType(); -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_ConstantMember]) - return SK_ConstantMember; +if (!Type.isNull() && Type.isConstQualified()) { + if (NamingStyles[SK_ConstantMember]) +return SK_ConstantMember; -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_Constant]) - return SK_Constant; + if (NamingStyles[SK_Constant]) +return SK_Constant; +} if (Decl->getAccess() == AS_private && NamingStyles[SK_PrivateMember]) return SK_PrivateMember; @@ -478,13 +478,13 @@ if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable]) return SK_ConstexprVariable; -if (!Type.isNull() && Type.isLocalConstQualified() && -NamingStyles[SK_ConstantParameter]) - return SK_ConstantPa
[PATCH] D40977: [OpenMP] NVPTX: Set default/minimum compute capability to sm_35
guansong added a comment. LGTM. Repository: rC Clang https://reviews.llvm.org/D40977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
berenm added a comment. In https://reviews.llvm.org/D39363#948134, @alexfh wrote: > Sorry for the delay. I missed this revision somehow. Please add cfe-commits > to the subscribers list so that others can chime in. No worries, I'll add it in the future. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D39363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
xazax.hun updated this revision to Diff 125986. xazax.hun added a comment. Herald added a subscriber: rnkovacs. - @gerazo addressed some review comments in scan-build-py. https://reviews.llvm.org/D30691 Files: include/clang/AST/ASTContext.h include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Core/CMakeLists.txt lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/PathDiagnostic.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp lib/StaticAnalyzer/Frontend/CMakeLists.txt test/Analysis/Inputs/ctu-chain.cpp test/Analysis/Inputs/ctu-other.cpp test/Analysis/Inputs/externalFnMap.txt test/Analysis/ctu-main.cpp tools/scan-build-py/libscanbuild/__init__.py tools/scan-build-py/libscanbuild/analyze.py tools/scan-build-py/libscanbuild/arguments.py tools/scan-build-py/libscanbuild/clang.py tools/scan-build-py/libscanbuild/report.py tools/scan-build-py/tests/unit/test_analyze.py tools/scan-build-py/tests/unit/test_clang.py Index: tools/scan-build-py/tests/unit/test_clang.py === --- tools/scan-build-py/tests/unit/test_clang.py +++ tools/scan-build-py/tests/unit/test_clang.py @@ -92,3 +92,15 @@ self.assertEqual('Checker One description', result.get('checker.one')) self.assertTrue('checker.two' in result) self.assertEqual('Checker Two description', result.get('checker.two')) + + +class ClangIsCtuCapableTest(unittest.TestCase): +def test_ctu_not_found(self): +is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping') +self.assertFalse(is_ctu) + + +class ClangGetTripleArchTest(unittest.TestCase): +def test_arch_is_not_empty(self): +arch = sut.get_triple_arch(['clang', '-E', '-'], '.') +self.assertTrue(len(arch) > 0) Index: tools/scan-build-py/tests/unit/test_analyze.py === --- tools/scan-build-py/tests/unit/test_analyze.py +++ tools/scan-build-py/tests/unit/test_analyze.py @@ -4,12 +4,12 @@ # This file is distributed under the University of Illinois Open Source # License. See LICENSE.TXT for details. -import libear -import libscanbuild.analyze as sut import unittest import re import os import os.path +import libear +import libscanbuild.analyze as sut class ReportDirectoryTest(unittest.TestCase): @@ -333,3 +333,83 @@ def test_method_exception_not_caught(self): self.assertRaises(Exception, method_exception_from_inside, dict()) + + +class PrefixWithTest(unittest.TestCase): + +def test_gives_empty_on_empty(self): +res = sut.prefix_with(0, []) +self.assertFalse(res) + +def test_interleaves_prefix(self): +res = sut.prefix_with(0, [1, 2, 3]) +self.assertListEqual([0, 1, 0, 2, 0, 3], res) + + +class MergeCtuMapTest(unittest.TestCase): + +def test_no_map_gives_empty(self): +pairs = sut.create_global_ctu_function_map([]) +self.assertFalse(pairs) + +def test_multiple_maps_merged(self): +concat_map = ['c:@F@fun1#I# ast/fun1.c.ast', + 'c:@F@fun2#I# ast/fun2.c.ast', + 'c:@F@fun3#I# ast/fun3.c.ast'] +pairs = sut.create_global_ctu_function_map(concat_map) +self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs) +self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs) +self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs) +self.assertEqual(3, len(pairs)) + +def test_not_unique_func_left_out(self): +concat_map = ['c:@F@fun1#I# ast/fun1.c.ast', + 'c:@F@fun2#I# ast/fun2.c.ast', + 'c:@F@fun1#I# ast/fun7.c.ast'] +pairs = sut.create_global_ctu_function_map(concat_map) +self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs) +self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs) +self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs) +self.assertEqual(1, len(pairs)) + +def test_duplicates_are_kept(self): +concat_map = ['c:@F@fun1#I# ast/fun1.c.ast', + 'c:@F@fun2#I# ast/fun2.c.ast', + 'c:@F@fun1#I# ast/fun1.c.ast'] +pairs = sut.create_global_ctu_function_map(concat_map) +self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs) +self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs) +self.assertEqual(2, len(pairs)) + +def test_space_handled_in_source(self): +concat_map = ['c:@F@fun1#I# ast/f un.c.ast'] +pairs = sut.create_global_ctu_function_map(concat_map) +self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs) +self.assertEqual(1, len(pairs)) + + +class FuncMapSrcToAstTest(unittest.TestCase): + +
[PATCH] D40977: [OpenMP] NVPTX: Set default/minimum compute capability to sm_35
This revision was automatically updated to reflect the committed changes. Closed by commit rC320082: [OpenMP] NVPTX: Set default/minimum compute capability to sm_35 (authored by grokos). Repository: rC Clang https://reviews.llvm.org/D40977 Files: CMakeLists.txt Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -241,14 +241,15 @@ set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING "Default OpenMP runtime used by -fopenmp.") -# OpenMP offloading requires at least sm_30 because we use shuffle instructions -# to generate efficient code for reductions. -set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +# OpenMP offloading requires at least sm_35 because we use shuffle instructions +# to generate efficient code for reductions and the atomicMax instruction on +# 64-bit integers in the implementation of conditional lastprivate. +set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs.") string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}") -if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 30) - message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_30") - set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35) + message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_35") + set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE) endif() Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -241,14 +241,15 @@ set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING "Default OpenMP runtime used by -fopenmp.") -# OpenMP offloading requires at least sm_30 because we use shuffle instructions -# to generate efficient code for reductions. -set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +# OpenMP offloading requires at least sm_35 because we use shuffle instructions +# to generate efficient code for reductions and the atomicMax instruction on +# 64-bit integers in the implementation of conditional lastprivate. +set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs.") string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}") -if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 30) - message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_30") - set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35) + message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_35") + set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320085 - [libclang] Record code-completion invocations to a temporary file when
Author: arphaman Date: Thu Dec 7 12:37:50 2017 New Revision: 320085 URL: http://llvm.org/viewvc/llvm-project?rev=320085&view=rev Log: [libclang] Record code-completion invocations to a temporary file when requested by client This is a follow up to r319702 which records parsing invocations. These files are not emitted by default, and the client has to specify the invocation emission path first. rdar://35322543 Added: cfe/trunk/test/Index/record-completion-invocation.c Modified: cfe/trunk/tools/c-index-test/c-index-test.c cfe/trunk/tools/libclang/CIndex.cpp cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp cfe/trunk/tools/libclang/CIndexer.cpp cfe/trunk/tools/libclang/CIndexer.h cfe/trunk/tools/libclang/CXTranslationUnit.h Added: cfe/trunk/test/Index/record-completion-invocation.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/record-completion-invocation.c?rev=320085&view=auto == --- cfe/trunk/test/Index/record-completion-invocation.c (added) +++ cfe/trunk/test/Index/record-completion-invocation.c Thu Dec 7 12:37:50 2017 @@ -0,0 +1,11 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: env CINDEXTEST_INVOCATION_EMISSION_PATH=%t not c-index-test -code-completion-at=%s:10:1 "-remap-file=%s,%S/Inputs/record-parsing-invocation-remap.c" %s +// RUN: cat %t/libclang-* | FileCheck %s + +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: env LIBCLANG_DISABLE_CRASH_RECOVERY=1 CINDEXTEST_INVOCATION_EMISSION_PATH=%t not --crash c-index-test -code-completion-at=%s:10:1 "-remap-file=%s,%S/Inputs/record-parsing-invocation-remap.c" %s +// RUN: cat %t/libclang-* | FileCheck %s + +// CHECK: {"toolchain":"{{.*}}","libclang.operation":"complete","libclang.opts":1,"args":["clang","-fno-spell-checking","{{.*}}record-completion-invocation.c","-Xclang","-detailed-preprocessing-record","-fallow-editor-placeholders"],"invocation-args":["-code-completion-at={{.*}}record-completion-invocation.c:10:1"],"unsaved_file_hashes":[{"name":"{{.*}}record-completion-invocation.c","md5":"aee23773de90e665992b48209351d70e"}]} Modified: cfe/trunk/tools/c-index-test/c-index-test.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/c-index-test.c?rev=320085&r1=320084&r2=320085&view=diff == --- cfe/trunk/tools/c-index-test/c-index-test.c (original) +++ cfe/trunk/tools/c-index-test/c-index-test.c Thu Dec 7 12:37:50 2017 @@ -2316,7 +2316,8 @@ int perform_code_completion(int argc, co CXTranslationUnit TU; unsigned I, Repeats = 1; unsigned completionOptions = clang_defaultCodeCompleteOptions(); - + const char *InvocationPath; + if (getenv("CINDEXTEST_CODE_COMPLETE_PATTERNS")) completionOptions |= CXCodeComplete_IncludeCodePatterns; if (getenv("CINDEXTEST_COMPLETION_BRIEF_COMMENTS")) @@ -2335,7 +2336,10 @@ int perform_code_completion(int argc, co return -1; CIdx = clang_createIndex(0, 0); - + InvocationPath = getenv("CINDEXTEST_INVOCATION_EMISSION_PATH"); + if (InvocationPath) +clang_CXIndex_setInvocationEmissionPathOption(CIdx, InvocationPath); + if (getenv("CINDEXTEST_EDITING")) Repeats = 5; Modified: cfe/trunk/tools/libclang/CIndex.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=320085&r1=320084&r2=320085&view=diff == --- cfe/trunk/tools/libclang/CIndex.cpp (original) +++ cfe/trunk/tools/libclang/CIndex.cpp Thu Dec 7 12:37:50 2017 @@ -81,6 +81,8 @@ CXTranslationUnit cxtu::MakeCXTranslatio D->Diagnostics = nullptr; D->OverridenCursorsPool = createOverridenCXCursorsPool(); D->CommentToXML = nullptr; + D->ParsingOptions = 0; + D->Arguments = {}; return D; } @@ -3440,7 +3442,8 @@ clang_parseTranslationUnit_Impl(CXIndex LibclangInvocationReporter InvocationReporter( *CXXIdx, LibclangInvocationReporter::OperationKind::ParseOperation, - options, llvm::makeArrayRef(*Args), unsaved_files); + options, llvm::makeArrayRef(*Args), /*InvocationArgs=*/None, + unsaved_files); std::unique_ptr Unit(ASTUnit::LoadFromCommandLine( Args->data(), Args->data() + Args->size(), CXXIdx->getPCHContainerOperations(), Diags, @@ -3467,7 +3470,14 @@ clang_parseTranslationUnit_Impl(CXIndex return CXError_ASTReadError; *out_TU = MakeCXTranslationUnit(CXXIdx, std::move(Unit)); - return *out_TU ? CXError_Success : CXError_Failure; + if (CXTranslationUnitImpl *TU = *out_TU) { +TU->ParsingOptions = options; +TU->Arguments.reserve(Args->size()); +for (const char *Arg : *Args) + TU->Arguments.push_back(Arg); +return CXError_Success; + } + return CXError_Failure; } CXTranslationUnit Modified: cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CInd
[PATCH] D39665: Support __has_c_attribute
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Please update the documentation and the release notes. https://reviews.llvm.org/D39665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40225: Add -std=c17 as a flag
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM, makes sense to add this for GCC compatibility even if the standard ends up being called C18. https://reviews.llvm.org/D40225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40625: Harmonizing attribute GNU/C++ spellings
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM. I think the `LateParsed` attributes are not going to work properly (because they'd be written in places where the function parameters are not in scope), so I wonder if we should avoid allowing them with [[clang::]] notation for now so we can choose to make them function type attributes in the future, but I'm happy with you making that call. https://reviews.llvm.org/D40625 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40948: Switch Clang's default C++ language target to C++14
t.p.northover added inline comments. Comment at: clang/test/SemaCXX/new-array-size-conv.cpp:1 -// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s +// RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=gnu++98 %s // RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=c++98 %s probinson wrote: > I think the intent here was "whatever the current default is" and not > specifically gnu++98, because the next line explicitly specifies c++98. So, > unless this test fails miserably for C++14 (which I wouldn't expect) I think > it should be adapted to whatever gets reported for that dialect. The original commit of this file (r107229 from Doug Gregor) called out the fact that what's being tested is a GNU and C++0x extension. I think that implies that the 3 run lines really *should* test gnu++98, c++98 and c++11. Repository: rC Clang https://reviews.llvm.org/D40948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40968: [OpenMP] Diagnose function name on the link clause
kkwli0 marked an inline comment as done. kkwli0 added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:12576 +if (MT == OMPDeclareTargetDeclAttr::MT_Link && isa(ND)) { + Diag(Id.getLoc(), diag::err_omp_function_in_link_clause); ABataev wrote: > I would like to see this some in `check` functions, like > `checkDeclIsAllowedInOpenMPTarget` rather than here Sure. I will move the check into checkDeclIsAllowedInOpenMPTarget. https://reviews.llvm.org/D40968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40968: [OpenMP] Diagnose function name on the link clause
kkwli0 updated this revision to Diff 126025. kkwli0 marked an inline comment as done. https://reviews.llvm.org/D40968 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaOpenMP.cpp test/OpenMP/declare_target_ast_print.cpp test/OpenMP/declare_target_messages.cpp Index: test/OpenMP/declare_target_messages.cpp === --- test/OpenMP/declare_target_messages.cpp +++ test/OpenMP/declare_target_messages.cpp @@ -19,6 +19,10 @@ void c(); // expected-warning {{declaration is not declared in any declare target region}} +void func() {} // expected-note {{'func' defined here}} + +#pragma omp declare target link(func) // expected-error {{function name is not allowed in 'link' clause}} + extern int b; struct NonT { Index: test/OpenMP/declare_target_ast_print.cpp === --- test/OpenMP/declare_target_ast_print.cpp +++ test/OpenMP/declare_target_ast_print.cpp @@ -108,9 +108,7 @@ // CHECK: #pragma omp end declare target int c1, c2, c3; -void f3() { -} -#pragma omp declare target link(c1) link(c2), link(c3, f3) +#pragma omp declare target link(c1) link(c2), link(c3) // CHECK: #pragma omp declare target link // CHECK: int c1; // CHECK: #pragma omp end declare target @@ -120,9 +118,6 @@ // CHECK: #pragma omp declare target link // CHECK: int c3; // CHECK: #pragma omp end declare target -// CHECK: #pragma omp declare target link -// CHECK: void f3() -// CHECK: #pragma omp end declare target struct SSSt { #pragma omp declare target Index: lib/Sema/SemaOpenMP.cpp === --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -12578,7 +12578,7 @@ ND->addAttr(A); if (ASTMutationListener *ML = Context.getASTMutationListener()) ML->DeclarationMarkedOpenMPDeclareTarget(ND, A); - checkDeclIsAllowedInOpenMPTarget(nullptr, ND); + checkDeclIsAllowedInOpenMPTarget(nullptr, ND, &Id); } else if (ND->getAttr()->getMapType() != MT) { Diag(Id.getLoc(), diag::err_omp_declare_target_to_and_link) << Id.getName(); @@ -12667,7 +12667,8 @@ return true; } -void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D) { +void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D, +const DeclarationNameInfo *Id) { if (!D || D->isInvalidDecl()) return; SourceRange SR = E ? E->getSourceRange() : D->getSourceRange(); @@ -12696,6 +12697,16 @@ return; } } + if (FunctionDecl *FD = dyn_cast(D)) { +if (FD->hasAttr() && +(FD->getAttr()->getMapType() == + OMPDeclareTargetDeclAttr::MT_Link)) { + assert(Id && "Null decl name info on link clause."); + Diag(Id->getLoc(), diag::err_omp_function_in_link_clause); + Diag(FD->getLocation(), diag::note_defined_here) << FD; + return; +} + } if (!E) { // Checking declaration inside declare target region. if (!D->hasAttr() && Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -8652,7 +8652,8 @@ OMPDeclareTargetDeclAttr::MapTypeTy MT, NamedDeclSetType &SameDirectiveDecls); /// Check declaration inside target region. - void checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D); + void checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D, +const DeclarationNameInfo *Id = nullptr); /// Return true inside OpenMP declare target region. bool isInOpenMPDeclareTargetContext() const { return IsInOpenMPDeclareTargetContext; Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -8709,6 +8709,8 @@ def warn_omp_not_in_target_context : Warning< "declaration is not declared in any declare target region">, InGroup; +def err_omp_function_in_link_clause : Error< + "function name is not allowed in 'link' clause">; def err_omp_aligned_expected_array_or_ptr : Error< "argument of aligned clause should be array" "%select{ or pointer|, pointer, reference to array or reference to pointer}1" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40948: Switch Clang's default C++ language target to C++14
rsmith added inline comments. Comment at: clang/test/CodeGenCXX/new-overflow.cpp:88 // CHECK: [[N:%.*]] = sext i16 {{%.*}} to i32 - // CHECK-NEXT: [[T0:%.*]] = icmp slt i32 [[N]], 0 - // CHECK-NEXT: [[T1:%.*]] = select i1 [[T0]], i32 -1, i32 [[N]] - // CHECK-NEXT: call i8* @_Znaj(i32 [[T1]]) + // CHECK-NEXT: call i8* @_Znaj(i32 [[N]]) // CHECK: getelementptr inbounds {{.*}}, i32 [[N]] The changes in this file are a regression; C++14 requires us to check whether the array bound prior to promotion is negative. Can you file a bug on that? Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:1-2 -// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -o %t -// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -O2 -disable-llvm-passes -emit-llvm -o %t.opt +// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -emit-llvm -o %t +// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 -disable-llvm-passes -emit-llvm -o %t.opt // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t Why does this one need a -std= flag? Comment at: clang/test/Parser/cxx1z-nested-namespace-definition.cpp:3-4 +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98 // RUN: not %clang_cc1 -x c++ -fixit %t -Werror -DFIXIT // RUN: %clang_cc1 -x c++ %t -DFIXIT // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -Wc++14-compat I think these two should also be `-std=c++98`, right? Comment at: clang/test/SemaCXX/unknown-type-name.cpp:98 -template int junk1(T::junk); // expected-warning{{variable templates are a C++14 extension}} +template int junk1(T::junk); +#if __cplusplus <= 201103L Huh, we should probably have a `-Wvexing-parse` warning for this. Can you file a bug? Comment at: clang/test/SemaCXX/unknown-type-name.cpp:110-111 // FIXME: We can tell this was intended to be a function because it does not //have a dependent nested name specifier. +template int i(T::type, int()); Maybe delete this (incorrect in C++14) FIXME? Repository: rC Clang https://reviews.llvm.org/D40948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39665: Support __has_c_attribute
aaron.ballman closed this revision. aaron.ballman added a comment. Commit in r320088. https://reviews.llvm.org/D39665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40983: Generate Libclang invocation reproducers using a new -cc1gen-reproducer option
arphaman created this revision. Herald added a subscriber: mgorny. This patch is a follow up to the previous work that recorded Libclang invocations into temporary files: https://reviews.llvm.org/D40527. It adds a new -cc1 mode to clang: `-cc1gen-reproducer`. The goal of this mode is to generate Clang reproducer files for Libclang tool invocation. The JSON format in the invocation files is not really intended to be stable, so Libclang and Clang should be of the same version when generating reproducers. The new mode emits the information about the temporary files in the reproducers to stdout using JSON. It also injects additional Libclang-specific information about the reproducer to the reproducer's .sh files. Thanks for taking a look! Repository: rC Clang https://reviews.llvm.org/D40983 Files: include/clang/Driver/Driver.h lib/Driver/Driver.cpp test/Index/create-libclang-completion-reproducer.c test/Index/create-libclang-parsing-reproducer.c tools/driver/CMakeLists.txt tools/driver/cc1gen_reproducer_main.cpp tools/driver/driver.cpp Index: tools/driver/driver.cpp === --- tools/driver/driver.cpp +++ tools/driver/driver.cpp @@ -205,6 +205,8 @@ void *MainAddr); extern int cc1as_main(ArrayRef Argv, const char *Argv0, void *MainAddr); +extern int cc1gen_reproducer_main(ArrayRef Argv, + const char *Argv0, void *MainAddr); static void insertTargetAndModeArgs(const ParsedClangName &NameParts, SmallVectorImpl &ArgVector, @@ -309,6 +311,8 @@ return cc1_main(argv.slice(2), argv[0], GetExecutablePathVP); if (Tool == "as") return cc1as_main(argv.slice(2), argv[0], GetExecutablePathVP); + if (Tool == "gen-reproducer") +return cc1gen_reproducer_main(argv.slice(2), argv[0], GetExecutablePathVP); // Reject unknown tools. llvm::errs() << "error: unknown integrated tool '" << Tool << "'\n"; Index: tools/driver/cc1gen_reproducer_main.cpp === --- /dev/null +++ tools/driver/cc1gen_reproducer_main.cpp @@ -0,0 +1,198 @@ +//===-- cc1gen_reproducer_main.cpp - Clang reproducer generator --===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// This is the entry point to the clang -cc1gen-reproducer functionality, which +// generates reproducers for invocations for clang-based tools. +// +//===--===// + +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/LLVM.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "clang/Driver/Compilation.h" +#include "clang/Driver/Driver.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/TargetSelect.h" +#include "llvm/Support/YAMLTraits.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; + +namespace { + +struct UnsavedFileHash { + std::string Name; + std::string MD5; +}; + +struct ClangInvocationInfo { + std::string Toolchain; + std::string LibclangOperation; + std::string LibclangOptions; + std::vector Arguments; + std::vector InvocationArguments; + std::vector UnsavedFileHashes; + bool Dump = false; +}; + +} // end anonymous namespace + +LLVM_YAML_IS_SEQUENCE_VECTOR(UnsavedFileHash) + +namespace llvm { +namespace yaml { + +template <> struct MappingTraits { + static void mapping(IO &IO, UnsavedFileHash &Info) { +IO.mapRequired("name", Info.Name); +IO.mapRequired("md5", Info.MD5); + } +}; + +template <> struct MappingTraits { + static void mapping(IO &IO, ClangInvocationInfo &Info) { +IO.mapRequired("toolchain", Info.Toolchain); +IO.mapOptional("libclang.operation", Info.LibclangOperation); +IO.mapOptional("libclang.opts", Info.LibclangOptions); +IO.mapRequired("args", Info.Arguments); +IO.mapOptional("invocation-args", Info.InvocationArguments); +IO.mapOptional("unsaved_file_hashes", Info.UnsavedFileHashes); + } +}; + +} // end namespace yaml +} // end namespace llvm + +static std::string generateReproducerMetaInfo(const ClangInvocationInfo &Info) { + std::string Result; + llvm::raw_string_ostream OS(Result); + OS << '{'; + bool NeedComma = false; + auto EmitKey = [&](StringRef Key) { +if (NeedComma) + OS << ", "; +NeedComma = true; +OS << '"' << Key << "\": "; + }; + auto EmitStringKey = [&](StringRef Key, StringRef Value) { +if (Value.empty()) + return; +EmitKey(Key); +OS << '"' << Value << '"'; + }; + EmitStringKey("libclang.operation", Info.LibclangOperation); + EmitStringKey("libclang.opts", Info.LibclangOpti
r320089 - Add new language mode flags for C17.
Author: aaronballman Date: Thu Dec 7 13:46:26 2017 New Revision: 320089 URL: http://llvm.org/viewvc/llvm-project?rev=320089&view=rev Log: Add new language mode flags for C17. This adds -std=c17, -std=gnu17, and -std=iso9899:2017 as language mode flags for C17 and updates the value of __STDC_VERSION__ to the value based on the C17 FDIS. Given that this ballot cannot succeed until 2018, it is expected that we (and GCC) will add c18 flags as aliases once the ballot passes. Modified: cfe/trunk/docs/ReleaseNotes.rst cfe/trunk/include/clang/Basic/LangOptions.def cfe/trunk/include/clang/Frontend/LangStandard.h cfe/trunk/include/clang/Frontend/LangStandards.def cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/unknown-std.c Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=320089&r1=320088&r2=320089&view=diff == --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Thu Dec 7 13:46:26 2017 @@ -121,6 +121,12 @@ New Compiler Flags number of attributes are supported outside of C++ mode. See the Clang attribute documentation for more information about which attributes are supported for each syntax. + +- Added the ``-std=c17``, ``-std=gnu17``, and ``-std=iso9899:2017`` language + mode flags for compatibility with GCC. This enables support for the next + version of the C standard, expected to be published by ISO in 2018. The only + difference between the ``-std=c17`` and ``-std=c11`` language modes is the + value of the ``__STDC_VERSION__`` macro, as C17 is a bug fix release. Deprecated Compiler Flags - Modified: cfe/trunk/include/clang/Basic/LangOptions.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=320089&r1=320088&r2=320089&view=diff == --- cfe/trunk/include/clang/Basic/LangOptions.def (original) +++ cfe/trunk/include/clang/Basic/LangOptions.def Thu Dec 7 13:46:26 2017 @@ -1,187 +1,188 @@ -//===--- LangOptions.def - Language option database -*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// -// -// This file defines the language options. Users of this file must -// define the LANGOPT macro to make use of this information. -// -// Optionally, the user may also define: -// -// BENIGN_LANGOPT: for options that don't affect the construction of the AST in -// any way (that is, the value can be different between an implicit module -// and the user of that module). -// -// COMPATIBLE_LANGOPT: for options that affect the construction of the AST in -// a way that doesn't prevent interoperability (that is, the value can be -// different between an explicit module and the user of that module). -// -// ENUM_LANGOPT: for options that have enumeration, rather than unsigned, type. -// -// VALUE_LANGOPT: for options that describe a value rather than a flag. -// -// BENIGN_ENUM_LANGOPT, COMPATIBLE_ENUM_LANGOPT, -// BENIGN_VALUE_LANGOPT, COMPATIBLE_VALUE_LANGOPT: combinations of the above. -// -// FIXME: Clients should be able to more easily select whether they want -// different levels of compatibility versus how to handle different kinds -// of option. -// -// The Description field should be a noun phrase, for instance "frobbing all -// widgets" or "C's implicit blintz feature". -//===--===// - -#ifndef LANGOPT -# error Define the LANGOPT macro to handle language options -#endif - -#ifndef COMPATIBLE_LANGOPT -# define COMPATIBLE_LANGOPT(Name, Bits, Default, Description) \ - LANGOPT(Name, Bits, Default, Description) -#endif - -#ifndef BENIGN_LANGOPT -# define BENIGN_LANGOPT(Name, Bits, Default, Description) \ - COMPATIBLE_LANGOPT(Name, Bits, Default, Description) -#endif - -#ifndef ENUM_LANGOPT -# define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \ - LANGOPT(Name, Bits, Default, Description) -#endif - -#ifndef COMPATIBLE_ENUM_LANGOPT -# define COMPATIBLE_ENUM_LANGOPT(Name, Type, Bits, Default, Description) \ - ENUM_LANGOPT(Name, Type, Bits, Default, Description) -#endif - -#ifndef BENIGN_ENUM_LANGOPT -# define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description) \ - COMPATIBLE_ENUM_LANGOPT(Name, Type, Bits, Default, Description) -#endif - -#ifndef VALUE_LANGOPT -# define VALUE_LANGOPT(Name, Bits, Default, Description) \ - LANGOPT(Name, Bits, Default, Description) -#endif - -#ifndef COMPATIBLE_VALUE_LANGOPT -# define COMPATIBLE_
[PATCH] D40225: Add -std=c17 as a flag
aaron.ballman closed this revision. aaron.ballman added a comment. Commit in r320089. https://reviews.llvm.org/D40225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320082 - [OpenMP] NVPTX: Set default/minimum compute capability to sm_35
Author: grokos Date: Thu Dec 7 12:27:31 2017 New Revision: 320082 URL: http://llvm.org/viewvc/llvm-project?rev=320082&view=rev Log: [OpenMP] NVPTX: Set default/minimum compute capability to sm_35 The current implementation of the nvptx runtime (to be upstreamed shortly) uses the atomicMax operation on 64-bit integers. This is only supported in compute capabilities 3.5 and later. I've changed the clang default to sm_35. Differential Revision: https://reviews.llvm.org/D40977 Modified: cfe/trunk/CMakeLists.txt Modified: cfe/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=320082&r1=320081&r2=320082&view=diff == --- cfe/trunk/CMakeLists.txt (original) +++ cfe/trunk/CMakeLists.txt Thu Dec 7 12:27:31 2017 @@ -241,14 +241,15 @@ set(CLANG_DEFAULT_OBJCOPY "objcopy" CACH set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING "Default OpenMP runtime used by -fopenmp.") -# OpenMP offloading requires at least sm_30 because we use shuffle instructions -# to generate efficient code for reductions. -set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +# OpenMP offloading requires at least sm_35 because we use shuffle instructions +# to generate efficient code for reductions and the atomicMax instruction on +# 64-bit integers in the implementation of conditional lastprivate. +set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs.") string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}") -if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 30) - message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_30") - set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING +if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35) + message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_35") + set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320091 - [Analysis] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).
Author: eugenezelenko Date: Thu Dec 7 13:55:09 2017 New Revision: 320091 URL: http://llvm.org/viewvc/llvm-project?rev=320091&view=rev Log: [Analysis] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC). Modified: cfe/trunk/include/clang/Analysis/CFG.h cfe/trunk/include/clang/Analysis/CallGraph.h cfe/trunk/include/clang/Analysis/Support/BumpVector.h cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/lib/Analysis/CallGraph.cpp Modified: cfe/trunk/include/clang/Analysis/CFG.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=320091&r1=320090&r2=320091&view=diff == --- cfe/trunk/include/clang/Analysis/CFG.h (original) +++ cfe/trunk/include/clang/Analysis/CFG.h Thu Dec 7 13:55:09 2017 @@ -1,4 +1,4 @@ -//===--- CFG.h - Classes for representing and building CFGs--*- C++ -*-===// +//===- CFG.h - Classes for representing and building CFGs ---*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -17,38 +17,38 @@ #include "clang/AST/Stmt.h" #include "clang/Analysis/Support/BumpVector.h" -#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/LLVM.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/GraphTraits.h" +#include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Allocator.h" -#include "llvm/Support/Casting.h" #include "llvm/Support/raw_ostream.h" #include #include +#include #include #include +#include namespace clang { - class CXXDestructorDecl; - class Decl; - class Stmt; - class Expr; - class FieldDecl; - class VarDecl; - class CXXCtorInitializer; - class CXXBaseSpecifier; - class CXXBindTemporaryExpr; - class CFG; - class PrinterHelper; - class LangOptions; - class ASTContext; - class CXXRecordDecl; - class CXXDeleteExpr; - class CXXNewExpr; - class BinaryOperator; + +class ASTContext; +class BinaryOperator; +class CFG; +class CXXBaseSpecifier; +class CXXBindTemporaryExpr; +class CXXCtorInitializer; +class CXXDeleteExpr; +class CXXDestructorDecl; +class CXXNewExpr; +class CXXRecordDecl; +class Decl; +class FieldDecl; +class LangOptions; +class VarDecl; /// CFGElement - Represents a top-level expression in a basic block. class CFGElement { @@ -76,14 +76,14 @@ protected: llvm::PointerIntPair Data2; CFGElement(Kind kind, const void *Ptr1, const void *Ptr2 = nullptr) -: Data1(const_cast(Ptr1), ((unsigned) kind) & 0x3), - Data2(const_cast(Ptr2), (((unsigned) kind) >> 2) & 0x3) { + : Data1(const_cast(Ptr1), ((unsigned) kind) & 0x3), +Data2(const_cast(Ptr2), (((unsigned) kind) >> 2) & 0x3) { assert(getKind() == kind); } - CFGElement() {} -public: + CFGElement() = default; +public: /// \brief Convert to the specified CFGElement type, asserting that this /// CFGElement is of the desired type. template @@ -125,7 +125,9 @@ public: private: friend class CFGElement; - CFGStmt() {} + + CFGStmt() = default; + static bool isKind(const CFGElement &E) { return E.getKind() == Statement; } @@ -144,7 +146,9 @@ public: private: friend class CFGElement; - CFGInitializer() {} + + CFGInitializer() = default; + static bool isKind(const CFGElement &E) { return E.getKind() == Initializer; } @@ -163,7 +167,9 @@ public: private: friend class CFGElement; - CFGNewAllocator() {} + + CFGNewAllocator() = default; + static bool isKind(const CFGElement &elem) { return elem.getKind() == NewAllocator; } @@ -177,19 +183,20 @@ private: /// entered. class CFGLoopExit : public CFGElement { public: -explicit CFGLoopExit(const Stmt *stmt) -: CFGElement(LoopExit, stmt) {} + explicit CFGLoopExit(const Stmt *stmt) : CFGElement(LoopExit, stmt) {} -const Stmt *getLoopStmt() const { - return static_cast(Data1.getPointer()); -} + const Stmt *getLoopStmt() const { +return static_cast(Data1.getPointer()); + } private: -friend class CFGElement; -CFGLoopExit() {} -static bool isKind(const CFGElement &elem) { - return elem.getKind() == LoopExit; -} + friend class CFGElement; + + CFGLoopExit() = default; + + static bool isKind(const CFGElement &elem) { +return elem.getKind() == LoopExit; + } }; /// Represents the point where the lifetime of an automatic object ends @@ -208,7 +215,9 @@ public: private: friend class CFGElement; - CFGLifetimeEnds() {} + + CFGLifetimeEnds() = default; + static bool isKind(const CFGElement &elem) { return elem.getKind() == LifetimeEnds; } @@ -218,7 +227,8 @@ private: /// by compiler on various occasions. class CFGImplicitDtor : public CFGElement { protected: - CFGImplicitDtor() {} + CFGImplicitDtor() = default; + CFGImplicitDtor(Kind kind, const void *data1, const void *data2
[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks
vsk updated this revision to Diff 126037. vsk added a comment. Thanks for your feedback. - Give up on 0-sized types. - Give up on pass_object_size(2 | 3). https://reviews.llvm.org/D40940 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.h test/CodeGen/ubsan-pass-object-size.c Index: test/CodeGen/ubsan-pass-object-size.c === --- /dev/null +++ test/CodeGen/ubsan-pass-object-size.c @@ -0,0 +1,54 @@ +// RUN: %clang_cc1 %s -emit-llvm -w -triple x86_64-apple-darwin10 -fsanitize=array-bounds -o - | FileCheck %s + +// CHECK-LABEL: define i32 @foo( +int foo(int *const p __attribute__((pass_object_size(0))), int n) { + int x = (p)[n]; + + // CHECK: [[SIZE_ALLOCA:%.*]] = alloca i64, align 8 + // CHECK: store i64 %{{.*}}, i64* [[SIZE_ALLOCA]], align 8 + // CHECK: [[LOAD_SIZE:%.*]] = load i64, i64* [[SIZE_ALLOCA]], align 8, !nosanitize + // CHECK-NEXT: [[SCALED_SIZE:%.*]] = udiv i64 [[LOAD_SIZE]], 4, !nosanitize + // CHECK-NEXT: [[SEXT_N:%.*]] = sext i32 %{{.*}} to i64, !nosanitize + // CHECK-NEXT: [[ICMP:%.*]] = icmp ult i64 [[SEXT_N]], [[SCALED_SIZE]], !nosanitize + // CHECK-NEXT: br i1 [[ICMP]], {{.*}} !nosanitize + // CHECK: __ubsan_handle_out_of_bounds + + { +int **p = &p; // Shadow the parameter. The pass_object_size info is lost. +// CHECK-NOT: __ubsan_handle_out_of_bounds +x = *p[n]; + } + + // CHECK: ret i32 + return x; +} + +typedef struct {} ZeroSizedType; + +// CHECK-LABEL: define void @bar( +ZeroSizedType bar(ZeroSizedType *const p __attribute__((pass_object_size(0))), int n) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret void + return p[n]; +} + +// CHECK-LABEL: define i32 @baz( +int baz(int *const p __attribute__((pass_object_size(1))), int n) { + // CHECK: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} + +// CHECK-LABEL: define i32 @mat( +int mat(int *const p __attribute__((pass_object_size(2))), int n) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} + +// CHECK-LABEL: define i32 @pat( +int pat(int *const p __attribute__((pass_object_size(3))), int n) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} Index: lib/CodeGen/CodeGenFunction.h === --- lib/CodeGen/CodeGenFunction.h +++ lib/CodeGen/CodeGenFunction.h @@ -3932,6 +3932,10 @@ LValueBaseInfo *BaseInfo = nullptr, TBAAAccessInfo *TBAAInfo = nullptr); + /// If \p E references a parameter with pass_object_size info, load the + /// object size and divide by the size of \p EltTy. Otherwise return null. + llvm::Value *LoadPassedObjectSize(const Expr *E, QualType EltTy); + void EmitSanitizerStatReport(llvm::SanitizerStatKind SSK); private: Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -814,6 +814,45 @@ return false; } +llvm::Value *CodeGenFunction::LoadPassedObjectSize(const Expr *E, + QualType EltTy) { + ASTContext &C = getContext(); + uint64_t EltSize = C.getTypeSizeInChars(EltTy).getQuantity(); + if (!EltSize) +return nullptr; + + auto *ArrayDeclRef = dyn_cast(E->IgnoreParenImpCasts()); + if (!ArrayDeclRef) +return nullptr; + + auto *ParamDecl = dyn_cast(ArrayDeclRef->getDecl()); + if (!ParamDecl) +return nullptr; + + auto *POSAttr = ParamDecl->getAttr(); + if (!POSAttr) +return nullptr; + + // Don't load the size if it's a lower bound. + int POSType = POSAttr->getType(); + if (POSType != 0 && POSType != 1) +return nullptr; + + // Find the implicit size parameter. + auto PassedSizeIt = SizeArguments.find(ParamDecl); + if (PassedSizeIt == SizeArguments.end()) +return nullptr; + + const ImplicitParamDecl *PassedSizeDecl = PassedSizeIt->second; + assert(LocalDeclMap.count(PassedSizeDecl) && "Passed size not loadable"); + Address AddrOfSize = LocalDeclMap.find(PassedSizeDecl)->second; + llvm::Value *SizeInBytes = EmitLoadOfScalar(AddrOfSize, /*Volatile=*/false, + C.getSizeType(), E->getExprLoc()); + llvm::Value *SizeOfElement = + llvm::ConstantInt::get(SizeInBytes->getType(), EltSize); + return Builder.CreateUDiv(SizeInBytes, SizeOfElement); +} + /// If Base is known to point to the start of an array, return the length of /// that array. Return 0 if the length cannot be determined. static llvm::Value *getArrayIndexingBound( @@ -835,9 +874,16 @@ return CGF.Builder.getInt(CAT->getSize()); else if (const auto *VAT = dyn_cast(AT)) return CGF.getVLASize(VAT).first; + // Ignore pass_object_size here. It's not applicable on decayed pointers. } } + QualType EltTy{Base->getType()->getPointeeOrArrayEle
[PATCH] D35181: Defer addition of keywords to identifier table when loading AST
jklaehn added a comment. Ping, can you take another look? https://reviews.llvm.org/D35181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D40671#947762, @xgsa wrote: > So can this patch be submitted? Should I do something to make it happen? There are still some outstanding concerns around the documentation wording, but once those are resolved it should be ready to commit. If you don't have commit access, I can commit on your behalf -- just let me know. Comment at: docs/clang-tidy/index.rst:254-255 +While :program:`clang-tidy` diagnostics are intended to call out code that does +not adhere to a coding standard, or is otherwise problematic in some way, there +are times when it is more appropriate to silence the diagnostic instead of +changing the semantics of the code. The preferable way of doing this is using xgsa wrote: > aaron.ballman wrote: > > I would reword this somewhat now. I would put a period before ", there are > > times" and then move that whole "there are times" clause below. > I tried to move the "there are times"-clause below, but in this case "The > preferable way of doing this is using..." becomes unclear, because it tries > to explain the way of doing something without naming the purpose that is > expected to achieve. So I suppose, it is necessary to place "However, there > are times when it is more appropriate to silence the diagnostic instead of > changing the semantics of the code" here. Am I missing something? The current formatting of this flows strangely. It starts by saying clang-tidy calls out problems, says there are times when it is more appropriate to silence the diagnostic without changing the code, then says the preferred way is to change the code, then says you can NOLINT it if you don't want to change the code. I'd prefer if the flow was: clang-tidy calls out problems, the preferred way is to change the code, but there are still times when changing the code is not helpful and thus you can use NOLINT. https://reviews.llvm.org/D40671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r320096 - Creating release candidate rc3 from release_501 branch
Author: tstellar Date: Thu Dec 7 14:10:39 2017 New Revision: 320096 URL: http://llvm.org/viewvc/llvm-project?rev=320096&view=rev Log: Creating release candidate rc3 from release_501 branch Added: libcxx/tags/RELEASE_501/rc3/ (props changed) - copied from r320095, libcxx/branches/release_50/ Propchange: libcxx/tags/RELEASE_501/rc3/ -- --- svn:mergeinfo (added) +++ svn:mergeinfo Thu Dec 7 14:10:39 2017 @@ -0,0 +1,2 @@ +/libcxx/branches/apple:136569-137939 +/libcxx/trunk:309296,309307,309474,309838,309851,309917,309920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxxabi] r320097 - Creating release candidate rc3 from release_501 branch
Author: tstellar Date: Thu Dec 7 14:10:43 2017 New Revision: 320097 URL: http://llvm.org/viewvc/llvm-project?rev=320097&view=rev Log: Creating release candidate rc3 from release_501 branch Added: libcxxabi/tags/RELEASE_501/rc3/ - copied from r320096, libcxxabi/branches/release_50/ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r320103 - Creating release candidate rc3 from release_501 branch
Author: tstellar Date: Thu Dec 7 14:11:09 2017 New Revision: 320103 URL: http://llvm.org/viewvc/llvm-project?rev=320103&view=rev Log: Creating release candidate rc3 from release_501 branch Added: libunwind/tags/RELEASE_501/rc3/ (props changed) - copied from r320102, libunwind/branches/release_50/ Propchange: libunwind/tags/RELEASE_501/rc3/ -- svn:mergeinfo = /libunwind/trunk:308871,309147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40937: [clang-tidy] Infinite loop checker
aaron.ballman added a reviewer: dcoughlin. aaron.ballman added a subscriber: dcoughlin. aaron.ballman added a comment. I share the concerns that @Eugene.Zelenko brought up regarding this not being in the analyzer. This is a path sensitive problem that I'm not certain clang-tidy is the best home for. You cover many of the simple cases, but fail to cover cases like nested loops, unsigned integer wrapping, non-integral types, function calls, globals, etc. I'm not certain this check will generalize over time as part of clang-tidy in the way it would in the analyzer. I'd be curious to hear what @alexfh and @dcoughlin think, though. https://reviews.llvm.org/D40937 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40948: Switch Clang's default C++ language target to C++14
t.p.northover added a comment. Thanks Richard. I'll file the bugs tomorrow for the issues you suggest. Do you see either of them blocking the change to C++14 as a default? On a scale of "no", "no but I want a commitment to fix them" and "yes" sort of thing. Tonight I've just got one comment that may or may not be useful context before I give you a proper answer tomorrow: Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:1-2 -// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -o %t -// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -O2 -disable-llvm-passes -emit-llvm -o %t.opt +// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -emit-llvm -o %t +// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 -disable-llvm-passes -emit-llvm -o %t.opt // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t rsmith wrote: > Why does this one need a -std= flag? It's a bit late here so I'll just give the proximal cause this evening in case that makes it obvious to you. I'll dig deeper tomorrow if not. In this particular case (without the std flag) on the -O2 run line the "vtable for Test11::D" does not get marked "available_externally". The diff on line 1 is definitely unneeded. Comment at: clang/test/Parser/cxx1z-nested-namespace-definition.cpp:3-4 +// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98 // RUN: not %clang_cc1 -x c++ -fixit %t -Werror -DFIXIT // RUN: %clang_cc1 -x c++ %t -DFIXIT // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -Wc++14-compat rsmith wrote: > I think these two should also be `-std=c++98`, right? Probably; I'll convince myself of that fact tomorrow. Comment at: clang/test/SemaCXX/unknown-type-name.cpp:110-111 // FIXME: We can tell this was intended to be a function because it does not //have a dependent nested name specifier. +template int i(T::type, int()); rsmith wrote: > Maybe delete this (incorrect in C++14) FIXME? Sounds sensible. Repository: rC Clang https://reviews.llvm.org/D40948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits