[PATCH] D47757: [Sema] Diagnose unavailable aligned deallocation functions called from deleting destructors.
EricWF added a comment. I think we should sink the `DiagnoseUnavailableAlignedAllocation` into `DiagnoseUseOfDecl`, and then adds call's in the few places they're needed. Most paths we care about already pass through there. Comment at: include/clang/Sema/Sema.h:5169 + void diagnoseUnavailableAlignedAllocation(const FunctionDecl , +SourceLocation Loc, bool IsDelete); `DiagnoseUnavailableAlignedAllocation`. Also, I think we can drop the `IsDelete` parameter and instead deduce it using `FD.getDeclName().getCXXOverloadedOperator()`. Repository: rC Clang https://reviews.llvm.org/D47757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47802: Allow std::vector to move construct its allocator
fidget324 abandoned this revision. fidget324 added a comment. That's fine. Your patch was much more thorough, anyways. Repository: rCXX libc++ https://reviews.llvm.org/D47802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47758: [Fuchsia] Include install-distribution-stripped in bootstrap targets
This revision was automatically updated to reflect the committed changes. Closed by commit rL334063: [Fuchsia] Include install-distribution-stripped in bootstrap targets (authored by phosek, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47758?vs=149898=150070#toc Repository: rL LLVM https://reviews.llvm.org/D47758 Files: cfe/trunk/cmake/caches/Fuchsia.cmake Index: cfe/trunk/cmake/caches/Fuchsia.cmake === --- cfe/trunk/cmake/caches/Fuchsia.cmake +++ cfe/trunk/cmake/caches/Fuchsia.cmake @@ -38,6 +38,7 @@ clang-test-depends distribution install-distribution + install-distribution-stripped clang CACHE STRING "") get_cmake_property(variableNames VARIABLES) Index: cfe/trunk/cmake/caches/Fuchsia.cmake === --- cfe/trunk/cmake/caches/Fuchsia.cmake +++ cfe/trunk/cmake/caches/Fuchsia.cmake @@ -38,6 +38,7 @@ clang-test-depends distribution install-distribution + install-distribution-stripped clang CACHE STRING "") get_cmake_property(variableNames VARIABLES) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334063 - [Fuchsia] Include install-distribution-stripped in bootstrap targets
Author: phosek Date: Tue Jun 5 22:18:39 2018 New Revision: 334063 URL: http://llvm.org/viewvc/llvm-project?rev=334063=rev Log: [Fuchsia] Include install-distribution-stripped in bootstrap targets This enables the use of install-distribution-stripped target in the 2-stage builds. Differential Revision: https://reviews.llvm.org/D47758 Modified: cfe/trunk/cmake/caches/Fuchsia.cmake Modified: cfe/trunk/cmake/caches/Fuchsia.cmake URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia.cmake?rev=334063=334062=334063=diff == --- cfe/trunk/cmake/caches/Fuchsia.cmake (original) +++ cfe/trunk/cmake/caches/Fuchsia.cmake Tue Jun 5 22:18:39 2018 @@ -38,6 +38,7 @@ set(CLANG_BOOTSTRAP_TARGETS clang-test-depends distribution install-distribution + install-distribution-stripped clang CACHE STRING "") get_cmake_property(variableNames VARIABLES) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45884: [Sema] Fix parsing of anonymous union in language linkage specification
This revision was automatically updated to reflect the committed changes. Closed by commit rC334062: [Sema] Fix parsing of anonymous union in language linkage specification (authored by jkorous, committed by ). Changed prior to commit: https://reviews.llvm.org/D45884?vs=143736=150069#toc Repository: rC Clang https://reviews.llvm.org/D45884 Files: lib/Sema/SemaDecl.cpp test/SemaCXX/anonymous-union-export.cpp test/SemaCXX/anonymous-union.cpp Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -4642,12 +4642,14 @@ unsigned DiagID; if (Record->isUnion()) { // C++ [class.union]p6: + // C++17 [class.union.anon]p2: // Anonymous unions declared in a named namespace or in the // global namespace shall be declared static. + DeclContext *OwnerScope = Owner->getRedeclContext(); if (DS.getStorageClassSpec() != DeclSpec::SCS_static && - (isa(Owner) || - (isa(Owner) && -cast(Owner)->getDeclName( { + (OwnerScope->isTranslationUnit() || + (OwnerScope->isNamespace() && +!cast(OwnerScope)->isAnonymousNamespace( { Diag(Record->getLocation(), diag::err_anonymous_union_not_static) << FixItHint::CreateInsertion(Record->getLocation(), "static "); Index: test/SemaCXX/anonymous-union.cpp === --- test/SemaCXX/anonymous-union.cpp +++ test/SemaCXX/anonymous-union.cpp @@ -80,6 +80,10 @@ float float_val; }; +extern "C++" { +union { }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} +} + static union { int int_val2; // expected-note{{previous definition is here}} float float_val2; Index: test/SemaCXX/anonymous-union-export.cpp === --- test/SemaCXX/anonymous-union-export.cpp +++ test/SemaCXX/anonymous-union-export.cpp @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -std=c++17 -fmodules-ts -emit-obj -verify %s + +export module M; +export { +union { bool a; }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} +} Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -4642,12 +4642,14 @@ unsigned DiagID; if (Record->isUnion()) { // C++ [class.union]p6: + // C++17 [class.union.anon]p2: // Anonymous unions declared in a named namespace or in the // global namespace shall be declared static. + DeclContext *OwnerScope = Owner->getRedeclContext(); if (DS.getStorageClassSpec() != DeclSpec::SCS_static && - (isa(Owner) || - (isa(Owner) && -cast(Owner)->getDeclName( { + (OwnerScope->isTranslationUnit() || + (OwnerScope->isNamespace() && +!cast(OwnerScope)->isAnonymousNamespace( { Diag(Record->getLocation(), diag::err_anonymous_union_not_static) << FixItHint::CreateInsertion(Record->getLocation(), "static "); Index: test/SemaCXX/anonymous-union.cpp === --- test/SemaCXX/anonymous-union.cpp +++ test/SemaCXX/anonymous-union.cpp @@ -80,6 +80,10 @@ float float_val; }; +extern "C++" { +union { }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} +} + static union { int int_val2; // expected-note{{previous definition is here}} float float_val2; Index: test/SemaCXX/anonymous-union-export.cpp === --- test/SemaCXX/anonymous-union-export.cpp +++ test/SemaCXX/anonymous-union-export.cpp @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -std=c++17 -fmodules-ts -emit-obj -verify %s + +export module M; +export { +union { bool a; }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334062 - [Sema] Fix parsing of anonymous union in language linkage specification
Author: jkorous Date: Tue Jun 5 22:16:34 2018 New Revision: 334062 URL: http://llvm.org/viewvc/llvm-project?rev=334062=rev Log: [Sema] Fix parsing of anonymous union in language linkage specification C++17 [dcl.link]p4: A linkage specification does not establish a scope. C++17 [class.union.anon]p2: Namespace level anonymous unions shall be declared static. Differential Revision: https://reviews.llvm.org/D45884 rdar://problem/37545925 Added: cfe/trunk/test/SemaCXX/anonymous-union-export.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaCXX/anonymous-union.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=334062=334061=334062=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jun 5 22:16:34 2018 @@ -4642,12 +4642,14 @@ Decl *Sema::BuildAnonymousStructOrUnion( unsigned DiagID; if (Record->isUnion()) { // C++ [class.union]p6: + // C++17 [class.union.anon]p2: // Anonymous unions declared in a named namespace or in the // global namespace shall be declared static. + DeclContext *OwnerScope = Owner->getRedeclContext(); if (DS.getStorageClassSpec() != DeclSpec::SCS_static && - (isa(Owner) || - (isa(Owner) && -cast(Owner)->getDeclName( { + (OwnerScope->isTranslationUnit() || + (OwnerScope->isNamespace() && +!cast(OwnerScope)->isAnonymousNamespace( { Diag(Record->getLocation(), diag::err_anonymous_union_not_static) << FixItHint::CreateInsertion(Record->getLocation(), "static "); Added: cfe/trunk/test/SemaCXX/anonymous-union-export.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/anonymous-union-export.cpp?rev=334062=auto == --- cfe/trunk/test/SemaCXX/anonymous-union-export.cpp (added) +++ cfe/trunk/test/SemaCXX/anonymous-union-export.cpp Tue Jun 5 22:16:34 2018 @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -std=c++17 -fmodules-ts -emit-obj -verify %s + +export module M; +export { +union { bool a; }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} +} Modified: cfe/trunk/test/SemaCXX/anonymous-union.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/anonymous-union.cpp?rev=334062=334061=334062=diff == --- cfe/trunk/test/SemaCXX/anonymous-union.cpp (original) +++ cfe/trunk/test/SemaCXX/anonymous-union.cpp Tue Jun 5 22:16:34 2018 @@ -80,6 +80,10 @@ union { // expected-error{{anonymous uni float float_val; }; +extern "C++" { +union { }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} +} + static union { int int_val2; // expected-note{{previous definition is here}} float float_val2; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334061 - [X86] Move the vec_set/vec_ext builtins for 64-bit elements to BuiltinsX86_64.def.
Author: ctopper Date: Tue Jun 5 21:51:52 2018 New Revision: 334061 URL: http://llvm.org/viewvc/llvm-project?rev=334061=rev Log: [X86] Move the vec_set/vec_ext builtins for 64-bit elements to BuiltinsX86_64.def. The instructions these correspond to and the intrinsics that use them are only available in 64-bit mode. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=334061=334060=334061=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Jun 5 21:51:52 2018 @@ -272,7 +272,6 @@ TARGET_BUILTIN(__builtin_ia32_packsswb12 TARGET_BUILTIN(__builtin_ia32_packssdw128, "V8sV4iV4i", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_packuswb128, "V16cV8sV8s", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_pmulhuw128, "V8sV8sV8s", "nc", "sse2") -TARGET_BUILTIN(__builtin_ia32_vec_ext_v2di, "LLiV2LLiIi", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_vec_ext_v4si, "iV4iIi", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_vec_ext_v4sf, "fV4fIi", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_vec_ext_v8hi, "sV8sIi", "nc", "sse2") @@ -395,7 +394,6 @@ TARGET_BUILTIN(__builtin_ia32_phminposuw TARGET_BUILTIN(__builtin_ia32_vec_ext_v16qi, "cV16cIi", "nc", "sse4.1") TARGET_BUILTIN(__builtin_ia32_vec_set_v16qi, "V16cV16ccIi", "nc", "sse4.1") TARGET_BUILTIN(__builtin_ia32_vec_set_v4si, "V4iV4iiIi", "nc", "sse4.1") -TARGET_BUILTIN(__builtin_ia32_vec_set_v2di, "V2LLiV2LLiLLiIi", "nc", "sse4.1") // SSE 4.2 TARGET_BUILTIN(__builtin_ia32_pcmpistrm128, "V16cV16cV16cIc", "nc", "sse4.2") @@ -531,11 +529,9 @@ TARGET_BUILTIN(__builtin_ia32_maskstorep TARGET_BUILTIN(__builtin_ia32_vec_ext_v32qi, "cV32cIi", "nc", "avx") TARGET_BUILTIN(__builtin_ia32_vec_ext_v16hi, "sV16sIi", "nc", "avx") TARGET_BUILTIN(__builtin_ia32_vec_ext_v8si, "iV8iIi", "nc", "avx") -TARGET_BUILTIN(__builtin_ia32_vec_ext_v4di, "LLiV4LLiIi", "nc", "avx") TARGET_BUILTIN(__builtin_ia32_vec_set_v32qi, "V32cV32ccIi", "nc", "avx") TARGET_BUILTIN(__builtin_ia32_vec_set_v16hi, "V16sV16ssIi", "nc", "avx") TARGET_BUILTIN(__builtin_ia32_vec_set_v8si, "V8iV8iiIi", "nc", "avx") -TARGET_BUILTIN(__builtin_ia32_vec_set_v4di, "V4LLiV4LLiLLiIi", "nc", "avx") // AVX2 TARGET_BUILTIN(__builtin_ia32_mpsadbw256, "V32cV32cV32cIc", "nc", "avx2") Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=334061=334060=334061=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Tue Jun 5 21:51:52 2018 @@ -49,7 +49,11 @@ TARGET_BUILTIN(__builtin_ia32_cvttss2si6 TARGET_BUILTIN(__builtin_ia32_cvtsd2si64, "LLiV2d", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_cvttsd2si64, "LLiV2d", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_movnti64, "vLLi*LLi", "n", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v2di, "LLiV2LLiIi", "nc", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_set_v2di, "V2LLiV2LLiLLiIi", "nc", "sse4.1") TARGET_BUILTIN(__builtin_ia32_crc32di, "ULLiULLiULLi", "nc", "sse4.2") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v4di, "LLiV4LLiIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_set_v4di, "V4LLiV4LLiLLiIi", "nc", "avx") TARGET_BUILTIN(__builtin_ia32_rdfsbase32, "Ui", "n", "fsgsbase") TARGET_BUILTIN(__builtin_ia32_rdfsbase64, "ULLi", "n", "fsgsbase") TARGET_BUILTIN(__builtin_ia32_rdgsbase32, "Ui", "n", "fsgsbase") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47360: Implement as a copy of .
Quuxplusone planned changes to this revision. Quuxplusone added a comment. Once the dependencies (https://reviews.llvm.org/D47111 and https://reviews.llvm.org/D47358) are merged, I need to update the unit tests in this patch to reflect the unit tests that were committed. Right now, the unit tests in this patch reflect an old, out-of-date snapshot of what had been in those patches a couple weeks ago. Comment at: include/__memory_resource_base:53 +#include +#include +#include This should be `#include <__tuple>` Comment at: include/memory_resource:47 +#include +#include +#include This line should be gone. Repository: rCXX libc++ https://reviews.llvm.org/D47360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46806: Remove unused code from __functional_base. NFC.
Quuxplusone updated this revision to Diff 150067. Quuxplusone edited the summary of this revision. Quuxplusone added a comment. Also, `` doesn't need a full definition of `std::tuple`; just the forward declaration in `<__tuple>` will suffice. Repository: rCXX libc++ https://reviews.llvm.org/D46806 Files: include/__functional_base include/experimental/memory_resource Index: include/experimental/memory_resource === --- include/experimental/memory_resource +++ include/experimental/memory_resource @@ -71,7 +71,7 @@ #include #include #include -#include +#include <__tuple> #include #include #include @@ -96,7 +96,7 @@ } // 8.5, memory.resource -class _LIBCPP_TEMPLATE_VIS memory_resource +class _LIBCPP_TYPE_VIS memory_resource { static const size_t __max_align = alignof(max_align_t); Index: include/__functional_base === --- include/__functional_base +++ include/__functional_base @@ -646,16 +646,6 @@ new (__storage) _Tp (_VSTD::forward<_Args>(__args)..., __a); } -// FIXME: Theis should have a version which takes a non-const alloc. -template -inline _LIBCPP_INLINE_VISIBILITY -void __user_alloc_construct (_Tp *__storage, const _Allocator &__a, _Args &&... __args) -{ -__user_alloc_construct_impl( - __uses_alloc_ctor<_Tp, _Allocator>(), - __storage, __a, _VSTD::forward<_Args>(__args)... -); -} #endif // _LIBCPP_CXX03_LANG _LIBCPP_END_NAMESPACE_STD Index: include/experimental/memory_resource === --- include/experimental/memory_resource +++ include/experimental/memory_resource @@ -71,7 +71,7 @@ #include #include #include -#include +#include <__tuple> #include #include #include @@ -96,7 +96,7 @@ } // 8.5, memory.resource -class _LIBCPP_TEMPLATE_VIS memory_resource +class _LIBCPP_TYPE_VIS memory_resource { static const size_t __max_align = alignof(max_align_t); Index: include/__functional_base === --- include/__functional_base +++ include/__functional_base @@ -646,16 +646,6 @@ new (__storage) _Tp (_VSTD::forward<_Args>(__args)..., __a); } -// FIXME: Theis should have a version which takes a non-const alloc. -template -inline _LIBCPP_INLINE_VISIBILITY -void __user_alloc_construct (_Tp *__storage, const _Allocator &__a, _Args &&... __args) -{ -__user_alloc_construct_impl( - __uses_alloc_ctor<_Tp, _Allocator>(), - __storage, __a, _VSTD::forward<_Args>(__args)... -); -} #endif // _LIBCPP_CXX03_LANG _LIBCPP_END_NAMESPACE_STD ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.
EricWF added a comment. In https://reviews.llvm.org/D45015#1123164, @ahatanak wrote: > In https://reviews.llvm.org/D45015#1123097, @EricWF wrote: > > > In https://reviews.llvm.org/D45015#1121874, @ahatanak wrote: > > > > > I see, thank you. > > > > > > clang front-end currently fails to issue a warning or error when an > > > aligned allocation/deallocation functions are required but not available > > > in a few cases (e.g., delete called from a deleting destructor, calls to > > > operator or builtin operator new/delete). I suppose those bugs should be > > > fixed in separate patches. > > > > > > I don't think we need to emit warnings from > > `__builtin_operator_new`/`__builtin_operator_delete`. Libc++ is the only > > consumer, and I think we can trust it to know what it's doing. > > > Shouldn't clang warn when users explicitly call an aligned builtin operator > new or delete in their code and the OS is too old to support the operator? > > For example: > > typedef __SIZE_TYPE__ size_t; > namespace std { > enum class align_val_t : size_t {}; > } > > int main() { > void *p = __builtin_operator_new(100, std::align_val_t(32)); > return 0; > } > Yeah, I think you're right. Initially I thought `__builtin_operator_new` was documented as being intended for usage only in the STL, but it doesn't quite say that. I have a patch which moves the checks into `DiagnoseUseOfDecl`, which should catch all of the cases we haven't already handled. https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.
ahatanak added a comment. In https://reviews.llvm.org/D45015#1123097, @EricWF wrote: > In https://reviews.llvm.org/D45015#1121874, @ahatanak wrote: > > > I see, thank you. > > > > clang front-end currently fails to issue a warning or error when an aligned > > allocation/deallocation functions are required but not available in a few > > cases (e.g., delete called from a deleting destructor, calls to operator or > > builtin operator new/delete). I suppose those bugs should be fixed in > > separate patches. > > > I don't think we need to emit warnings from > `__builtin_operator_new`/`__builtin_operator_delete`. Libc++ is the only > consumer, and I think we can trust it to know what it's doing. Shouldn't clang warn when users explicitly call an aligned builtin operator new or delete in their code and the OS is too old to support the operator? For example: typedef __SIZE_TYPE__ size_t; namespace std { enum class align_val_t : size_t {}; } int main() { void *p = __builtin_operator_new(100, std::align_val_t(32)); return 0; } https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47671: [analyzer] Implement copy elision.
NoQ added a comment. In https://reviews.llvm.org/D47671#1122994, @george.karpenkov wrote: > From my understanding, that's just how c++17 is defined, and that's unrelated > to what compiler implementation is used. Yeah, but this patch is specifically about supporting the pre-C++17 AST, where copy elision was already a well-defined concept, but it wasn't mandatory. In C++17 copy-constructors are simply omitted from the AST (but `CXXBindTemporaryExpr` might still be accidentally present, and additionally more different construction syntax patterns may appear, eg. we may elide all the way through ?: operators). Support for C++17 was added in the previous patches. This patch addresses pre-C++17 AST that does include copy-constructors, but they are marked as "(elidable)". They are here because whether to elide them or not was implementation-defined, and even if the elided constructor had arbitrary side effects the compiler was still allowed to choose not to elide it. In this case we decided not to skip in in CFG, but only skip it in the analyzer, because that'd allow the Environment to work with the AST in a straightforward and intuitive manner as it always did. Hence the patch. So there are two reasons why to add an option: - To be able to disable the new feature as a workaround to a bug in this feature. - To be able to actually maintain two different behaviors forever as an opt-in feature. I'm totally fine with the former. I'm not sure if it'll be easy and worthwile to try doing the latter. We might as well try and see if it will be easy to maintain. I hope it'll mostly work because most syntax patterns with elidable constructors will also appear anyway with non-elidable constructors, albeit much more rarely. I guess i'll add the flag and see how it goes. Repository: rC Clang https://reviews.llvm.org/D47671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47672: [Headers] Add _Interlocked*_HLEAcquire/_HLERelease
rnk added a comment. In https://reviews.llvm.org/D47672#1122013, @ethanhs wrote: > In https://reviews.llvm.org/D47672#1121181, @rnk wrote: > > > > > > > > > They are a hint to the processor that this is a short critical section, and > > it is likely that the entire critical section can be entered, run, and > > committed to memory before another thread needs to use the memory used by > > the critical section. Your implementation doesn't add these prefixes, but > > the code will execute correctly. This functionality relies on TSX, which it > > looks like Intel added some time in 2014, so it's probably widely available. > > > > Personally, I don't want to implement these intrinsics this way. We've > > already implemented intrinsics "our way" instead of doing whatever Visual C > > does, and it just leads to developer confusion when they discover that the > > compiler didn't emit the instruction they want. For example, the rotate > > intrinsics often don't work (https://llvm.org/pr37387) and the > > bittestandset (bts) intrinsics are just broken (http://llvm.org/pr33188). > > I see. AIUI, the HLE versions just add optional hints, so there should not be > any functional differences beyond using the hints or not (theses are > optimization hints, yes?). Perhaps I misunderstood the documentation. Yes, as far as I can tell. >> are there any plans for representing HLE hints on atomic instructions in >> LLVM IR? > > This would be quite nice to have, the main reason I implemented these > functions this way is I don't have the bandwidth at the moment to correctly > implement HLE in LLVM/Clang, nor some of the requisite knowledge I'm sure. I > also am keen to have Python build with clang-cl on Windows :) But I > definitely can understand wanting to do things correctly from the start. Yeah, I was kind of hoping an HLE expert would appear and suggest a straightforward implementation path. > I am going to the LLVM Bay Area monthly social Thursday so if someone wants > to discuss this there I'd be more than happy to. I should also be able to make it this week, hope to see you there! Repository: rC Clang https://reviews.llvm.org/D47672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334060 - Fix std::tuple errors
Author: rnk Date: Tue Jun 5 18:44:10 2018 New Revision: 334060 URL: http://llvm.org/viewvc/llvm-project?rev=334060=rev Log: Fix std::tuple errors Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=334060=334059=334060=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Jun 5 18:44:10 2018 @@ -492,30 +492,30 @@ static std::tuple getBitTestActionSizeAndLocking(unsigned BuiltinID) { switch (BuiltinID) { case Builtin::BI_bittest: -return {'\0', 'l', false}; +return std::make_tuple('\0', 'l', false); case Builtin::BI_bittestandcomplement: -return {'c', 'l', false}; +return std::make_tuple('c', 'l', false); case Builtin::BI_bittestandreset: -return {'r', 'l', false}; +return std::make_tuple('r', 'l', false); case Builtin::BI_bittestandset: -return {'s', 'l', false}; +return std::make_tuple('s', 'l', false); case Builtin::BI_interlockedbittestandreset: -return {'r', 'l', /*Locked=*/true}; +return std::make_tuple('r', 'l', /*Locked=*/true); case Builtin::BI_interlockedbittestandset: -return {'s', 'l', /*Locked=*/true}; +return std::make_tuple('s', 'l', /*Locked=*/true); case Builtin::BI_bittest64: -return {'\0', 'q', false}; +return std::make_tuple('\0', 'q', false); case Builtin::BI_bittestandcomplement64: -return {'c', 'q', false}; +return std::make_tuple('c', 'q', false); case Builtin::BI_bittestandreset64: -return {'r', 'q', false}; +return std::make_tuple('r', 'q', false); case Builtin::BI_bittestandset64: -return {'s', 'q', false}; +return std::make_tuple('s', 'q', false); case Builtin::BI_interlockedbittestandreset64: -return {'r', 'q', /*Locked=*/true}; +return std::make_tuple('r', 'q', /*Locked=*/true); case Builtin::BI_interlockedbittestandset64: -return {'s', 'q', /*Locked=*/true}; +return std::make_tuple('s', 'q', /*Locked=*/true); } llvm_unreachable("expected only bittest builtins"); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334059 - Implement bittest intrinsics generically for non-x86 platforms
Author: rnk Date: Tue Jun 5 18:35:08 2018 New Revision: 334059 URL: http://llvm.org/viewvc/llvm-project?rev=334059=rev Log: Implement bittest intrinsics generically for non-x86 platforms I tested these locally on an x86 machine by disabling the inline asm codepath and confirming that it does the same bitflips as we do with the inline asm. Addresses code review feedback. Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/test/CodeGen/bittest-intrin.c Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=334059=334058=334059=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Jun 5 18:35:08 2018 @@ -484,11 +484,48 @@ CodeGenFunction::emitBuiltinObjectSize(c return Builder.CreateCall(F, {Ptr, Min, NullIsUnknown}); } -static RValue EmitBitTestIntrinsic(CodeGenFunction , const CallExpr *E, - char TestAnd, char Size, - bool Locked = false) { - Value *BitBase = CGF.EmitScalarExpr(E->getArg(0)); - Value *BitPos = CGF.EmitScalarExpr(E->getArg(1)); +// Get properties of an X86 BT* assembly instruction. The first returned value +// is the action character code, which can be for complement, reset, or set. The +// second is the size suffix which our assembler needs. The last is whether to +// add the lock prefix. +static std::tuple +getBitTestActionSizeAndLocking(unsigned BuiltinID) { + switch (BuiltinID) { + case Builtin::BI_bittest: +return {'\0', 'l', false}; + case Builtin::BI_bittestandcomplement: +return {'c', 'l', false}; + case Builtin::BI_bittestandreset: +return {'r', 'l', false}; + case Builtin::BI_bittestandset: +return {'s', 'l', false}; + case Builtin::BI_interlockedbittestandreset: +return {'r', 'l', /*Locked=*/true}; + case Builtin::BI_interlockedbittestandset: +return {'s', 'l', /*Locked=*/true}; + + case Builtin::BI_bittest64: +return {'\0', 'q', false}; + case Builtin::BI_bittestandcomplement64: +return {'c', 'q', false}; + case Builtin::BI_bittestandreset64: +return {'r', 'q', false}; + case Builtin::BI_bittestandset64: +return {'s', 'q', false}; + case Builtin::BI_interlockedbittestandreset64: +return {'r', 'q', /*Locked=*/true}; + case Builtin::BI_interlockedbittestandset64: +return {'s', 'q', /*Locked=*/true}; + } + llvm_unreachable("expected only bittest builtins"); +} + +static RValue EmitX86BitTestIntrinsic(CodeGenFunction , unsigned BuiltinID, + const CallExpr *E, Value *BitBase, + Value *BitPos) { + char Action, Size; + bool Locked; + std::tie(Action, Size, Locked) = getBitTestActionSizeAndLocking(BuiltinID); // Build the assembly. SmallString<64> Asm; @@ -496,12 +533,12 @@ static RValue EmitBitTestIntrinsic(CodeG if (Locked) AsmOS << "lock "; AsmOS << "bt"; - if (TestAnd) -AsmOS << TestAnd; + if (Action) +AsmOS << Action; AsmOS << Size << " $2, ($1)\n\tsetc ${0:b}"; // Build the constraints. FIXME: We should support immediates when possible. - std::string Constraints = "=r,r,r,~{cc},~{flags},~{memory},~{fpsr}"; + std::string Constraints = "=r,r,r,~{cc},~{flags},~{fpsr}"; llvm::IntegerType *IntType = llvm::IntegerType::get( CGF.getLLVMContext(), CGF.getContext().getTypeSize(E->getArg(1)->getType())); @@ -515,6 +552,97 @@ static RValue EmitBitTestIntrinsic(CodeG return RValue::get(CS.getInstruction()); } +/// Emit a _bittest* intrinsic. These intrinsics take a pointer to an array of +/// bits and a bit position and read and optionally modify the bit at that +/// position. The position index can be arbitrarily large, i.e. it can be larger +/// than 31 or 63, so we need an indexed load in the general case. +static RValue EmitBitTestIntrinsic(CodeGenFunction , unsigned BuiltinID, + const CallExpr *E) { + Value *BitBase = CGF.EmitScalarExpr(E->getArg(0)); + Value *BitPos = CGF.EmitScalarExpr(E->getArg(1)); + + // X86 has special BT, BTC, BTR, and BTS instructions that handle the array + // indexing operation internally. Use them if possible. + llvm::Triple::ArchType Arch = CGF.getTarget().getTriple().getArch(); + if (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64) +return EmitX86BitTestIntrinsic(CGF, BuiltinID, E, BitBase, BitPos); + + // Otherwise, use generic code to load one byte and test the bit. Use all but + // the bottom three bits as the array index, and the bottom three bits to form + // a mask. + // Bit = BitBaseI8[BitPos >> 3] & (1 << (BitPos & 0x7)) != 0; + Value *ByteIndex = CGF.Builder.CreateAShr( + BitPos, llvm::ConstantInt::get(BitPos->getType(), 3), "bittest.byteidx"); + Value *BitBaseI8 =
[PATCH] D47802: Allow std::vector to move construct its allocator
EricWF added a comment. @fidget324 Thanks for the patch! Unfortunatly I didn't know you were planning on contributing a fix, so I went ahead and fixed it myself in r334053. Repository: rCXX libc++ https://reviews.llvm.org/D47802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47376: [CUDA][HIP] Do not offload for -M
tra added a comment. Just to make it clear, I'm not against making a sensible default choice, but rather want to make sure that it is possible to override it if the user needs to. https://reviews.llvm.org/D47376 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47376: [CUDA][HIP] Do not offload for -M
tra requested changes to this revision. tra added a comment. This revision now requires changes to proceed. I'm not sure this is the right thing to do. What if user explicitly wants device-side dependencies and runs `clang --cuda-device-only -M` ? This patch makes it impossible. I'd rather tell users that they have to explicitly provide *which* compilation they want to generate dependencies for. If they want them for the host, just pass along --cuda-host-only. https://reviews.llvm.org/D47376 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334057 - [X86] Add builtins for vector element insert and extract for different 128 and 256 bit vector types. Use them to implement the extract and insert intrinsics.
Author: ctopper Date: Tue Jun 5 17:24:55 2018 New Revision: 334057 URL: http://llvm.org/viewvc/llvm-project?rev=334057=rev Log: [X86] Add builtins for vector element insert and extract for different 128 and 256 bit vector types. Use them to implement the extract and insert intrinsics. Previously we were just using extended vector operations in the header file. This unfortunately allowed non-constant indices to be used with the intrinsics. This is incompatible with gcc, icc, and MSVC. It also introduces a different performance characteristic because non-constant index gets lowered to a vector store and an element sized load. By adding the builtins we can check for the index to be a constant and ensure its in range of the vector element count. User code still has the option to use extended vector operations themselves if they need non-constant indexing. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/Headers/avxintrin.h cfe/trunk/lib/Headers/emmintrin.h cfe/trunk/lib/Headers/smmintrin.h cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/CodeGen/avx-builtins.c cfe/trunk/test/CodeGen/sse2-builtins.c cfe/trunk/test/CodeGen/sse41-builtins.c cfe/trunk/test/CodeGen/target-features-error-2.c cfe/trunk/test/CodeGen/vector.c Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=334057=334056=334057=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Jun 5 17:24:55 2018 @@ -272,6 +272,11 @@ TARGET_BUILTIN(__builtin_ia32_packsswb12 TARGET_BUILTIN(__builtin_ia32_packssdw128, "V8sV4iV4i", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_packuswb128, "V16cV8sV8s", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_pmulhuw128, "V8sV8sV8s", "nc", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v2di, "LLiV2LLiIi", "nc", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v4si, "iV4iIi", "nc", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v4sf, "fV4fIi", "nc", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v8hi, "sV8sIi", "nc", "sse2") +TARGET_BUILTIN(__builtin_ia32_vec_set_v8hi, "V8sV8ssIi", "nc", "sse2") TARGET_BUILTIN(__builtin_ia32_addsubps, "V4fV4fV4f", "nc", "sse3") TARGET_BUILTIN(__builtin_ia32_addsubpd, "V2dV2dV2d", "nc", "sse3") @@ -387,6 +392,10 @@ TARGET_BUILTIN(__builtin_ia32_ptestc128, TARGET_BUILTIN(__builtin_ia32_ptestnzc128, "iV2LLiV2LLi", "nc", "sse4.1") TARGET_BUILTIN(__builtin_ia32_mpsadbw128, "V16cV16cV16cIc", "nc", "sse4.1") TARGET_BUILTIN(__builtin_ia32_phminposuw128, "V8sV8s", "nc", "sse4.1") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v16qi, "cV16cIi", "nc", "sse4.1") +TARGET_BUILTIN(__builtin_ia32_vec_set_v16qi, "V16cV16ccIi", "nc", "sse4.1") +TARGET_BUILTIN(__builtin_ia32_vec_set_v4si, "V4iV4iiIi", "nc", "sse4.1") +TARGET_BUILTIN(__builtin_ia32_vec_set_v2di, "V2LLiV2LLiLLiIi", "nc", "sse4.1") // SSE 4.2 TARGET_BUILTIN(__builtin_ia32_pcmpistrm128, "V16cV16cV16cIc", "nc", "sse4.2") @@ -519,6 +528,14 @@ TARGET_BUILTIN(__builtin_ia32_maskstorep TARGET_BUILTIN(__builtin_ia32_maskstoreps, "vV4f*V4iV4f", "n", "avx") TARGET_BUILTIN(__builtin_ia32_maskstorepd256, "vV4d*V4LLiV4d", "n", "avx") TARGET_BUILTIN(__builtin_ia32_maskstoreps256, "vV8f*V8iV8f", "n", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v32qi, "cV32cIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v16hi, "sV16sIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v8si, "iV8iIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v4di, "LLiV4LLiIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_set_v32qi, "V32cV32ccIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_set_v16hi, "V16sV16ssIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_set_v8si, "V8iV8iiIi", "nc", "avx") +TARGET_BUILTIN(__builtin_ia32_vec_set_v4di, "V4LLiV4LLiLLiIi", "nc", "avx") // AVX2 TARGET_BUILTIN(__builtin_ia32_mpsadbw256, "V32cV32cV32cIc", "nc", "avx2") Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=334057=334056=334057=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Jun 5 17:24:55 2018 @@ -8778,8 +8778,29 @@ Value *CodeGenFunction::EmitX86BuiltinEx return Builder.CreateBitCast(BuildVector(Ops), llvm::Type::getX86_MMXTy(getLLVMContext())); case X86::BI__builtin_ia32_vec_ext_v2si: -return Builder.CreateExtractElement(Ops[0], - cast(Ops[1])->getZExtValue()); + case X86::BI__builtin_ia32_vec_ext_v16qi: + case X86::BI__builtin_ia32_vec_ext_v8hi: + case X86::BI__builtin_ia32_vec_ext_v4si: + case
[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.
EricWF added a comment. In https://reviews.llvm.org/D45015#1121874, @ahatanak wrote: > I see, thank you. > > clang front-end currently fails to issue a warning or error when an aligned > allocation/deallocation functions are required but not available in a few > cases (e.g., delete called from a deleting destructor, calls to operator or > builtin operator new/delete). I suppose those bugs should be fixed in > separate patches. I don't think we need to emit warnings from `__builtin_operator_new`/`__builtin_operator_delete`. Libc++ is the only consumer, and I think we can trust it to know what it's doing. https://reviews.llvm.org/D45015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r334056 - Fix test failures after r334053.
Author: ericwf Date: Tue Jun 5 17:13:49 2018 New Revision: 334056 URL: http://llvm.org/viewvc/llvm-project?rev=334056=rev Log: Fix test failures after r334053. Modified: libcxx/trunk/test/std/containers/associative/map/map.cons/move.pass.cpp libcxx/trunk/test/std/containers/associative/multimap/multimap.cons/move.pass.cpp libcxx/trunk/test/std/containers/associative/multiset/multiset.cons/move.pass.cpp libcxx/trunk/test/std/containers/associative/set/set.cons/move.pass.cpp libcxx/trunk/test/std/containers/container.requirements/container.requirements.general/allocator_move.pass.cpp libcxx/trunk/test/std/containers/sequences/deque/deque.cons/move.pass.cpp Modified: libcxx/trunk/test/std/containers/associative/map/map.cons/move.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/associative/map/map.cons/move.pass.cpp?rev=334056=334055=334056=diff == --- libcxx/trunk/test/std/containers/associative/map/map.cons/move.pass.cpp (original) +++ libcxx/trunk/test/std/containers/associative/map/map.cons/move.pass.cpp Tue Jun 5 17:13:49 2018 @@ -35,7 +35,7 @@ int main() assert(m.size() == 0); assert(distance(m.begin(), m.end()) == 0); -assert(mo.get_allocator() == A(7)); +assert(mo.get_allocator() == A(test_alloc_base::moved_value)); assert(mo.key_comp() == C(5)); assert(mo.size() == 0); assert(distance(mo.begin(), mo.end()) == 0); @@ -65,7 +65,7 @@ int main() assert(*next(m.begin()) == V(2, 1)); assert(*next(m.begin(), 2) == V(3, 1)); -assert(mo.get_allocator() == A(7)); +assert(mo.get_allocator() == A(test_alloc_base::moved_value)); assert(mo.key_comp() == C(5)); assert(mo.size() == 0); assert(distance(mo.begin(), mo.end()) == 0); Modified: libcxx/trunk/test/std/containers/associative/multimap/multimap.cons/move.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/associative/multimap/multimap.cons/move.pass.cpp?rev=334056=334055=334056=diff == --- libcxx/trunk/test/std/containers/associative/multimap/multimap.cons/move.pass.cpp (original) +++ libcxx/trunk/test/std/containers/associative/multimap/multimap.cons/move.pass.cpp Tue Jun 5 17:13:49 2018 @@ -35,7 +35,7 @@ int main() assert(m.size() == 0); assert(distance(m.begin(), m.end()) == 0); -assert(mo.get_allocator() == A(7)); +assert(mo.get_allocator() == A(test_alloc_base::moved_value)); assert(mo.key_comp() == C(5)); assert(mo.size() == 0); assert(distance(mo.begin(), mo.end()) == 0); @@ -71,7 +71,7 @@ int main() assert(*next(m.begin(), 7) == V(3, 1.5)); assert(*next(m.begin(), 8) == V(3, 2)); -assert(mo.get_allocator() == A(7)); +assert(mo.get_allocator() == A(test_alloc_base::moved_value)); assert(mo.key_comp() == C(5)); assert(mo.size() == 0); assert(distance(mo.begin(), mo.end()) == 0); Modified: libcxx/trunk/test/std/containers/associative/multiset/multiset.cons/move.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/associative/multiset/multiset.cons/move.pass.cpp?rev=334056=334055=334056=diff == --- libcxx/trunk/test/std/containers/associative/multiset/multiset.cons/move.pass.cpp (original) +++ libcxx/trunk/test/std/containers/associative/multiset/multiset.cons/move.pass.cpp Tue Jun 5 17:13:49 2018 @@ -35,7 +35,7 @@ int main() assert(m.size() == 0); assert(distance(m.begin(), m.end()) == 0); -assert(mo.get_allocator() == A(7)); +assert(mo.get_allocator() == A(test_alloc_base::moved_value)); assert(mo.key_comp() == C(5)); assert(mo.size() == 0); assert(distance(mo.begin(), mo.end()) == 0); @@ -72,7 +72,7 @@ int main() assert(*next(m.begin(), 7) == 3); assert(*next(m.begin(), 8) == 3); -assert(mo.get_allocator() == A(7)); +assert(mo.get_allocator() == A(test_alloc_base::moved_value)); assert(mo.key_comp() == C(5)); assert(mo.size() == 0); assert(distance(mo.begin(), mo.end()) == 0); Modified: libcxx/trunk/test/std/containers/associative/set/set.cons/move.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/associative/set/set.cons/move.pass.cpp?rev=334056=334055=334056=diff == --- libcxx/trunk/test/std/containers/associative/set/set.cons/move.pass.cpp (original) +++ libcxx/trunk/test/std/containers/associative/set/set.cons/move.pass.cpp Tue Jun 5 17:13:49 2018 @@ -35,7 +35,7 @@ int main()
[PATCH] D47804: [CUDA] Replace 'nv_weak' attributes in CUDA headers with 'weak'.
tra added a comment. I'll wait to see if that fixes @Hahnfeld's problem. AFAICT, nv_weak is not used for anything interesting other than the device-side runtime stubs that return errors (apparently, until they are linked with the proper device runtime which would have strong version of the symbols. I've tested with 8.0, 9.0 and 9.2 with -fcuda-rdc and all appear to produce .weak variants of those stubs in PTX. One thing that I've noticed that we do differently is that those stubs being present in clang-generated PTX regardless of whether -fcuda-rdc is in effect. Well, there's only so much we can do with preprocessor magic. Other than wasted space, it's benign AFAICT. If it turns out that it's problematic after all, we should be able to disable them away with __attribute__((if_enabled...)). https://reviews.llvm.org/D47804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47804: [CUDA] Replace 'nv_weak' attributes in CUDA headers with 'weak'.
jlebar accepted this revision. jlebar added a comment. This revision is now accepted and ready to land. What could possibly go wrong. https://reviews.llvm.org/D47804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47804: [CUDA] Replace 'nv_weak' attributes in CUDA headers with 'weak'.
tra created this revision. tra added reviewers: Hahnfeld, aaron.ballman, jlebar. Herald added subscribers: bixia, sanjoy. An alternative to implementing nv_weak attribute (https://reviews.llvm.org/D47201). The patch should make runtime sub functions to have .weak attribute in PTX and that should avoid GPU-side linking errors. https://reviews.llvm.org/D47804 Files: clang/lib/Headers/__clang_cuda_runtime_wrapper.h Index: clang/lib/Headers/__clang_cuda_runtime_wrapper.h === --- clang/lib/Headers/__clang_cuda_runtime_wrapper.h +++ clang/lib/Headers/__clang_cuda_runtime_wrapper.h @@ -100,11 +100,17 @@ #include "host_config.h" #include "host_defines.h" +// Temporarily replace "nv_weak" with weak, so __attribute__((nv_weak)) in +// cuda_device_runtime_api.h ends up being __attribute__((weak)) which is the +// functional equivalent of what we need. +#pragma push_macro("nv_weak") +#define nv_weak weak #undef __CUDABE__ #undef __CUDA_LIBDEVICE__ #define __CUDACC__ #include "cuda_runtime.h" +#pragma pop_macro("nv_weak") #undef __CUDACC__ #define __CUDABE__ Index: clang/lib/Headers/__clang_cuda_runtime_wrapper.h === --- clang/lib/Headers/__clang_cuda_runtime_wrapper.h +++ clang/lib/Headers/__clang_cuda_runtime_wrapper.h @@ -100,11 +100,17 @@ #include "host_config.h" #include "host_defines.h" +// Temporarily replace "nv_weak" with weak, so __attribute__((nv_weak)) in +// cuda_device_runtime_api.h ends up being __attribute__((weak)) which is the +// functional equivalent of what we need. +#pragma push_macro("nv_weak") +#define nv_weak weak #undef __CUDABE__ #undef __CUDA_LIBDEVICE__ #define __CUDACC__ #include "cuda_runtime.h" +#pragma pop_macro("nv_weak") #undef __CUDACC__ #define __CUDABE__ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46911: [Fixed Point Arithmetic] Addition of the remaining fixed point types and their saturated equivalents
leonardchan added a comment. @ebevhan Does everything look good in this patch? I'm almost done with the next one. Writing tests for it still. Repository: rC Clang https://reviews.llvm.org/D46911 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
tra added a comment. I've experimented a bit and I think that we may not need this patch at all. As far as I can tell, nv_weak is only applicable to __device__ functions. It's ignored for __global__ kernels and is apparently forbidden for data. For __device__ functions nvcc produces .weak attribute in PTX. Using plain old __attribute__((weak)) does exactly the same. Considering that nv_weak is only used inside CUDA SDK headers, substituting weak in place of nv_weak will result in correct PTX, which is all we really need. I don't see much benefit turning it into full blown attribute just to mimic an internal CUDA implementation detail we don't care all that much about. Now, replacing it in CUDA headers for all CUDA versions we support may be tricky. Let me give it a try. I'll send a patch, if I manage to make it work. Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
tra added a comment. With the updated patch description + the discussion I'm OK with the approach from the general "how do we compile/use CUDA" point of view. I'll leave the question of whether the approach works for OpenMP to someone more familiar with it. While I'm not completely convinced that [fatbin]->.c->[clang]->.o (with device code only)->[ld -r] -> host.o (host+device code) is ideal (things could be done with smaller number of tool invocations), it should help to deal with -rdc compilation until we get a chance to improve support for it in Clang. We may revisit and change this portion of the pipeline when clang can incorporate -rdc GPU binaries in a way compatible with CUDA tools. Repository: rC Clang https://reviews.llvm.org/D47394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r334053 - Fix PR37694 - std::vector doesn't correctly move construct allocators.
Author: ericwf Date: Tue Jun 5 15:32:52 2018 New Revision: 334053 URL: http://llvm.org/viewvc/llvm-project?rev=334053=rev Log: Fix PR37694 - std::vector doesn't correctly move construct allocators. C++2a[container.requirements.general]p8 states that when move constructing a container, the allocator is move constructed. Vector previously copy constructed these allocators. This patch fixes that bug. Additionally it cleans up some unnecessary allocator conversions when copy constructing containers. Libc++ uses __internal_allocator_traits::select_on_copy_construction to select the correct allocator during copy construction, but it unnecessarily converted the resulting allocator to the user specified allocator type and back. After this patch list and forward_list no longer do that. Technically we're supposed to be using allocator_traits::select_on_copy_construction, but that should seemingly be addressed as a separate patch, if at all. Added: libcxx/trunk/test/std/containers/container.requirements/container.requirements.general/allocator_move.pass.cpp Modified: libcxx/trunk/include/forward_list libcxx/trunk/include/list libcxx/trunk/include/vector libcxx/trunk/test/std/containers/sequences/vector.bool/move.pass.cpp libcxx/trunk/test/std/containers/sequences/vector/vector.cons/move.pass.cpp libcxx/trunk/test/support/test_allocator.h Modified: libcxx/trunk/include/forward_list URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/forward_list?rev=334053=334052=334053=diff == --- libcxx/trunk/include/forward_list (original) +++ libcxx/trunk/include/forward_list Tue Jun 5 15:32:52 2018 @@ -457,6 +457,10 @@ protected: typedef typename allocator_traits<__begin_node_allocator>::pointer __begin_node_pointer; +static_assert((!is_same::value), + "internal allocator type must differ from user-specified " + "type; otherwise overload resolution breaks"); + __compressed_pair<__begin_node, __node_allocator> __before_begin_; _LIBCPP_INLINE_VISIBILITY @@ -481,9 +485,11 @@ protected: _NOEXCEPT_(is_nothrow_default_constructible<__node_allocator>::value) : __before_begin_(__begin_node()) {} _LIBCPP_INLINE_VISIBILITY -__forward_list_base(const allocator_type& __a) +explicit __forward_list_base(const allocator_type& __a) : __before_begin_(__begin_node(), __node_allocator(__a)) {} - +_LIBCPP_INLINE_VISIBILITY +explicit __forward_list_base(const __node_allocator& __a) +: __before_begin_(__begin_node(), __a) {} #ifndef _LIBCPP_CXX03_LANG public: _LIBCPP_INLINE_VISIBILITY @@ -954,12 +960,9 @@ forward_list<_Tp, _Alloc>::forward_list( template forward_list<_Tp, _Alloc>::forward_list(const forward_list& __x) -: base(allocator_type( - __node_traits::select_on_container_copy_construction(__x.__alloc()) - ) - ) -{ -insert_after(cbefore_begin(), __x.begin(), __x.end()); +: base( + __node_traits::select_on_container_copy_construction(__x.__alloc())) { + insert_after(cbefore_begin(), __x.begin(), __x.end()); } template Modified: libcxx/trunk/include/list URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/list?rev=334053=334052=334053=diff == --- libcxx/trunk/include/list (original) +++ libcxx/trunk/include/list Tue Jun 5 15:32:52 2018 @@ -556,6 +556,9 @@ protected: typedef typename __rebind_alloc_helper<__alloc_traits, __node_base>::type __node_base_allocator; typedef typename allocator_traits<__node_base_allocator>::pointer __node_base_pointer; +static_assert((!is_same::value), + "internal allocator type must differ from user-specified " + "type; otherwise overload resolution breaks"); __node_base __end_; __compressed_pair __size_alloc_; @@ -590,6 +593,11 @@ protected: _NOEXCEPT_(is_nothrow_default_constructible<__node_allocator>::value); _LIBCPP_INLINE_VISIBILITY __list_imp(const allocator_type& __a); +_LIBCPP_INLINE_VISIBILITY +__list_imp(const __node_allocator& __a); +#ifndef _LIBCPP_CXX03_LANG +__list_imp(__node_allocator&& __a) _NOEXCEPT; +#endif ~__list_imp(); void clear() _NOEXCEPT; _LIBCPP_INLINE_VISIBILITY @@ -713,9 +721,18 @@ __list_imp<_Tp, _Alloc>::__list_imp(cons } template -__list_imp<_Tp, _Alloc>::~__list_imp() -{ -clear(); +inline __list_imp<_Tp, _Alloc>::__list_imp(const __node_allocator& __a) +: __size_alloc_(0, __a) {} + +#ifndef _LIBCPP_CXX03_LANG +template +inline __list_imp<_Tp, _Alloc>::__list_imp(__node_allocator&& __a) _NOEXCEPT +: __size_alloc_(0, std::move(__a)) {} +#endif + +template +__list_imp<_Tp, _Alloc>::~__list_imp() { +
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
Meinersbur added a comment. In https://reviews.llvm.org/D47267#1123013, @dmgreen wrote: > I see your point about the mix of underscores. "nounroll_and_jam" also comes > from the Intel docs, and theres already "nounroll" vs "unroll". The "no" > becomes a qualifier on "unroll_and_jam". "no_unroll_and_jam" feels less > consistent to me. `nounroll_and_jam` looks like it should be parsed as "(no unroll) and jam" (do not unroll, but fuse) instead of "no (unroll-and-jam)" because `nounroll` is one word and as you mentioned, already used as a keyword somewhere else. Other variants use the underscore to append an option, e.g. `vectorize_width`. > But if you have a strong opinion, I'm happy enough to change it. I don't. Feel free to chose the name you think fits best. We might support multiple spellings if necessary. If we want to add more transformations, it would be nice to have an explicit naming scheme. E.g. for "register tiling", "stream_unroll" (supported by xlc), "index set splitting", "statement reordering", "strip mine", "overlap tiling", "diamond tiling", "thread-parallelization", "task-parallelization", "loop unswitching", etc. https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47802: Allow std::vector to move construct its allocator
fidget324 created this revision. fidget324 added reviewers: hiraditya, EricWF. Herald added a subscriber: cfe-commits. Fix an issue that was preventing std::vector from invoking the move constructor on its allocator when appropriate. Added a constructor to __vector_base which accepts an rvalue reference to the allocator, thus allowing the move constructor to be invoked. Previously, only a const lvalue reference was being accepted. This fixes bug 37694: https://bugs.llvm.org/show_bug.cgi?id=37694. Repository: rCXX libc++ https://reviews.llvm.org/D47802 Files: include/vector Index: include/vector === --- include/vector +++ include/vector @@ -355,6 +355,7 @@ __vector_base() _NOEXCEPT_(is_nothrow_default_constructible::value); _LIBCPP_INLINE_VISIBILITY __vector_base(const allocator_type& __a); +_LIBCPP_INLINE_VISIBILITY __vector_base(allocator_type&& __a); ~__vector_base(); _LIBCPP_INLINE_VISIBILITY @@ -438,6 +439,15 @@ { } +template +inline _LIBCPP_INLINE_VISIBILITY +__vector_base<_Tp, _Allocator>::__vector_base(allocator_type&& __a) +: __begin_(nullptr), + __end_(nullptr), + __end_cap_(nullptr, std::move(__a)) +{ +} + template __vector_base<_Tp, _Allocator>::~__vector_base() { Index: include/vector === --- include/vector +++ include/vector @@ -355,6 +355,7 @@ __vector_base() _NOEXCEPT_(is_nothrow_default_constructible::value); _LIBCPP_INLINE_VISIBILITY __vector_base(const allocator_type& __a); +_LIBCPP_INLINE_VISIBILITY __vector_base(allocator_type&& __a); ~__vector_base(); _LIBCPP_INLINE_VISIBILITY @@ -438,6 +439,15 @@ { } +template +inline _LIBCPP_INLINE_VISIBILITY +__vector_base<_Tp, _Allocator>::__vector_base(allocator_type&& __a) +: __begin_(nullptr), + __end_(nullptr), + __end_cap_(nullptr, std::move(__a)) +{ +} + template __vector_base<_Tp, _Allocator>::~__vector_base() { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size
joerg updated this revision to Diff 150049. joerg added a comment. After a careful review of newer GCC / libgcc and the assembler annotations from LLVM, I have come to the following conclusions: (1) The semantics have been somewhat changed by GCC in recent years. There is no actual specification, so we have to go by what behavior actually makes sense. (2) The primary motivation is still that the DW_CFA_GNU_args_size is a call-site specific annotation. It is expected to be applied when the IP is moved by the personality routine to compensate for the call site specific (temporary) adjustment. (3) It is not clear with plain unw_set_ip outside the scope of the Itanium EH handling should have this behavior, so it might need to be split into an internal routine. (4) LLVM does not produce correct CFA annotation for stdcall and similar cases where the callee removes additional stack space. The patch covers the first two items. https://reviews.llvm.org/D38680 Files: src/UnwindCursor.hpp src/libunwind.cpp Index: src/libunwind.cpp === --- src/libunwind.cpp +++ src/libunwind.cpp @@ -188,8 +188,13 @@ co->setReg(regNum, (pint_t)value); // specical case altering IP to re-find info (being called by personality // function) -if (regNum == UNW_REG_IP) +if (regNum == UNW_REG_IP) { + unw_proc_info_t info; + co->getInfo(); co->setInfoBasedOnIPRegister(false); + if (info.gp) +co->setReg(UNW_REG_SP, co->getReg(UNW_REG_SP) + info.gp); +} return UNW_ESUCCESS; } return UNW_EBADREG; Index: src/UnwindCursor.hpp === --- src/UnwindCursor.hpp +++ src/UnwindCursor.hpp @@ -1411,8 +1411,6 @@ this->setInfoBasedOnIPRegister(true); if (_unwindInfoMissing) return UNW_STEP_END; -if (_info.gp) - setReg(UNW_REG_SP, getReg(UNW_REG_SP) + _info.gp); } return result; Index: src/libunwind.cpp === --- src/libunwind.cpp +++ src/libunwind.cpp @@ -188,8 +188,13 @@ co->setReg(regNum, (pint_t)value); // specical case altering IP to re-find info (being called by personality // function) -if (regNum == UNW_REG_IP) +if (regNum == UNW_REG_IP) { + unw_proc_info_t info; + co->getInfo(); co->setInfoBasedOnIPRegister(false); + if (info.gp) +co->setReg(UNW_REG_SP, co->getReg(UNW_REG_SP) + info.gp); +} return UNW_ESUCCESS; } return UNW_EBADREG; Index: src/UnwindCursor.hpp === --- src/UnwindCursor.hpp +++ src/UnwindCursor.hpp @@ -1411,8 +1411,6 @@ this->setInfoBasedOnIPRegister(true); if (_unwindInfoMissing) return UNW_STEP_END; -if (_info.gp) - setReg(UNW_REG_SP, getReg(UNW_REG_SP) + _info.gp); } return result; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
dmgreen added a comment. I quite like the UnrollAndFuse naming. I'd not heard that the xlc compiler called it that. The UnrollAndJam pass was origin named that before I renamed for similar reasons (UnrollAndJam being more well known). I see your point about the mix of underscores. "nounroll_and_jam" also comes from the Intel docs, and theres already "nounroll" vs "unroll". The "no" becomes a qualifier on "unroll_and_jam". "no_unroll_and_jam" feels less consistent to me. But if you have a strong opinion, I'm happy enough to change it. https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47671: [analyzer] Implement copy elision.
xazax.hun added a comment. In https://reviews.llvm.org/D47671#1122994, @george.karpenkov wrote: > > So having an analyzer-config option would be useful for those codebases > > that are expected to be compiled with less advanced compilers that do not > > elide copies or not doing it aggressively enough. > > I'm not sure it makes sense. From my understanding, that's just how c++17 is > defined, and that's unrelated to what compiler implementation is used. Yeah, this is guaranteed for C++17. My only concern are the projects that need to support platforms where no comforming C++17 compiler is available (I think this might be common in the embedded world). I do not have any numbers though how frequent those projects are. That is the reason why I was wondering if it is worth asking. It is also possible that the authors of those project do not care about the analyzer. But since this config would most probably be removed at some point, I am fine with not adding it. Repository: rC Clang https://reviews.llvm.org/D47671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334051 - [X86] Make __builtin_ia32_vec_ext_v2si require ICE for its index argument. Add warnings for out of range indices for __builtin_ia32_vec_ext_v2si, __builtin_ia32_vec_ext_v4hi, and __builtin_i
Author: ctopper Date: Tue Jun 5 14:54:35 2018 New Revision: 334051 URL: http://llvm.org/viewvc/llvm-project?rev=334051=rev Log: [X86] Make __builtin_ia32_vec_ext_v2si require ICE for its index argument. Add warnings for out of range indices for __builtin_ia32_vec_ext_v2si, __builtin_ia32_vec_ext_v4hi, and __builtin_ia32_vec_set_v4hi. These should take a constant value for an index and that constant should be a valid element number. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/lib/Sema/SemaChecking.cpp Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=334051=334050=334051=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Jun 5 14:54:35 2018 @@ -142,7 +142,7 @@ TARGET_BUILTIN(__builtin_ia32_movntq, "v TARGET_BUILTIN(__builtin_ia32_vec_init_v2si, "V2iii", "nc", "mmx") TARGET_BUILTIN(__builtin_ia32_vec_init_v4hi, "V4s", "nc", "mmx") TARGET_BUILTIN(__builtin_ia32_vec_init_v8qi, "V8c", "nc", "mmx") -TARGET_BUILTIN(__builtin_ia32_vec_ext_v2si, "iV2ii", "nc", "mmx") +TARGET_BUILTIN(__builtin_ia32_vec_ext_v2si, "iV2iIi", "nc", "mmx") // MMX2 (MMX+SSE) intrinsics TARGET_BUILTIN(__builtin_ia32_cvtpi2ps, "V4fV4fV2i", "nc", "mmx,sse") Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=334051=334050=334051=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Jun 5 14:54:35 2018 @@ -2555,7 +2555,14 @@ bool Sema::CheckX86BuiltinFunctionCall(u case X86::BI_mm_prefetch: i = 1; l = 0; u = 7; break; + case X86::BI__builtin_ia32_vec_ext_v2si: +i = 1; l = 0; u = 1; +break; + case X86::BI__builtin_ia32_vec_ext_v4hi: +i = 1; l = 0; u = 3; +break; case X86::BI__builtin_ia32_sha1rnds4: + case X86::BI__builtin_ia32_vec_set_v4hi: i = 2; l = 0; u = 3; break; case X86::BI__builtin_ia32_vpermil2pd: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47671: [analyzer] Implement copy elision.
george.karpenkov added a comment. > So having an analyzer-config option would be useful for those codebases that > are expected to be compiled with less advanced compilers that do not elide > copies or not doing it aggressively enough. I'm not sure it makes sense. From my understanding, that's just how c++17 is defined, and that's unrelated to what compiler implementation is used. > I am not sure though how often the end users end up fine tuning the analyzer. My guess will be "never" (apart from turning on checkers) Repository: rC Clang https://reviews.llvm.org/D47671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47733: [CUDA][HIP] Set kernel calling convention before arrange function
tra added a comment. Looks OK overall, but I'm not very familiar with CGCall, so you may want to dig through the history and find someone with more expertise. Comment at: test/CodeGenCUDA/kernel-args-amdgcn.cu:1 +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -emit-llvm %s -o - | FileCheck %s +#include "Inputs/cuda.h" Please add some checks that verify that CUDA kernels still don't have any extra calling conventions set. https://reviews.llvm.org/D47733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47291: Proposal to make rtti errors more generic.
Sunil_Srivastava added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; wristow wrote: > probinson wrote: > > wristow wrote: > > > Sunil_Srivastava wrote: > > > > probinson wrote: > > > > > filcab wrote: > > > > > > I'd prefer to have the way to enable RTTI mentioned in the message. > > > > > > Could we have something like `ToolChain::getRTTIMode()` and have a > > > > > > "RTTI was on/of" or "RTTI defaulted to on/off"? That way we'd be > > > > > > able to have a message similar to the current one (mentioning "you > > > > > > passed -fno-rtti") on platforms that default to RTTI=on, and have > > > > > > your updated message (possibly with a mention of "use -frtti") on > > > > > > platforms that default to RTTI=off. > > > > > > > > > > > > (This is a minor usability comment about this patch, I don't > > > > > > consider it a blocker or anything) > > > > > If the options are spelled differently for clang-cl and we had a way > > > > > to retrieve the appropriate spellings, providing the option to use in > > > > > the diagnostic does seem like a nice touch. > > > > The idea of suggestion as to how-to-turn-on-rtti is appealing, but it > > > > is not trivial. > > > > > > > > First, clang-cl does not give this warning at all, so the issue is moot > > > > for clang-cl. > > > > > > > > For unix-line command-line, if the default RTTI mode in ENABLED (the > > > > unknown-linux) > > > > then this warning appears only if the user gives -fno-rtti options, so > > > > again we do > > > > not need to say anything more. > > > > > > > > The only cases left are compilers a where the default RTTI mode is > > > > DISABLED. > > > > Here an addendum like '[-frtti]' may make sense. AFAIK, PS4 is the only > > > > compiler > > > > with this default, but there may be other such private compilers. > > > > > > > > So should we append '[-frtti]' if Target.getTriple().isPS4() is true? > > > > So should we append '[-frtti]' if Target.getTriple().isPS4() is true? > > > > > > Personally, I'd be OK with producing a suggestion of how to enable RTTI > > > based on the PS4 triple. But I'd also be OK with not enhancing this > > > diagnostic to suggest how to turn on RTTI (that is, going with the patch > > > as originally proposed here). > > > > > > If clang-cl produced a warning when a dynamic_cast or typeid construct > > > was encountered in `/GR-` mode, then I'd feel it's worth complicating the > > > code to provide a target-sensitive way for advising the user how to turn > > > RTTI on. But clang-cl doesn't produce a warning in that case, so the > > > effort to add the framework for producing a target-sensitive warning > > > doesn't seem worth it to me. > > > > > > Improving clang-cl to produce a diagnostic in this `/GR-` situation seems > > > like a good idea, but separate from this proposed patch. If that work > > > gets done at some point, then it would be natural to revisit this > > > diagnostic at that time. > > If clang-cl is not a consideration, then I think the easiest and clearest > > way to do this is simply to say `requires -frtti` without hair-splitting > > which targets default which way. > Saying `requires -frtti` makes good sense to me. Done https://reviews.llvm.org/D47291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47291: Proposal to make rtti errors more generic.
Sunil_Srivastava updated this revision to Diff 150038. Sunil_Srivastava added a comment. Changed as per Paul's suggestion. https://reviews.llvm.org/D47291 Files: include/clang/Basic/DiagnosticSemaKinds.td test/SemaCXX/no-rtti.cpp Index: test/SemaCXX/no-rtti.cpp === --- test/SemaCXX/no-rtti.cpp +++ test/SemaCXX/no-rtti.cpp @@ -6,7 +6,7 @@ void f() { - (void)typeid(int); // expected-error {{cannot use typeid with -fno-rtti}} + (void)typeid(int); // expected-error {{use of typeid requires -frtti}} } namespace { @@ -20,7 +20,7 @@ } bool isa_B(A *a) { - return dynamic_cast(a) != 0; // expected-error {{cannot use dynamic_cast with -fno-rtti}} + return dynamic_cast(a) != 0; // expected-error {{use of dynamic_cast requires -frtti}} } void* getMostDerived(A* a) { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -6607,9 +6607,9 @@ "no %select{struct|interface|union|class|enum}0 named %1 in %2">; def err_no_typeid_with_fno_rtti : Error< - "cannot use typeid with -fno-rtti">; + "use of typeid requires -frtti">; def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires -frtti">; def err_cannot_form_pointer_to_member_of_reference_type : Error< "cannot form a pointer-to-member to member %0 of reference type %1">; Index: test/SemaCXX/no-rtti.cpp === --- test/SemaCXX/no-rtti.cpp +++ test/SemaCXX/no-rtti.cpp @@ -6,7 +6,7 @@ void f() { - (void)typeid(int); // expected-error {{cannot use typeid with -fno-rtti}} + (void)typeid(int); // expected-error {{use of typeid requires -frtti}} } namespace { @@ -20,7 +20,7 @@ } bool isa_B(A *a) { - return dynamic_cast(a) != 0; // expected-error {{cannot use dynamic_cast with -fno-rtti}} + return dynamic_cast(a) != 0; // expected-error {{use of dynamic_cast requires -frtti}} } void* getMostDerived(A* a) { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -6607,9 +6607,9 @@ "no %select{struct|interface|union|class|enum}0 named %1 in %2">; def err_no_typeid_with_fno_rtti : Error< - "cannot use typeid with -fno-rtti">; + "use of typeid requires -frtti">; def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires -frtti">; def err_cannot_form_pointer_to_member_of_reference_type : Error< "cannot form a pointer-to-member to member %0 of reference type %1">; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47720: [DebugInfo] Inline for without DebugLocation
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. As is the case with https://reviews.llvm.org/D47097, this helps preserve more scope information after SROA. LGTM. Thanks! Repository: rC Clang https://reviews.llvm.org/D47720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47720: [DebugInfo] Inline for without DebugLocation
gramanas added inline comments. Comment at: test/CodeGen/debug-info-inline-for.c:13 + +// CHECK: ![[DbgLoc]] = !DILocation( aprantl wrote: > Shouldn't you also check *which* location is attached to it? I wasn't sure if this should be an artificial location or not so I left it like that for the time being. It's been updated to reflect the latest changes now. Repository: rC Clang https://reviews.llvm.org/D47720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47720: [DebugInfo] Inline for without DebugLocation
gramanas updated this revision to Diff 150036. gramanas marked an inline comment as done. gramanas added a comment. Add artificial debug location to the phi instruction Repository: rC Clang https://reviews.llvm.org/D47720 Files: lib/CodeGen/CGExprScalar.cpp test/CodeGen/debug-info-inline-for.c Index: test/CodeGen/debug-info-inline-for.c === --- /dev/null +++ test/CodeGen/debug-info-inline-for.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s +// Check that clang emits Debug location in the phi instruction + +int func(int n) { + int a; + for(a = 10; a>0 && n++; a--); + return n; +} + +// CHECK: land.end: +// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]] + +// CHECK: ![[DbgLoc]] = !DILocation(line: 0 Index: lib/CodeGen/CGExprScalar.cpp === --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -3427,6 +3427,11 @@ } // Insert an entry into the phi node for the edge with the value of RHSCond. PN->addIncoming(RHSCond, RHSBlock); + { +// Artificial location to preserve the scope information +auto NL = ApplyDebugLocation::CreateArtificial(CGF); +PN->setDebugLoc(Builder.getCurrentDebugLocation()); + } // ZExt result to int. return Builder.CreateZExtOrBitCast(PN, ResTy, "land.ext"); Index: test/CodeGen/debug-info-inline-for.c === --- /dev/null +++ test/CodeGen/debug-info-inline-for.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s +// Check that clang emits Debug location in the phi instruction + +int func(int n) { + int a; + for(a = 10; a>0 && n++; a--); + return n; +} + +// CHECK: land.end: +// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]] + +// CHECK: ![[DbgLoc]] = !DILocation(line: 0 Index: lib/CodeGen/CGExprScalar.cpp === --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -3427,6 +3427,11 @@ } // Insert an entry into the phi node for the edge with the value of RHSCond. PN->addIncoming(RHSCond, RHSBlock); + { +// Artificial location to preserve the scope information +auto NL = ApplyDebugLocation::CreateArtificial(CGF); +PN->setDebugLoc(Builder.getCurrentDebugLocation()); + } // ZExt result to int. return Builder.CreateZExtOrBitCast(PN, ResTy, "land.ext"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.
yunlian updated this revision to Diff 150028. https://reviews.llvm.org/D44788 Files: include/clang/Driver/Options.td lib/Driver/ToolChains/CommonArgs.cpp test/Driver/lto-dwo.c Index: test/Driver/lto-dwo.c === --- /dev/null +++ test/Driver/lto-dwo.c @@ -0,0 +1,12 @@ +// Confirm that -gsplit-dwarf=DIR is passed to linker + +// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -gsplit-dwarf=DIR 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-DWO-DIR < %t %s +// +// CHECK-LINK-DWO-DIR: "-plugin-opt=dwo_dir=DIR" +// + +// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -gsplit-dwarf 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-DWO-DIR-DEFAULT < %t %s +// +// CHECK-LINK-DWO-DIR-DEFAULT: "-plugin-opt=dwo_dir=." Index: lib/Driver/ToolChains/CommonArgs.cpp === --- lib/Driver/ToolChains/CommonArgs.cpp +++ lib/Driver/ToolChains/CommonArgs.cpp @@ -417,6 +417,19 @@ if (IsThinLTO) CmdArgs.push_back("-plugin-opt=thinlto"); + if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ)) { +StringRef DWO_Dir = A->getValue(); +CmdArgs.push_back( +Args.MakeArgString(Twine("-plugin-opt=dwo_dir=") + DWO_Dir)); + } + + if (Args.hasArg(options::OPT_gsplit_dwarf)) { +if (!Args.getLastArg(options::OPT_gsplit_dwarf_EQ)) { + CmdArgs.push_back( + Args.MakeArgString(Twine("-plugin-opt=dwo_dir=."))); +} + } + if (unsigned Parallelism = getLTOParallelism(Args, ToolChain.getDriver())) CmdArgs.push_back( Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism))); Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1762,6 +1762,7 @@ def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, Flags<[CoreOption]>; def gno_column_info : Flag<["-"], "gno-column-info">, Group, Flags<[CoreOption]>; def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group; +def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group; def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group, Flags<[CC1Option]>; def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group; def gmodules : Flag <["-"], "gmodules">, Group, Index: test/Driver/lto-dwo.c === --- /dev/null +++ test/Driver/lto-dwo.c @@ -0,0 +1,12 @@ +// Confirm that -gsplit-dwarf=DIR is passed to linker + +// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -gsplit-dwarf=DIR 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-DWO-DIR < %t %s +// +// CHECK-LINK-DWO-DIR: "-plugin-opt=dwo_dir=DIR" +// + +// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -gsplit-dwarf 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-DWO-DIR-DEFAULT < %t %s +// +// CHECK-LINK-DWO-DIR-DEFAULT: "-plugin-opt=dwo_dir=." Index: lib/Driver/ToolChains/CommonArgs.cpp === --- lib/Driver/ToolChains/CommonArgs.cpp +++ lib/Driver/ToolChains/CommonArgs.cpp @@ -417,6 +417,19 @@ if (IsThinLTO) CmdArgs.push_back("-plugin-opt=thinlto"); + if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ)) { +StringRef DWO_Dir = A->getValue(); +CmdArgs.push_back( +Args.MakeArgString(Twine("-plugin-opt=dwo_dir=") + DWO_Dir)); + } + + if (Args.hasArg(options::OPT_gsplit_dwarf)) { +if (!Args.getLastArg(options::OPT_gsplit_dwarf_EQ)) { + CmdArgs.push_back( + Args.MakeArgString(Twine("-plugin-opt=dwo_dir=."))); +} + } + if (unsigned Parallelism = getLTOParallelism(Args, ToolChain.getDriver())) CmdArgs.push_back( Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism))); Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1762,6 +1762,7 @@ def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, Flags<[CoreOption]>; def gno_column_info : Flag<["-"], "gno-column-info">, Group, Flags<[CoreOption]>; def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group; +def gsplit_dwarf_EQ : Joined<["-"], "gsplit-dwarf=">, Group; def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group, Flags<[CC1Option]>; def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group; def gmodules : Flag <["-"], "gmodules">, Group, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47724: [X86] Add back _mask, _maskz, and _mask3 builtins for some 512-bit fmadd/fmsub/fmaddsub/fmsubadd builtins.
craig.topper updated this revision to Diff 150027. craig.topper added a comment. Add subtract builtins to SemaChecking.cpp Repository: rC Clang https://reviews.llvm.org/D47724 Files: include/clang/Basic/BuiltinsX86.def lib/CodeGen/CGBuiltin.cpp lib/Headers/avx512fintrin.h lib/Sema/SemaChecking.cpp test/CodeGen/avx512f-builtins.c Index: test/CodeGen/avx512f-builtins.c === --- test/CodeGen/avx512f-builtins.c +++ test/CodeGen/avx512f-builtins.c @@ -461,7 +461,7 @@ // CHECK-LABEL: @test_mm512_maskz_fmadd_round_pd // CHECK: @llvm.x86.avx512.vfmadd.pd.512 // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fmadd_round_pd(__U, __A, __B, __C, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC); } __m512d test_mm512_fmsub_round_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -483,7 +483,7 @@ // CHECK: fsub <8 x double> // CHECK: @llvm.x86.avx512.vfmadd.pd.512 // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fmsub_round_pd(__U, __A, __B, __C, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC); } __m512d test_mm512_fnmadd_round_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -505,7 +505,7 @@ // CHECK: fsub <8 x double> // CHECK: @llvm.x86.avx512.vfmadd.pd.512 // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fnmadd_round_pd(__U, __A, __B, __C, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC); } __m512d test_mm512_fnmsub_round_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -521,7 +521,7 @@ // CHECK: fsub <8 x double> // CHECK: @llvm.x86.avx512.vfmadd.pd.512 // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fnmsub_round_pd(__U, __A, __B, __C, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC); } __m512d test_mm512_fmadd_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -547,7 +547,7 @@ // CHECK-LABEL: @test_mm512_maskz_fmadd_pd // CHECK: call <8 x double> @llvm.fma.v8f64(<8 x double> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}) // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fmadd_pd(__U, __A, __B, __C); } __m512d test_mm512_fmsub_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -569,7 +569,7 @@ // CHECK: fsub <8 x double> , %{{.*}} // CHECK: call <8 x double> @llvm.fma.v8f64(<8 x double> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}) // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fmsub_pd(__U, __A, __B, __C); } __m512d test_mm512_fnmadd_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -591,7 +591,7 @@ // CHECK: fsub <8 x double> , %{{.*}} // CHECK: call <8 x double> @llvm.fma.v8f64(<8 x double> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}) // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fnmadd_pd(__U, __A, __B, __C); } __m512d test_mm512_fnmsub_pd(__m512d __A, __m512d __B, __m512d __C) { @@ -607,7 +607,7 @@ // CHECK: fsub <8 x double> , %{{.*}} // CHECK: call <8 x double> @llvm.fma.v8f64(<8 x double> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}) // CHECK: bitcast i8 %{{.*}} to <8 x i1> - // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}} + // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> zeroinitializer return _mm512_maskz_fnmsub_pd(__U, __A, __B, __C); } __m512 test_mm512_fmadd_round_ps(__m512 __A, __m512 __B, __m512 __C) { @@ -633,7 +633,7 @@ // CHECK-LABEL: @test_mm512_maskz_fmadd_round_ps // CHECK: @llvm.x86.avx512.vfmadd.ps.512 // CHECK: bitcast i16 %{{.*}} to <16 x i1> - // CHECK: select <16 x i1> %{{.*}}, <16 x float> %{{.*}}, <16 x float> %{{.*}} + // CHECK: select <16 x i1> %{{.*}}, <16 x float> %{{.*}}, <16 x float> zeroinitializer return _mm512_maskz_fmadd_round_ps(__U, __A, __B, __C,
[PATCH] D47758: [Fuchsia] Include install-distribution-stripped in bootstrap targets
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D47758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.
shuaiwang added a comment. Thanks a lot for the review! Could you also help commit this diff as well? Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.
shuaiwang updated this revision to Diff 150026. shuaiwang marked an inline comment as done. shuaiwang added a comment. Remove stale comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 Files: clang-tidy/utils/CMakeLists.txt clang-tidy/utils/ExprMutationAnalyzer.cpp clang-tidy/utils/ExprMutationAnalyzer.h unittests/clang-tidy/CMakeLists.txt unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp === --- /dev/null +++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp @@ -0,0 +1,608 @@ +//===-- ExprMutationAnalyzerTest.cpp - clang-tidy -===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "../clang-tidy/utils/ExprMutationAnalyzer.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace tidy { +namespace test { + +using namespace clang::ast_matchers; +using ::testing::ElementsAre; +using ::testing::IsEmpty; +using ::testing::ResultOf; +using ::testing::StartsWith; +using ::testing::Values; + +namespace { + +using ExprMatcher = internal::Matcher; +using StmtMatcher = internal::Matcher; + +ExprMatcher declRefTo(StringRef Name) { + return declRefExpr(to(namedDecl(hasName(Name; +} + +StmtMatcher withEnclosingCompound(ExprMatcher Matcher) { + return expr(Matcher, hasAncestor(compoundStmt().bind("stmt"))).bind("expr"); +} + +bool isMutated(const SmallVectorImpl , ASTUnit *AST) { + const auto *const S = selectFirst("stmt", Results); + const auto *const E = selectFirst("expr", Results); + return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E); +} + +SmallVector +mutatedBy(const SmallVectorImpl , ASTUnit *AST) { + const auto *const S = selectFirst("stmt", Results); + SmallVector Chain; + utils::ExprMutationAnalyzer Analyzer(S, >getASTContext()); + for (const auto *E = selectFirst("expr", Results); E != nullptr;) { +const Stmt *By = Analyzer.findMutation(E); +std::string buffer; +llvm::raw_string_ostream stream(buffer); +By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy()); +Chain.push_back(StringRef(stream.str()).trim().str()); +E = dyn_cast(By); + } + return Chain; +} + +std::string removeSpace(std::string s) { + s.erase(std::remove_if(s.begin(), s.end(), + [](char c) { return std::isspace(c); }), + s.end()); + return s; +} + +} // namespace + +TEST(ExprMutationAnalyzerTest, Trivial) { + const auto AST = tooling::buildASTFromCode("void f() { int x; x; }"); + const auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); +} + +class AssignmentTest : public ::testing::TestWithParam {}; + +TEST_P(AssignmentTest, AssignmentModifies) { + const std::string ModExpr = "x " + GetParam() + " 10"; + const auto AST = + tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }"); + const auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr)); +} + +INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest, +Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=", + "^=", "<<=", ">>="), ); + +class IncDecTest : public ::testing::TestWithParam {}; + +TEST_P(IncDecTest, IncDecModifies) { + const std::string ModExpr = GetParam(); + const auto AST = + tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }"); + const auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr)); +} + +INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest, +Values("++x", "--x", "x++", "x--"), ); + +TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) { + const auto AST = tooling::buildASTFromCode( + "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }"); + const auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()")); +} + +TEST(ExprMutationAnalyzerTest, ConstMemberFunc) { + const auto AST = tooling::buildASTFromCode( + "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }"); + const auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); +} + +TEST(ExprMutationAnalyzerTest, NonConstOperator) { +
[PATCH] D43667: [clang-doc] Implement a YAML generator
juliehockett added inline comments. Comment at: clang-doc/YAMLGenerator.cpp:265 +// and thus register the generator. +volatile int YAMLGeneratorAnchorSource = 0; + ioeric wrote: > nit: add `static`? `static` declares it as file-local, and so the `extern` reference to this in Generators.cpp wouldn't be allowed. Comment at: clang-doc/generators/YAMLGenerator.cpp:223 + +template <> struct MappingTraits> { + static void mapping(IO , std::unique_ptr ) { ioeric wrote: > juliehockett wrote: > > ioeric wrote: > > > YAML mapping for `unique_ptr` is a bit unusual. I wonder whether this > > > would work all the time e.g. if the unique_ptr has not been allocated. > > Mmm yes it's a little weird -- that said, is there a better way emit the > > info given a pointer? > Not sure what the initial intention was for using `unique_ptr` here, but one > option is to get rid of the `unique_ptr` and just store `CommentInfo` in the > structure. Another (less ideal) option is to check whether `I` is nullptr and > allocate when it's null (only when deserialization though; I think you could > tell this from `IO`); and you'd probably want to avoid the allocation when > the comment info is actually not present in `IO` (IIRC, mapping would be > called even when the info is not present). Sorry this slipped through before -- it's a pointer because it has to be able to store itself (the other infos just store the `CommentInfo` directly, but each `CommentInfo` can have comment children). The pointer only appears there. Comment at: clang-doc/tool/ClangDocMain.cpp:221 +} +std::error_code FileErr; +llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr, llvm::sys::fs::F_None); ioeric wrote: > nit: I wonder if you could merge creation of `InfoOS` into `getPath` so that > the helper function would just return `llvm::Expected` You can't, because when you return `llvm::make_error` it invokes a copy constructor, which is deleted in `raw_ostream`s. https://reviews.llvm.org/D43667 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
GBuella marked an inline comment as done. GBuella added inline comments. Comment at: test/CodeGen/target-features-error-2.c:39 __m128d need_avx(__m128d a, __m128d b) { return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs target feature avx}} } craig.topper wrote: > GBuella wrote: > > craig.topper wrote: > > > The 4 compare functions here are basically the same for the purpose of > > > this check. What value do we get by testing all 4 of them? > > Which four more specifically? > _mm_cmp_ps, _mm_cmp_ss, _mm_cmp_pd, _mm_cmp_sd. Or the cases for > NEED_AVX_2-5. They're all basically checking the same thing. There is nothing > different about the command lines other than the -D NEED_AVX Yes, I guess we could just remove NEED_AVX_3,4,5 https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
GBuella updated this revision to Diff 150018. https://reviews.llvm.org/D46541 Files: lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h test/CodeGen/target-features-error-2.c test/CodeGen/target-features-error.c Index: test/CodeGen/target-features-error.c === --- test/CodeGen/target-features-error.c +++ test/CodeGen/target-features-error.c @@ -3,6 +3,5 @@ return a + 4; } int bar() { - return foo(4); // expected-error {{always_inline function 'foo' requires target feature 'sse4.2', but would be inlined into function 'bar' that is compiled without support for 'sse4.2'}} + return foo(4); // expected-error {{always_inline function 'foo' requires target feature 'avx', but would be inlined into function 'bar' that is compiled without support for 'avx'}} } - Index: test/CodeGen/target-features-error-2.c === --- test/CodeGen/target-features-error-2.c +++ test/CodeGen/target-features-error-2.c @@ -1,38 +1,46 @@ -// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_SSE42 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_1 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_2 -// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_3 -// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_4 +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX512f +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +movdir64b -S -verify -o - -D NEED_MOVDIRI +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx512vnni -target-feature +movdiri -S -verify -o - -D NEED_CLWB #define __MM_MALLOC_H #include -#if NEED_SSE42 +#if NEED_AVX_1 int baz(__m256i a) { - return _mm256_extract_epi32(a, 3); // expected-error {{always_inline function '_mm256_extract_epi32' requires target feature 'sse4.2', but would be inlined into function 'baz' that is compiled without support for 'sse4.2'}} + return _mm256_extract_epi32(a, 3); // expected-error {{always_inline function '_mm256_extract_epi32' requires target feature 'avx', but would be inlined into function 'baz' that is compiled without support for 'avx'}} } #endif -#if NEED_AVX_1 +#if NEED_AVX_2 __m128 need_avx(__m128 a, __m128 b) { return _mm_cmp_ps(a, b, 0); // expected-error {{'__builtin_ia32_cmpps' needs target feature avx}} } #endif -#if NEED_AVX_2 -__m128 need_avx(__m128 a, __m128 b) { - return _mm_cmp_ss(a, b, 0); // expected-error {{'__builtin_ia32_cmpss' needs target feature avx}} +#if NEED_AVX512f +unsigned short need_avx512f(unsigned short a, unsigned short b) { + return __builtin_ia32_korhi(a, b); // expected-error {{'__builtin_ia32_korhi' needs target feature avx512f}} } #endif -#if NEED_AVX_3 -__m128d need_avx(__m128d a, __m128d b) { - return _mm_cmp_pd(a, b, 0); // expected-error {{'__builtin_ia32_cmppd' needs target feature avx}} +#if NEED_MOVDIRI +void need_movdiri(unsigned int *a, unsigned int b) { + __builtin_ia32_directstore_u32(a, b); // expected-error {{'__builtin_ia32_directstore_u32' needs target feature movdiri}} } #endif -#if NEED_AVX_4 -__m128d need_avx(__m128d a, __m128d b) { - return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs target feature avx}} +#if NEED_CLWB +static __inline__ void + __attribute__((__always_inline__, __nodebug__, __target__("avx512vnni,clwb,movdiri,movdir64b"))) + func(unsigned int *a, unsigned int b) +{ + __builtin_ia32_directstore_u32(a, b); +} + +void need_clwb(unsigned int *a, unsigned int b) { + func(a, b); // expected-error {{always_inline function 'func' requires target feature 'clwb', but would be inlined into function 'need_clwb' that is compiled without support for 'clwb'}} + } #endif Index: lib/CodeGen/CodeGenModule.h === --- lib/CodeGen/CodeGenModule.h +++ lib/CodeGen/CodeGenModule.h @@ -1089,6 +1089,10 @@ /// It's up to you to ensure that this is safe. void AddDefaultFnAttrs(llvm::Function ); + /// Parses the target attributes passed in, and returns only the ones that are + /// valid feature names. + TargetAttr::ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD); + // Fills in the supplied string map with the set of target features for the // passed in function. void getFunctionFeatureMap(llvm::StringMap , Index: lib/CodeGen/CodeGenModule.cpp === --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -5033,22 +5033,28 @@ } } +TargetAttr::ParsedTargetAttr CodeGenModule::filterFunctionTargetAttrs(const TargetAttr *TD) { + assert(TD != nullptr); + TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse(); + + ParsedAttr.Features.erase( +
[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.
pcc added inline comments. Comment at: lib/Driver/ToolChains/CommonArgs.cpp:435 + CmdArgs.push_back( + Args.MakeArgString(Twine("-plugin-opt=objcopy=") + Objcopy)); + CmdArgs.push_back( You no longer need to pass `objcopy=`, see D47091. https://reviews.llvm.org/D44788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47291: Proposal to make rtti errors more generic.
wristow added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; probinson wrote: > wristow wrote: > > Sunil_Srivastava wrote: > > > probinson wrote: > > > > filcab wrote: > > > > > I'd prefer to have the way to enable RTTI mentioned in the message. > > > > > Could we have something like `ToolChain::getRTTIMode()` and have a > > > > > "RTTI was on/of" or "RTTI defaulted to on/off"? That way we'd be able > > > > > to have a message similar to the current one (mentioning "you passed > > > > > -fno-rtti") on platforms that default to RTTI=on, and have your > > > > > updated message (possibly with a mention of "use -frtti") on > > > > > platforms that default to RTTI=off. > > > > > > > > > > (This is a minor usability comment about this patch, I don't consider > > > > > it a blocker or anything) > > > > If the options are spelled differently for clang-cl and we had a way to > > > > retrieve the appropriate spellings, providing the option to use in the > > > > diagnostic does seem like a nice touch. > > > The idea of suggestion as to how-to-turn-on-rtti is appealing, but it is > > > not trivial. > > > > > > First, clang-cl does not give this warning at all, so the issue is moot > > > for clang-cl. > > > > > > For unix-line command-line, if the default RTTI mode in ENABLED (the > > > unknown-linux) > > > then this warning appears only if the user gives -fno-rtti options, so > > > again we do > > > not need to say anything more. > > > > > > The only cases left are compilers a where the default RTTI mode is > > > DISABLED. > > > Here an addendum like '[-frtti]' may make sense. AFAIK, PS4 is the only > > > compiler > > > with this default, but there may be other such private compilers. > > > > > > So should we append '[-frtti]' if Target.getTriple().isPS4() is true? > > > So should we append '[-frtti]' if Target.getTriple().isPS4() is true? > > > > Personally, I'd be OK with producing a suggestion of how to enable RTTI > > based on the PS4 triple. But I'd also be OK with not enhancing this > > diagnostic to suggest how to turn on RTTI (that is, going with the patch as > > originally proposed here). > > > > If clang-cl produced a warning when a dynamic_cast or typeid construct was > > encountered in `/GR-` mode, then I'd feel it's worth complicating the code > > to provide a target-sensitive way for advising the user how to turn RTTI > > on. But clang-cl doesn't produce a warning in that case, so the effort to > > add the framework for producing a target-sensitive warning doesn't seem > > worth it to me. > > > > Improving clang-cl to produce a diagnostic in this `/GR-` situation seems > > like a good idea, but separate from this proposed patch. If that work gets > > done at some point, then it would be natural to revisit this diagnostic at > > that time. > If clang-cl is not a consideration, then I think the easiest and clearest way > to do this is simply to say `requires -frtti` without hair-splitting which > targets default which way. Saying `requires -frtti` makes good sense to me. Repository: rC Clang https://reviews.llvm.org/D47291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47784: [MS][ARM64]: Promote _setjmp to_setjmpex as there is no _setjmp in the ARM64 libvcruntime.lib
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I'm happy to refactor this after landing it as is, or you can do it if you like. Comment at: lib/CodeGen/CGBuiltin.cpp:2808 case Builtin::BI_setjmpex: { if (getTarget().getTriple().isOSMSVCRT()) { llvm::Type *ArgTypes[] = {Int8PtrTy, Int8PtrTy}; We should probably refactor all this to use shared code. Repository: rC Clang https://reviews.llvm.org/D47784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r334033 - [clangd] Fix inverted test again, sigh
Author: sammccall Date: Tue Jun 5 11:00:48 2018 New Revision: 334033 URL: http://llvm.org/viewvc/llvm-project?rev=334033=rev Log: [clangd] Fix inverted test again, sigh Modified: clang-tools-extra/trunk/clangd/Quality.cpp Modified: clang-tools-extra/trunk/clangd/Quality.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=334033=334032=334033=diff == --- clang-tools-extra/trunk/clangd/Quality.cpp (original) +++ clang-tools-extra/trunk/clangd/Quality.cpp Tue Jun 5 11:00:48 2018 @@ -69,7 +69,7 @@ raw_ostream <<(raw_ostream , static SymbolRelevanceSignals::AccessibleScope ComputeScope(const NamedDecl ) { - bool InClass = true; + bool InClass = false; for (const DeclContext *DC = D.getDeclContext(); !DC->isFileContext(); DC = DC->getParent()) { if (DC->isFunctionOrMethod()) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r334032 - [clangd] Quality fixes (uninit var, missing debug output, pattern decl CCRs).
Author: sammccall Date: Tue Jun 5 10:58:12 2018 New Revision: 334032 URL: http://llvm.org/viewvc/llvm-project?rev=334032=rev Log: [clangd] Quality fixes (uninit var, missing debug output, pattern decl CCRs). Modified: clang-tools-extra/trunk/clangd/Quality.cpp Modified: clang-tools-extra/trunk/clangd/Quality.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=334032=334031=334032=diff == --- clang-tools-extra/trunk/clangd/Quality.cpp (original) +++ clang-tools-extra/trunk/clangd/Quality.cpp Tue Jun 5 10:58:12 2018 @@ -69,7 +69,7 @@ raw_ostream <<(raw_ostream , static SymbolRelevanceSignals::AccessibleScope ComputeScope(const NamedDecl ) { - bool InClass; + bool InClass = true; for (const DeclContext *DC = D.getDeclContext(); !DC->isFileContext(); DC = DC->getParent()) { if (DC->isFunctionOrMethod()) @@ -103,7 +103,7 @@ void SymbolRelevanceSignals::merge(const } // Declarations are scoped, others (like macros) are assumed global. - if (SemaCCResult.Kind == CodeCompletionResult::RK_Declaration) + if (SemaCCResult.Declaration) Scope = std::min(Scope, ComputeScope(*SemaCCResult.Declaration)); } @@ -142,6 +142,9 @@ raw_ostream <<(raw_ostream , OS << formatv("=== Symbol relevance: {0}\n", S.evaluate()); OS << formatv("\tName match: {0}\n", S.NameMatch); OS << formatv("\tForbidden: {0}\n", S.Forbidden); + OS << formatv("\tProximity: {0}\n", S.ProximityScore); + OS << formatv("\tQuery type: {0}\n", static_cast(S.Query)); + OS << formatv("\tScope: {0}\n", static_cast(S.Scope)); return OS; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
Meinersbur added a comment. In https://reviews.llvm.org/D47267#1122425, @dmgreen wrote: > I noticed in the paper that you used the name "unrollandjam", minus > underscores. Should I change this use that spelling here? I have no strong > opinion of one over the other (was just using what I had found from the Intel > docs). IMHO you can keep `unroll_and_jam` (which is already supported by Intel syntax). When I imagined the name, I had xlc's `unrollandfuse` in mind, but found that "and jam" is better known than "and fuse". We can have a discussion about how to name them in general. `nounroll_and_jam` seems a strange mix of write words together and separate words by underscores. https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.
yunlian marked an inline comment as done. yunlian added a comment. Herald added a subscriber: steven_wu. ping? https://reviews.llvm.org/D44788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47600: [UBSan] DO NOT COMMIT: precise UBSan checks experiment
alekseyshl abandoned this revision. alekseyshl added a comment. Experimental. Repository: rC Clang https://reviews.llvm.org/D47600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47720: [DebugInfo] Inline for without DebugLocation
aprantl added inline comments. Comment at: test/CodeGen/debug-info-inline-for.c:13 + +// CHECK: ![[DbgLoc]] = !DILocation( Shouldn't you also check *which* location is attached to it? Repository: rC Clang https://reviews.llvm.org/D47720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47291: Proposal to make rtti errors more generic.
probinson added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; wristow wrote: > Sunil_Srivastava wrote: > > probinson wrote: > > > filcab wrote: > > > > I'd prefer to have the way to enable RTTI mentioned in the message. > > > > Could we have something like `ToolChain::getRTTIMode()` and have a > > > > "RTTI was on/of" or "RTTI defaulted to on/off"? That way we'd be able > > > > to have a message similar to the current one (mentioning "you passed > > > > -fno-rtti") on platforms that default to RTTI=on, and have your updated > > > > message (possibly with a mention of "use -frtti") on platforms that > > > > default to RTTI=off. > > > > > > > > (This is a minor usability comment about this patch, I don't consider > > > > it a blocker or anything) > > > If the options are spelled differently for clang-cl and we had a way to > > > retrieve the appropriate spellings, providing the option to use in the > > > diagnostic does seem like a nice touch. > > The idea of suggestion as to how-to-turn-on-rtti is appealing, but it is > > not trivial. > > > > First, clang-cl does not give this warning at all, so the issue is moot for > > clang-cl. > > > > For unix-line command-line, if the default RTTI mode in ENABLED (the > > unknown-linux) > > then this warning appears only if the user gives -fno-rtti options, so > > again we do > > not need to say anything more. > > > > The only cases left are compilers a where the default RTTI mode is > > DISABLED. > > Here an addendum like '[-frtti]' may make sense. AFAIK, PS4 is the only > > compiler > > with this default, but there may be other such private compilers. > > > > So should we append '[-frtti]' if Target.getTriple().isPS4() is true? > > So should we append '[-frtti]' if Target.getTriple().isPS4() is true? > > Personally, I'd be OK with producing a suggestion of how to enable RTTI based > on the PS4 triple. But I'd also be OK with not enhancing this diagnostic to > suggest how to turn on RTTI (that is, going with the patch as originally > proposed here). > > If clang-cl produced a warning when a dynamic_cast or typeid construct was > encountered in `/GR-` mode, then I'd feel it's worth complicating the code to > provide a target-sensitive way for advising the user how to turn RTTI on. > But clang-cl doesn't produce a warning in that case, so the effort to add the > framework for producing a target-sensitive warning doesn't seem worth it to > me. > > Improving clang-cl to produce a diagnostic in this `/GR-` situation seems > like a good idea, but separate from this proposed patch. If that work gets > done at some point, then it would be natural to revisit this diagnostic at > that time. If clang-cl is not a consideration, then I think the easiest and clearest way to do this is simply to say `requires -frtti` without hair-splitting which targets default which way. Repository: rC Clang https://reviews.llvm.org/D47291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47554: [analyzer] Check for dead/impossible status checks
trixirt updated this revision to Diff 150003. trixirt added a comment. cleanup of last patch Repository: rC Clang https://reviews.llvm.org/D47554 Files: include/clang/AST/ExprCXX.h include/clang/ASTMatchers/ASTMatchers.h include/clang/StaticAnalyzer/Checkers/Checkers.td lib/AST/ExprCXX.cpp lib/ASTMatchers/Dynamic/Registry.cpp lib/CodeGen/CGExprCXX.cpp lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp test/Analysis/dead-status-new-nullptr.cpp Index: test/Analysis/dead-status-new-nullptr.cpp === --- /dev/null +++ test/Analysis/dead-status-new-nullptr.cpp @@ -0,0 +1,44 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=deadcode.DeadStatus -verify %s + +void *t1(unsigned n, bool *fail); +void *t1(unsigned n, bool *fail) { + char *m = new char[n]; + if (nullptr == m) // expected-warning{{This variable can never be a nullptr}} +*fail = true; + else +*fail = false; + return m; +} + +void *t2(unsigned n, bool *fail); +void *t2(unsigned n, bool *fail) { + char *m = new char[n]; + if (m == nullptr) // expected-warning{{This variable can never be a nullptr}} +*fail = true; + else +*fail = false; + return m; +} + +// a variant of nullptr +void *t3(unsigned n, bool *fail); +void *t3(unsigned n, bool *fail) { + char *m = new char[n]; + if (m == 0) // expected-warning{{This variable can never be a nullptr}} +*fail = true; + else +*fail = false; + return m; +} + +// a variant of nullptr +#define NULL __null +void *t4(unsigned n, bool *fail); +void *t4(unsigned n, bool *fail) { + char *m = new char[n]; + if (m == NULL) // expected-warning{{This variable can never be a nullptr}} +*fail = true; + else +*fail = false; + return m; +} Index: lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp === --- /dev/null +++ lib/StaticAnalyzer/Checkers/DeadStatusChecker.cpp @@ -0,0 +1,140 @@ +//==- DeadStatusChecker.cpp - Check for impossible status -*- 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 a DeadStatus, a checker that looks for impossible +// status checks. +// +//===--===// + +#include "ClangSACheckers.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class DeadStatusChecker : public Checker { + class Walker : public RecursiveASTVisitor { +BugReporter +const DeadStatusChecker *Checker; +AnalysisDeclContext *AC; +llvm::SmallPtrSet throwingNews; + +const Expr *hasNullptr(const BinaryOperator *BO) const { + const Expr *ret = nullptr; + const Expr *E[2] = {BO->getLHS()->IgnoreParenCasts(), + BO->getRHS()->IgnoreParenCasts()}; + const Expr *EO[2] = {E[1], E[0]}; + for (int i = 0; i < 2; i++) { +if (isa(E[i]) || isa(E[i]) || +(isa(E[i]) && + dyn_cast(E[i])->getValue() == 0)) { + ret = EO[i]; + break; +} + } + return ret; +} + +const Expr *hasThrowingNew(const BinaryOperator *BO) const { + const Expr *ret = nullptr; + const Expr *rhs = BO->getRHS()->IgnoreParenCasts(); + if (isa(rhs)) { +auto NE = static_cast(rhs); +if (!NE->shouldNullCheckAllocation()) { + ret = BO->getLHS()->IgnoreParenCasts(); +} + } + return ret; +} + +bool hasThrowingNew(const VarDecl *VD) const { + bool ret = false; + const Expr *E = VD->getInit(); + if (E != nullptr) { +E = E->IgnoreParenCasts(); +if (isa(E)) { + auto NE = static_cast(E); + if (!NE->shouldNullCheckAllocation()) { +ret = true; + } +} + } + return ret; +} + + public: +explicit Walker(BugReporter , const DeadStatusChecker *C, +AnalysisDeclContext *AC) +: BR(BR), Checker(C), AC(AC) {} +bool VisitBinaryOperator(const BinaryOperator *BO) { + const Expr *E = nullptr; + BinaryOperator::Opcode Op = BO->getOpcode(); + if (BinaryOperator::isEqualityOp(Op)) { +E = hasNullptr(BO); + } else if (BinaryOperator::isAssignmentOp(Op)) { +E = hasThrowingNew(BO); + } + if (E != nullptr) { +if (isa(E)) { + auto DR = static_cast(E); + if
[PATCH] D47787: [clangd] Adjust symbol score based on crude symbol type.
sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. Numbers are guesses to be adjusted later. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47787 Files: clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp Index: unittests/clangd/QualityTests.cpp === --- unittests/clangd/QualityTests.cpp +++ unittests/clangd/QualityTests.cpp @@ -41,20 +41,23 @@ EXPECT_FALSE(Quality.Deprecated); EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority); EXPECT_EQ(Quality.References, SymbolQualitySignals().References); + EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable); Symbol F = findSymbol(Symbols, "f"); F.References = 24; // TestTU doesn't count references, so fake it. Quality = {}; Quality.merge(F); EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index. EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority); EXPECT_EQ(Quality.References, 24u); + EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function); Quality = {}; Quality.merge(CodeCompletionResult((AST, "f"), /*Priority=*/42)); EXPECT_TRUE(Quality.Deprecated); EXPECT_EQ(Quality.SemaCCPriority, 42u); EXPECT_EQ(Quality.References, SymbolQualitySignals().References); + EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function); } TEST(QualityTests, SymbolRelevanceSignalExtraction) { @@ -123,6 +126,12 @@ HighPriority.SemaCCPriority = 20; EXPECT_GT(HighPriority.evaluate(), Default.evaluate()); EXPECT_LT(LowPriority.evaluate(), Default.evaluate()); + + SymbolQualitySignals Variable, Macro; + Variable.Category = SymbolQualitySignals::Variable; + Macro.Category = SymbolQualitySignals::Macro; + EXPECT_GT(Variable.evaluate(), Default.evaluate()); + EXPECT_LT(Macro.evaluate(), Default.evaluate()); } TEST(QualityTests, SymbolRelevanceSignalsSanity) { Index: clangd/Quality.h === --- clangd/Quality.h +++ clangd/Quality.h @@ -50,6 +50,15 @@ bool Deprecated = false; unsigned References = 0; + enum SymbolCategory { +Variable, +Macro, +Type, +Function, +Namespace, +Unknown, + } Category = Unknown; + void merge(const CodeCompletionResult ); void merge(const Symbol ); Index: clangd/Quality.cpp === --- clangd/Quality.cpp +++ clangd/Quality.cpp @@ -9,6 +9,7 @@ #include "Quality.h" #include "index/Index.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclVisitor.h" #include "clang/Basic/SourceManager.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "llvm/Support/FormatVariadic.h" @@ -29,14 +30,82 @@ return false; } +static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl ) { + class Switch + : public ConstDeclVisitor { + public: +#define MAP(DeclType, Category)\ + SymbolQualitySignals::SymbolCategory Visit##DeclType(const DeclType *) { \ +return SymbolQualitySignals::Category;\ + } + +MAP(NamespaceDecl, Namespace); +MAP(NamespaceAliasDecl, Namespace); +MAP(TypeDecl, Type); +MAP(TypeAliasTemplateDecl, Type); +MAP(ClassTemplateDecl, Type); +MAP(ValueDecl, Variable); +MAP(VarTemplateDecl, Variable); +MAP(FunctionDecl, Function); +MAP(FunctionTemplateDecl, Function); +MAP(Decl, Unknown); + }; + return Switch().Visit(); +} + +static SymbolQualitySignals::SymbolCategory +categorize(const index::SymbolInfo ) { + switch (D.Kind) { +case index::SymbolKind::Namespace: +case index::SymbolKind::NamespaceAlias: + return SymbolQualitySignals::Namespace; +case index::SymbolKind::Macro: + return SymbolQualitySignals::Macro; +case index::SymbolKind::Enum: +case index::SymbolKind::Struct: +case index::SymbolKind::Class: +case index::SymbolKind::Protocol: +case index::SymbolKind::Extension: +case index::SymbolKind::Union: +case index::SymbolKind::TypeAlias: + return SymbolQualitySignals::Type; +case index::SymbolKind::Function: +case index::SymbolKind::ClassMethod: +case index::SymbolKind::InstanceMethod: +case index::SymbolKind::StaticMethod: +case index::SymbolKind::InstanceProperty: +case index::SymbolKind::ClassProperty: +case index::SymbolKind::StaticProperty: +case index::SymbolKind::Constructor: +case index::SymbolKind::Destructor: +case index::SymbolKind::ConversionFunction: + return SymbolQualitySignals::Function; +case index::SymbolKind::Variable: +case index::SymbolKind::Field: +case index::SymbolKind::EnumConstant: +case index::SymbolKind::Parameter: +
[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)
ioeric added inline comments. Comment at: unittests/clangd/QualityTests.cpp:1 //===-- SourceCodeTests.cpp *- C++ -*-===// // sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > ioeric wrote: > > > > Could you also add a test case for code completion? > > > The code completion scoring is tested in SymbolRelevanceSignalsSanity: > > > file scope is boosted compared to default when the query is > > > code-complete, but not when it's generic. > > > > > > What kind of test do you mean? > > I was thinking a test case that covers the changes in CodeComplete.cpp e.g. > > check that Relevance and Quality play well together, and locals/members are > > boosted? Would that make sense? > Actually one of the purposes of pulling out the `Quality` module is to stop > writing such tests :-) > They're fragile because ranking depends on many factors, e.g. at the moment > you can't construct a completion candidate with a different scope that won't > also get a different sema priority, so it's not clear why a test is > passing/failing. > And for every signal, you need a test in code complete, and in workspace > symbols... > > It *would* be useful to have a smoke test in CodeCompletion to make sure > we're using those scores. Maybe it would make sense to turn > ReferencesAffectRanking or so into that? That makes sense. A smoke test in code completion sounds good. Repository: rL LLVM https://reviews.llvm.org/D47762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r334026 - [clangd] Boost code completion results that are narrowly scoped (local, members)
Author: sammccall Date: Tue Jun 5 09:30:25 2018 New Revision: 334026 URL: http://llvm.org/viewvc/llvm-project?rev=334026=rev Log: [clangd] Boost code completion results that are narrowly scoped (local, members) Summary: This signal is considered a relevance rather than a quality signal because it's dependent on the query (the fact that it's completion, and implicitly the query context). This is part of the effort to reduce reliance on Sema priority, so we can have consistent ranking between Index and Sema results. Reviewers: ioeric Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D47762 Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/Quality.cpp clang-tools-extra/trunk/clangd/Quality.h clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp clang-tools-extra/trunk/unittests/clangd/TestTU.cpp clang-tools-extra/trunk/unittests/clangd/TestTU.h Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=334026=334025=334026=diff == --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Jun 5 09:30:25 2018 @@ -51,11 +51,11 @@ template std::size_t getUsedBy class DeclTrackingASTConsumer : public ASTConsumer { public: - DeclTrackingASTConsumer(std::vector ) + DeclTrackingASTConsumer(std::vector ) : TopLevelDecls(TopLevelDecls) {} bool HandleTopLevelDecl(DeclGroupRef DG) override { -for (const Decl *D : DG) { +for (Decl *D : DG) { // ObjCMethodDecl are not actually top-level decls. if (isa(D)) continue; @@ -66,14 +66,12 @@ public: } private: - std::vector + std::vector }; class ClangdFrontendAction : public SyntaxOnlyAction { public: - std::vector takeTopLevelDecls() { -return std::move(TopLevelDecls); - } + std::vector takeTopLevelDecls() { return std::move(TopLevelDecls); } protected: std::unique_ptr CreateASTConsumer(CompilerInstance , @@ -82,7 +80,7 @@ protected: } private: - std::vector TopLevelDecls; + std::vector TopLevelDecls; }; class CppFilePreambleCallbacks : public PreambleCallbacks { @@ -174,7 +172,7 @@ ParsedAST::Build(std::unique_ptr ParsedDecls = Action->takeTopLevelDecls(); + std::vector ParsedDecls = Action->takeTopLevelDecls(); std::vector Diags = ASTDiags.take(); // Add diagnostics from the preamble, if any. if (Preamble) @@ -210,7 +208,7 @@ const Preprocessor ::getPrepro return Clang->getPreprocessor(); } -ArrayRef ParsedAST::getLocalTopLevelDecls() { +ArrayRef ParsedAST::getLocalTopLevelDecls() { return LocalTopLevelDecls; } @@ -261,7 +259,7 @@ PreambleData::PreambleData(PrecompiledPr ParsedAST::ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, - std::vector LocalTopLevelDecls, + std::vector LocalTopLevelDecls, std::vector Diags, std::vector Inclusions) : Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Diags(std::move(Diags)), Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=334026=334025=334026=diff == --- clang-tools-extra/trunk/clangd/ClangdUnit.h (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.h Tue Jun 5 09:30:25 2018 @@ -91,7 +91,8 @@ public: /// This function returns top-level decls present in the main file of the AST. /// The result does not include the decls that come from the preamble. - ArrayRef getLocalTopLevelDecls(); + /// (These should be const, but RecursiveASTVisitor requires Decl*). + ArrayRef getLocalTopLevelDecls(); const std::vector () const; @@ -104,8 +105,8 @@ private: ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, -std::vector LocalTopLevelDecls, -std::vector Diags, std::vector Inclusions); +std::vector LocalTopLevelDecls, std::vector Diags, +std::vector Inclusions); // In-memory preambles must outlive the AST, it is important that this member // goes before Clang and Action. @@ -122,7 +123,7 @@ private: std::vector Diags; // Top-level decls inside the current file. Not that this does not include // top-level decls from the preamble. - std::vector LocalTopLevelDecls; + std::vector LocalTopLevelDecls; std::vector Inclusions; }; Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)
This revision was automatically updated to reflect the committed changes. Closed by commit rL334026: [clangd] Boost code completion results that are narrowly scoped (local, members) (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47762?vs=149951=14#toc Repository: rL LLVM https://reviews.llvm.org/D47762 Files: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/Quality.cpp clang-tools-extra/trunk/clangd/Quality.h clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp clang-tools-extra/trunk/unittests/clangd/TestTU.cpp clang-tools-extra/trunk/unittests/clangd/TestTU.h Index: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp === --- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp +++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp @@ -10,6 +10,7 @@ #include "TestFS.h" #include "index/FileIndex.h" #include "index/MemIndex.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Frontend/Utils.h" @@ -49,7 +50,6 @@ return MemIndex::build(headerSymbols()); } -// Look up a symbol by qualified name, which must be unique. const Symbol (const SymbolSlab , llvm::StringRef QName) { const Symbol *Result = nullptr; for (const Symbol : Slab) { @@ -92,5 +92,26 @@ return LookupDecl(*Scope, Components.back()); } +const NamedDecl (ParsedAST , llvm::StringRef Name) { + struct Visitor : RecursiveASTVisitor { +llvm::StringRef Name; +llvm::SmallVector Decls; +bool VisitNamedDecl(const NamedDecl *ND) { + if (auto *ID = ND->getIdentifier()) +if (ID->getName() == Name) + Decls.push_back(ND); + return true; +} + } Visitor; + Visitor.Name = Name; + for (Decl *D : AST.getLocalTopLevelDecls()) +Visitor.TraverseDecl(D); + if (Visitor.Decls.size() != 1) { +ADD_FAILURE() << Visitor.Decls.size() << " symbols named " << Name; +assert(Visitor.Decls.size() == 1); + } + return *Visitor.Decls.front(); +} + } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp @@ -69,6 +69,8 @@ [[deprecated]] int deprecated() { return 0; } + +namespace { struct X { void y() { int z; } }; } )cpp"; auto AST = Test.build(); @@ -78,6 +80,7 @@ /*Accessible=*/false)); EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals().NameMatch); EXPECT_TRUE(Relevance.Forbidden); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope); Relevance = {}; Relevance.merge(CodeCompletionResult((AST, "main"), 42)); @@ -88,6 +91,16 @@ Relevance = {}; Relevance.merge(CodeCompletionResult((AST, "header_main"), 42)); EXPECT_FLOAT_EQ(Relevance.ProximityScore, 1.0) << "Current file and header"; + + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "X"), 42)); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope); + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "y"), 42)); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::ClassScope); + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "z"), 42)); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FunctionScope); } // Do the signals move the scores in the direction we expect? @@ -127,6 +140,12 @@ SymbolRelevanceSignals WithProximity; WithProximity.ProximityScore = 0.2f; EXPECT_GT(WithProximity.evaluate(), Default.evaluate()); + + SymbolRelevanceSignals Scoped; + Scoped.Scope = SymbolRelevanceSignals::FileScope; + EXPECT_EQ(Scoped.evaluate(), Default.evaluate()); + Scoped.Query = SymbolRelevanceSignals::CodeComplete; + EXPECT_GT(Scoped.evaluate(), Default.evaluate()); } TEST(QualityTests, SortText) { Index: clang-tools-extra/trunk/unittests/clangd/TestTU.h === --- clang-tools-extra/trunk/unittests/clangd/TestTU.h +++ clang-tools-extra/trunk/unittests/clangd/TestTU.h @@ -53,6 +53,8 @@ const Symbol (const SymbolSlab &, llvm::StringRef QName); // Look up an AST symbol by qualified name, which must be unique and top-level. const NamedDecl (ParsedAST , llvm::StringRef QName); +// Look up a main-file AST symbol by unqualified name, which must be unique. +const NamedDecl (ParsedAST , llvm::StringRef Name); } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/Quality.cpp === ---
[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)
sammccall marked an inline comment as done. sammccall added inline comments. Comment at: unittests/clangd/QualityTests.cpp:1 //===-- SourceCodeTests.cpp *- C++ -*-===// // ioeric wrote: > sammccall wrote: > > ioeric wrote: > > > Could you also add a test case for code completion? > > The code completion scoring is tested in SymbolRelevanceSignalsSanity: file > > scope is boosted compared to default when the query is code-complete, but > > not when it's generic. > > > > What kind of test do you mean? > I was thinking a test case that covers the changes in CodeComplete.cpp e.g. > check that Relevance and Quality play well together, and locals/members are > boosted? Would that make sense? Actually one of the purposes of pulling out the `Quality` module is to stop writing such tests :-) They're fragile because ranking depends on many factors, e.g. at the moment you can't construct a completion candidate with a different scope that won't also get a different sema priority, so it's not clear why a test is passing/failing. And for every signal, you need a test in code complete, and in workspace symbols... It *would* be useful to have a smoke test in CodeCompletion to make sure we're using those scores. Maybe it would make sense to turn ReferencesAffectRanking or so into that? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43667: [clang-doc] Implement a YAML generator
juliehockett updated this revision to Diff 149997. juliehockett marked 3 inline comments as done. juliehockett added a comment. Fixing comments https://reviews.llvm.org/D43667 Files: clang-doc/CMakeLists.txt clang-doc/Generators.cpp clang-doc/Generators.h clang-doc/Representation.h clang-doc/YAMLGenerator.cpp clang-doc/tool/ClangDocMain.cpp test/clang-doc/yaml-comments.cpp test/clang-doc/yaml-namespace.cpp test/clang-doc/yaml-record.cpp Index: test/clang-doc/yaml-record.cpp === --- /dev/null +++ test/clang-doc/yaml-record.cpp @@ -0,0 +1,250 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: echo "" > %t/compile_flags.txt +// RUN: cp "%s" "%t/test.cpp" +// RUN: clang-doc -doxygen -p %t %t/test.cpp -output=%t/docs +// RUN: cat %t/docs/A.yaml | FileCheck %s --check-prefix=CHECK-A +// RUN: cat %t/docs/Bc.yaml | FileCheck %s --check-prefix=CHECK-BC +// RUN: cat %t/docs/B.yaml | FileCheck %s --check-prefix=CHECK-B +// RUN: cat %t/docs/C.yaml | FileCheck %s --check-prefix=CHECK-C +// RUN: cat %t/docs/D.yaml | FileCheck %s --check-prefix=CHECK-D +// RUN: cat %t/docs/E.yaml | FileCheck %s --check-prefix=CHECK-E +// RUN: cat %t/docs/E/ProtectedMethod.yaml | FileCheck %s --check-prefix=CHECK-EPM +// RUN: cat %t/docs/E/E.yaml | FileCheck %s --check-prefix=CHECK-ECON +// RUN: cat %t/docs/E/'~E.yaml' | FileCheck %s --check-prefix=CHECK-EDES +// RUN: cat %t/docs/F.yaml | FileCheck %s --check-prefix=CHECK-F +// RUN: cat %t/docs/X.yaml | FileCheck %s --check-prefix=CHECK-X +// RUN: cat %t/docs/X/Y.yaml | FileCheck %s --check-prefix=CHECK-Y +// RUN: cat %t/docs/H.yaml | FileCheck %s --check-prefix=CHECK-H +// RUN: cat %t/docs/H/I.yaml | FileCheck %s --check-prefix=CHECK-I + +union A { int X; int Y; }; + +// CHECK-A: --- +// CHECK-A-NEXT: USR: 'ACE81AFA6627B4CEF2B456FB6E1252925674AF7E' +// CHECK-A-NEXT: Name:'A' +// CHECK-A-NEXT: DefLocation: +// CHECK-A-NEXT: LineNumber: 21 +// CHECK-A-NEXT: Filename:'{{.*}}' +// CHECK-A-NEXT: TagType: Union +// CHECK-A-NEXT: Members: +// CHECK-A-NEXT: - Type: +// CHECK-A-NEXT: Name:'int' +// CHECK-A-NEXT: Name:'X' +// CHECK-A-NEXT: - Type: +// CHECK-A-NEXT: Name:'int' +// CHECK-A-NEXT: Name:'Y' +// CHECK-A-NEXT: ... + + +enum B { X, Y }; + +// CHECK-B: --- +// CHECK-B-NEXT: USR: 'FC07BD34D5E77782C263FA97929EA8753740' +// CHECK-B-NEXT: Name:'B' +// CHECK-B-NEXT: DefLocation: +// CHECK-B-NEXT: LineNumber: 40 +// CHECK-B-NEXT: Filename:'{{.*}}' +// CHECK-B-NEXT: Members: +// CHECK-B-NEXT: - 'X' +// CHECK-B-NEXT: - 'Y' +// CHECK-B-NEXT: ... + +enum class Bc { A, B }; + +// CHECK-BC: --- +// CHECK-BC-NEXT: USR: '1E3438A08BA22025C0B46289FF0686F92C8924C5' +// CHECK-BC-NEXT: Name:'Bc' +// CHECK-BC-NEXT: DefLocation: +// CHECK-BC-NEXT: LineNumber: 53 +// CHECK-BC-NEXT: Filename:'{{.*}}' +// CHECK-BC-NEXT: Scoped: true +// CHECK-BC-NEXT: Members: +// CHECK-BC-NEXT: - 'A' +// CHECK-BC-NEXT: - 'B' +// CHECK-BC-NEXT: ... + +struct C { int i; }; + +// CHECK-C: --- +// CHECK-C-NEXT: USR: '06B5F6A19BA9F6A832E127C9968282B94619B210' +// CHECK-C-NEXT: Name:'C' +// CHECK-C-NEXT: DefLocation: +// CHECK-C-NEXT: LineNumber: 67 +// CHECK-C-NEXT: Filename:'{{.*}}' +// CHECK-C-NEXT: Members: +// CHECK-C-NEXT: - Type: +// CHECK-C-NEXT: Name:'int' +// CHECK-C-NEXT: Name:'i' +// CHECK-C-NEXT: ... + +class D {}; + +// CHECK-D: --- +// CHECK-D-NEXT: USR: '0921737541208B8FA9BB42B60F78AC1D779AA054' +// CHECK-D-NEXT: Name:'D' +// CHECK-D-NEXT: DefLocation: +// CHECK-D-NEXT: LineNumber: 81 +// CHECK-D-NEXT: Filename:'{{.*}}' +// CHECK-D-NEXT: TagType: Class +// CHECK-D-NEXT: ... + +class E { +public: + E() {} + +// CHECK-ECON: --- +// CHECK-ECON-NEXT: USR: 'DEB4AC1CD9253CD9EF7FBE6BCAC506D77984ABD4' +// CHECK-ECON-NEXT: Name:'E' +// CHECK-ECON-NEXT: Namespace: +// CHECK-ECON-NEXT: - Type:Record +// CHECK-ECON-NEXT: Name:'E' +// CHECK-ECON-NEXT: USR: '289584A8E0FF4178A794622A547AA622503967A1' +// CHECK-ECON-NEXT: DefLocation: +// CHECK-ECON-NEXT: LineNumber: 94 +// CHECK-ECON-NEXT: Filename:'{{.*}}' +// CHECK-ECON-NEXT: IsMethod:true +// CHECK-ECON-NEXT: Parent: +// CHECK-ECON-NEXT: Type:Record +// CHECK-ECON-NEXT: Name:'E' +// CHECK-ECON-NEXT: USR: '289584A8E0FF4178A794622A547AA622503967A1' +// CHECK-ECON-NEXT: ReturnType: +// CHECK-ECON-NEXT: Type: +// CHECK-ECON-NEXT: Name:'void' +// CHECK-ECON-NEXT: ... + +
[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions
martong added a comment. > Are you OK with incremental review? Hi Aleksei, thanks for reviewing this! Of course I am OK with an incremental review. Just a minor thing, I'll be back in the office only at the 18th, so I can address all of the comments only then. Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47630: [Sema] Allow creating types with multiple of the same addrspace.
Anastasia added inline comments. Comment at: test/Sema/address_spaces.c:17 int *_AS1 _AS2 *Z; // expected-error {{multiple address spaces specified for type}} + int *_AS1 _AS1 *M; ebevhan wrote: > bader wrote: > > I think it might be valuable to give a warning or remark to user. > > Using the same address space qualifier multiple times is not something > > OpenCL C developers are supposed to do. > > > The test is obviously a bit contrived, but it could happen by mistake, or as > a result of some typedef or macro combination. It also cannot go wrong, so > there's no harm in it happening. > > I see your point, though. A warning feels like a bit much, so I'm not sure > what else to use. A note? Just checked for const qualifier we get a warning: warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier] We could do the same... not sure if we could try to share the diagnostic as well. Repository: rC Clang https://reviews.llvm.org/D47630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
craig.topper added inline comments. Comment at: test/CodeGen/target-features-error-2.c:39 __m128d need_avx(__m128d a, __m128d b) { return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs target feature avx}} } GBuella wrote: > craig.topper wrote: > > The 4 compare functions here are basically the same for the purpose of this > > check. What value do we get by testing all 4 of them? > Which four more specifically? _mm_cmp_ps, _mm_cmp_ss, _mm_cmp_pd, _mm_cmp_sd. Or the cases for NEED_AVX_2-5. They're all basically checking the same thing. There is nothing different about the command lines other than the -D NEED_AVX https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47720: [DebugInfo] Inline for without DebugLocation
gramanas updated this revision to Diff 149996. gramanas added a comment. Add comment explaining the test Repository: rC Clang https://reviews.llvm.org/D47720 Files: test/CodeGen/debug-info-inline-for.c Index: test/CodeGen/debug-info-inline-for.c === --- /dev/null +++ test/CodeGen/debug-info-inline-for.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s +// Check that clang emits Debug location in the phi instruction + +int func(int n) { + int a; + for(a = 10; a>0 && n++; a--); + return n; +} + +// CHECK: land.end: +// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]] + +// CHECK: ![[DbgLoc]] = !DILocation( Index: test/CodeGen/debug-info-inline-for.c === --- /dev/null +++ test/CodeGen/debug-info-inline-for.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s +// Check that clang emits Debug location in the phi instruction + +int func(int n) { + int a; + for(a = 10; a>0 && n++; a--); + return n; +} + +// CHECK: land.end: +// CHECK-NEXT: {{.*}} = phi i1 {{.*}} !dbg ![[DbgLoc:[0-9]+]] + +// CHECK: ![[DbgLoc]] = !DILocation( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47669: [cmake] Support LLD for CLANG_ORDER_FILE
ruiu added a comment. LGTM Repository: rL LLVM https://reviews.llvm.org/D47669 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
GBuella added inline comments. Comment at: test/CodeGen/target-features-error-2.c:39 __m128d need_avx(__m128d a, __m128d b) { return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs target feature avx}} } craig.topper wrote: > The 4 compare functions here are basically the same for the purpose of this > check. What value do we get by testing all 4 of them? Which four more specifically? https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes
craig.topper added inline comments. Comment at: test/CodeGen/target-features-error-2.c:39 __m128d need_avx(__m128d a, __m128d b) { return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs target feature avx}} } The 4 compare functions here are basically the same for the purpose of this check. What value do we get by testing all 4 of them? https://reviews.llvm.org/D46541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r334021 - [CUDA][HIP] Do not emit type info when compiling for device
Author: yaxunl Date: Tue Jun 5 08:11:02 2018 New Revision: 334021 URL: http://llvm.org/viewvc/llvm-project?rev=334021=rev Log: [CUDA][HIP] Do not emit type info when compiling for device CUDA/HIP does not support RTTI on device side, therefore there is no point of emitting type info when compiling for device. Emitting type info for device not only clutters the IR with useless global variables, but also causes undefined symbol at linking since vtable for cxxabiv1::class_type_info has external linkage. Differential Revision: https://reviews.llvm.org/D47694 Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/test/CodeGenCUDA/device-vtable.cu Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=334021=334020=334021=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Jun 5 08:11:02 2018 @@ -4900,7 +4900,7 @@ llvm::Constant *CodeGenModule::GetAddrOf // Return a bogus pointer if RTTI is disabled, unless it's for EH. // FIXME: should we even be calling this method if RTTI is disabled // and it's not for EH? - if (!ForEH && !getLangOpts().RTTI) + if ((!ForEH && !getLangOpts().RTTI) || getLangOpts().CUDAIsDevice) return llvm::Constant::getNullValue(Int8PtrTy); if (ForEH && Ty->isObjCObjectPointerType() && Modified: cfe/trunk/test/CodeGenCUDA/device-vtable.cu URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/device-vtable.cu?rev=334021=334020=334021=diff == --- cfe/trunk/test/CodeGenCUDA/device-vtable.cu (original) +++ cfe/trunk/test/CodeGenCUDA/device-vtable.cu Tue Jun 5 08:11:02 2018 @@ -19,7 +19,9 @@ struct H { //CHECK-HOST: @_ZTV1H = //CHECK-HOST-SAME: @_ZN1H6methodEv //CHECK-DEVICE-NOT: @_ZTV1H = - +//CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +//CHECK-DEVICE-NOT: @_ZTS1H +//CHECK-DEVICE-NOT: @_ZTI1H struct D { __device__ virtual void method(); }; @@ -27,7 +29,9 @@ struct D { //CHECK-DEVICE: @_ZTV1D //CHECK-DEVICE-SAME: @_ZN1D6methodEv //CHECK-HOST-NOT: @_ZTV1D - +//CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +//CHECK-DEVICE-NOT: @_ZTS1D +//CHECK-DEVICE-NOT: @_ZTI1D // This is the case with mixed host and device virtual methods. It's // impossible to emit a valid vtable in that case because only host or // only device methods would be available during host or device @@ -45,6 +49,9 @@ struct HD { // CHECK-HOST-NOT: @_ZN2HD8d_methodEv // CHECK-HOST-SAME: null // CHECK-BOTH-SAME: ] +// CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +// CHECK-DEVICE-NOT: @_ZTS2HD +// CHECK-DEVICE-NOT: @_ZTI2HD void H::method() {} //CHECK-HOST: define void @_ZN1H6methodEv ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47694: [CUDA][HIP] Do not emit type info when compiling for device
This revision was automatically updated to reflect the committed changes. Closed by commit rC334021: [CUDA][HIP] Do not emit type info when compiling for device (authored by yaxunl, committed by ). Repository: rC Clang https://reviews.llvm.org/D47694 Files: lib/CodeGen/CodeGenModule.cpp test/CodeGenCUDA/device-vtable.cu Index: test/CodeGenCUDA/device-vtable.cu === --- test/CodeGenCUDA/device-vtable.cu +++ test/CodeGenCUDA/device-vtable.cu @@ -19,15 +19,19 @@ //CHECK-HOST: @_ZTV1H = //CHECK-HOST-SAME: @_ZN1H6methodEv //CHECK-DEVICE-NOT: @_ZTV1H = - +//CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +//CHECK-DEVICE-NOT: @_ZTS1H +//CHECK-DEVICE-NOT: @_ZTI1H struct D { __device__ virtual void method(); }; //CHECK-DEVICE: @_ZTV1D //CHECK-DEVICE-SAME: @_ZN1D6methodEv //CHECK-HOST-NOT: @_ZTV1D - +//CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +//CHECK-DEVICE-NOT: @_ZTS1D +//CHECK-DEVICE-NOT: @_ZTI1D // This is the case with mixed host and device virtual methods. It's // impossible to emit a valid vtable in that case because only host or // only device methods would be available during host or device @@ -45,6 +49,9 @@ // CHECK-HOST-NOT: @_ZN2HD8d_methodEv // CHECK-HOST-SAME: null // CHECK-BOTH-SAME: ] +// CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +// CHECK-DEVICE-NOT: @_ZTS2HD +// CHECK-DEVICE-NOT: @_ZTI2HD void H::method() {} //CHECK-HOST: define void @_ZN1H6methodEv Index: lib/CodeGen/CodeGenModule.cpp === --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -4900,7 +4900,7 @@ // Return a bogus pointer if RTTI is disabled, unless it's for EH. // FIXME: should we even be calling this method if RTTI is disabled // and it's not for EH? - if (!ForEH && !getLangOpts().RTTI) + if ((!ForEH && !getLangOpts().RTTI) || getLangOpts().CUDAIsDevice) return llvm::Constant::getNullValue(Int8PtrTy); if (ForEH && Ty->isObjCObjectPointerType() && Index: test/CodeGenCUDA/device-vtable.cu === --- test/CodeGenCUDA/device-vtable.cu +++ test/CodeGenCUDA/device-vtable.cu @@ -19,15 +19,19 @@ //CHECK-HOST: @_ZTV1H = //CHECK-HOST-SAME: @_ZN1H6methodEv //CHECK-DEVICE-NOT: @_ZTV1H = - +//CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +//CHECK-DEVICE-NOT: @_ZTS1H +//CHECK-DEVICE-NOT: @_ZTI1H struct D { __device__ virtual void method(); }; //CHECK-DEVICE: @_ZTV1D //CHECK-DEVICE-SAME: @_ZN1D6methodEv //CHECK-HOST-NOT: @_ZTV1D - +//CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +//CHECK-DEVICE-NOT: @_ZTS1D +//CHECK-DEVICE-NOT: @_ZTI1D // This is the case with mixed host and device virtual methods. It's // impossible to emit a valid vtable in that case because only host or // only device methods would be available during host or device @@ -45,6 +49,9 @@ // CHECK-HOST-NOT: @_ZN2HD8d_methodEv // CHECK-HOST-SAME: null // CHECK-BOTH-SAME: ] +// CHECK-DEVICE-NOT: @_ZTVN10__cxxabiv117__class_type_infoE +// CHECK-DEVICE-NOT: @_ZTS2HD +// CHECK-DEVICE-NOT: @_ZTI2HD void H::method() {} //CHECK-HOST: define void @_ZN1H6methodEv Index: lib/CodeGen/CodeGenModule.cpp === --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -4900,7 +4900,7 @@ // Return a bogus pointer if RTTI is disabled, unless it's for EH. // FIXME: should we even be calling this method if RTTI is disabled // and it's not for EH? - if (!ForEH && !getLangOpts().RTTI) + if ((!ForEH && !getLangOpts().RTTI) || getLangOpts().CUDAIsDevice) return llvm::Constant::getNullValue(Int8PtrTy); if (ForEH && Ty->isObjCObjectPointerType() && ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47784: [MS][ARM64]: Promote _setjmp to_setjmpex as there is no _setjmp in the ARM64 libvcruntime.lib
arm-chrjan01 created this revision. arm-chrjan01 added reviewers: simon_tatham, majnemer, rnk. Herald added subscribers: cfe-commits, chrib. Repository: rC Clang https://reviews.llvm.org/D47784 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGen/ms-setjmp.c Index: test/CodeGen/ms-setjmp.c === --- test/CodeGen/ms-setjmp.c +++ test/CodeGen/ms-setjmp.c @@ -1,7 +1,9 @@ // RUN: %clang_cc1 -fms-extensions -DDECLARE_SETJMP -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=I386 %s // RUN: %clang_cc1 -fms-extensions -DDECLARE_SETJMP -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=X64 %s +// RUN: %clang_cc1 -fms-extensions -DDECLARE_SETJMP -triple aarch64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=AARCH64 %s // RUN: %clang_cc1 -fms-extensions -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=I386 %s // RUN: %clang_cc1 -fms-extensions -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=X64 %s +// RUN: %clang_cc1 -fms-extensions -triple aarch64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=AARCH64 %s typedef char jmp_buf[1]; #ifdef DECLARE_SETJMP @@ -21,12 +23,22 @@ // X64: %[[addr:.*]] = call i8* @llvm.frameaddress(i32 0) // X64: %[[call:.*]] = call i32 @_setjmp(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @jb, i32 0, i32 0), i8* %[[addr]]) // X64-NEXT: ret i32 %[[call]] + + // AARCH64-LABEL: define dso_local i32 @test_setjmp + // AARCH64: %[[addr:.*]] = call i8* @llvm.frameaddress(i32 0) + // AARCH64: %[[call:.*]] = call i32 @_setjmpex(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @jb, i32 0, i32 0), i8* %[[addr]]) + // AARCH64-NEXT: ret i32 %[[call]] } int test_setjmpex() { return _setjmpex(jb); // X64-LABEL: define dso_local i32 @test_setjmpex // X64: %[[addr:.*]] = call i8* @llvm.frameaddress(i32 0) // X64: %[[call:.*]] = call i32 @_setjmpex(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @jb, i32 0, i32 0), i8* %[[addr]]) // X64-NEXT: ret i32 %[[call]] + + // AARCH64-LABEL: define dso_local i32 @test_setjmpex + // AARCH64: %[[addr:.*]] = call i8* @llvm.frameaddress(i32 0) + // AARCH64: %[[call:.*]] = call i32 @_setjmpex(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @jb, i32 0, i32 0), i8* %[[addr]]) + // AARCH64-NEXT: ret i32 %[[call]] } Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -2841,6 +2841,18 @@ llvm::Value *Count = ConstantInt::get(IntTy, 0); llvm::Value *Args[] = {Buf, Count}; CS = EmitRuntimeCallOrInvoke(SetJmp3, Args); + } else if (getTarget().getTriple().getArch() == llvm::Triple::aarch64) { +llvm::Type *ArgTypes[] = {Int8PtrTy, Int8PtrTy}; +// There is no _setjmp in the ARM64 libvcruntime.lib. +// _setjmp is promoted to _setjmpex by the Microsoft C++ compiler. +llvm::Constant *SetJmp = CGM.CreateRuntimeFunction( +llvm::FunctionType::get(IntTy, ArgTypes, /*isVarArg=*/false), +"_setjmpex", ReturnsTwiceAttr, /*Local=*/true); +llvm::Value *FrameAddr = +Builder.CreateCall(CGM.getIntrinsic(Intrinsic::frameaddress), + ConstantInt::get(Int32Ty, 0)); +llvm::Value *Args[] = {Buf, FrameAddr}; +CS = EmitRuntimeCallOrInvoke(SetJmp, Args); } else { llvm::Type *ArgTypes[] = {Int8PtrTy, Int8PtrTy}; llvm::Constant *SetJmp = CGM.CreateRuntimeFunction( @@ -8484,7 +8496,7 @@ return EmitX86Select(CGF, Ops[4], Ternlog, PassThru); } -static Value *EmitX86SExtMask(CodeGenFunction , Value *Op, +static Value *EmitX86SExtMask(CodeGenFunction , Value *Op, llvm::Type *DstTy) { unsigned NumberOfElements = DstTy->getVectorNumElements(); Value *Mask = getMaskVecValue(CGF, Op, NumberOfElements); Index: test/CodeGen/ms-setjmp.c === --- test/CodeGen/ms-setjmp.c +++ test/CodeGen/ms-setjmp.c @@ -1,7 +1,9 @@ // RUN: %clang_cc1 -fms-extensions -DDECLARE_SETJMP -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=I386 %s // RUN: %clang_cc1 -fms-extensions -DDECLARE_SETJMP -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=X64 %s +// RUN: %clang_cc1 -fms-extensions -DDECLARE_SETJMP -triple aarch64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=AARCH64 %s // RUN: %clang_cc1 -fms-extensions -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=I386 %s // RUN: %clang_cc1 -fms-extensions -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck --check-prefix=X64 %s +// RUN: %clang_cc1 -fms-extensions -triple aarch64-windows-msvc -emit-llvm %s -o - | FileCheck
[PATCH] D47737: [clangd] Remove unused variables
ioeric added a comment. fyi, you could actually accept your own patch phab (e.g. when you would like post-commit review) :P Repository: rL LLVM https://reviews.llvm.org/D47737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
dmgreen added a comment. Thanks. I noticed in the paper that you used the name "unrollandjam", minus underscores. Should I change this use that spelling here? I have no strong opinion of one over the other (was just using what I had found from the Intel docs). https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47737: [clangd] Remove unused variables
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL334018: [clangd] Remove unused variables (authored by malaperle, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47737 Files: clang-tools-extra/trunk/clangd/ClangdServer.cpp Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -298,9 +298,8 @@ void ClangdServer::findDefinitions(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS, this](Callback> CB, -llvm::Expected InpAST) { + auto Action = [Pos, this](Callback> CB, +llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get())); @@ -391,10 +390,8 @@ void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { - - auto FS = FSProvider.getFileSystem(); - auto Action = [FS, Pos](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDocumentHighlights(InpAST->AST, Pos)); @@ -405,9 +402,8 @@ void ClangdServer::findHover(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::getHover(InpAST->AST, Pos)); Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -298,9 +298,8 @@ void ClangdServer::findDefinitions(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS, this](Callback> CB, -llvm::Expected InpAST) { + auto Action = [Pos, this](Callback> CB, +llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get())); @@ -391,10 +390,8 @@ void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { - - auto FS = FSProvider.getFileSystem(); - auto Action = [FS, Pos](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDocumentHighlights(InpAST->AST, Pos)); @@ -405,9 +402,8 @@ void ClangdServer::findHover(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::getHover(InpAST->AST, Pos)); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47737: [clangd] Remove unused variables
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rCTE334018: [clangd] Remove unused variables (authored by malaperle, committed by ). Changed prior to commit: https://reviews.llvm.org/D47737?vs=149832=149976#toc Repository: rL LLVM https://reviews.llvm.org/D47737 Files: clangd/ClangdServer.cpp Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -298,9 +298,8 @@ void ClangdServer::findDefinitions(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS, this](Callback> CB, -llvm::Expected InpAST) { + auto Action = [Pos, this](Callback> CB, +llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get())); @@ -391,10 +390,8 @@ void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { - - auto FS = FSProvider.getFileSystem(); - auto Action = [FS, Pos](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDocumentHighlights(InpAST->AST, Pos)); @@ -405,9 +402,8 @@ void ClangdServer::findHover(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::getHover(InpAST->AST, Pos)); Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -298,9 +298,8 @@ void ClangdServer::findDefinitions(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS, this](Callback> CB, -llvm::Expected InpAST) { + auto Action = [Pos, this](Callback> CB, +llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get())); @@ -391,10 +390,8 @@ void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { - - auto FS = FSProvider.getFileSystem(); - auto Action = [FS, Pos](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDocumentHighlights(InpAST->AST, Pos)); @@ -405,9 +402,8 @@ void ClangdServer::findHover(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::getHover(InpAST->AST, Pos)); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r334018 - [clangd] Remove unused variables
Author: malaperle Date: Tue Jun 5 07:07:45 2018 New Revision: 334018 URL: http://llvm.org/viewvc/llvm-project?rev=334018=rev Log: [clangd] Remove unused variables Summary: Signed-off-by: Marc-Andre Laperle Subscribers: klimek, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D47737 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=334018=334017=334018=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jun 5 07:07:45 2018 @@ -298,9 +298,8 @@ void ClangdServer::dumpAST(PathRef File, void ClangdServer::findDefinitions(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS, this](Callback> CB, -llvm::Expected InpAST) { + auto Action = [Pos, this](Callback> CB, +llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get())); @@ -391,10 +390,8 @@ ClangdServer::formatCode(llvm::StringRef void ClangdServer::findDocumentHighlights( PathRef File, Position Pos, Callback> CB) { - - auto FS = FSProvider.getFileSystem(); - auto Action = [FS, Pos](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::findDocumentHighlights(InpAST->AST, Pos)); @@ -405,9 +402,8 @@ void ClangdServer::findDocumentHighlight void ClangdServer::findHover(PathRef File, Position Pos, Callback> CB) { - auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos](Callback> CB, + llvm::Expected InpAST) { if (!InpAST) return CB(InpAST.takeError()); CB(clangd::getHover(InpAST->AST, Pos)); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44954: [clangd] Add "member" symbols to the index
This revision was automatically updated to reflect the committed changes. Closed by commit rL334017: [clangd] Add member symbols to the index (authored by malaperle, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44954 Files: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/CodeComplete.h clang-tools-extra/trunk/clangd/index/Index.h clang-tools-extra/trunk/clangd/index/MemIndex.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -32,6 +32,7 @@ using ::testing::Each; using ::testing::ElementsAre; using ::testing::Field; +using ::testing::IsEmpty; using ::testing::Not; using ::testing::UnorderedElementsAre; @@ -153,6 +154,7 @@ Sym.CompletionSnippetInsertText = Sym.Name; Sym.CompletionLabel = Sym.Name; Sym.SymInfo.Kind = Kind; + Sym.IsIndexedForCodeCompletion = true; return Sym; } Symbol func(StringRef Name) { // Assumes the function has no args. @@ -684,6 +686,20 @@ Contains(AllOf(Named("baz"), Doc("Multi-line\nblock comment"; } +TEST(CompletionTest, GlobalCompletionFiltering) { + + Symbol Class = cls("XYZ"); + Class.IsIndexedForCodeCompletion = false; + Symbol Func = func("XYZ::f"); + Func.IsIndexedForCodeCompletion = false; + + auto Results = completions(R"(// void f() { + XYZ::f^ + })", + {Class, Func}); + EXPECT_THAT(Results.items, IsEmpty()); +} + TEST(CodeCompleteTest, DisableTypoCorrection) { auto Results = completions(R"cpp( namespace clang { int v; } Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp === --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp @@ -67,6 +67,9 @@ Pos.end.character); } MATCHER_P(Refs, R, "") { return int(arg.References) == R; } +MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") { + return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion; +} namespace clang { namespace clangd { @@ -132,9 +135,13 @@ CollectorOpts, PragmaHandler.get()); std::vector Args = { -"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11", -"-include", TestHeaderName, TestFileName}; +"symbol_collector", "-fsyntax-only", "-xc++", +"-std=c++11", "-include", TestHeaderName}; Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end()); +// This allows to override the "-xc++" with something else, i.e. +// -xobjective-c++. +Args.push_back(TestFileName); + tooling::ToolInvocation Invocation( Args, Factory->create(), Files.get(), @@ -163,8 +170,20 @@ TEST_F(SymbolCollectorTest, CollectSymbols) { const std::string Header = R"( class Foo { + Foo() {} + Foo(int a) {} void f(); + friend void f1(); + friend class Friend; + Foo& operator=(const Foo&); + ~Foo(); + class Nested { + void f(); + }; }; +class Friend { +}; + void f1(); inline void f2() {} static const int KInt = 2; @@ -200,23 +219,78 @@ runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAreArray( - {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"), - QName("kStr"), QName("foo"), QName("foo::bar"), - QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"), - QName("foo::bar::v2"), QName("foo::baz")})); + {AllOf(QName("Foo"), ForCodeCompletion(true)), + AllOf(QName("Foo::Foo"), ForCodeCompletion(false)), + AllOf(QName("Foo::Foo"), ForCodeCompletion(false)), + AllOf(QName("Foo::f"), ForCodeCompletion(false)), + AllOf(QName("Foo::~Foo"), ForCodeCompletion(false)), + AllOf(QName("Foo::operator="), ForCodeCompletion(false)), + AllOf(QName("Foo::Nested"), ForCodeCompletion(false)), + AllOf(QName("Foo::Nested::f"), ForCodeCompletion(false)), + + AllOf(QName("Friend"), ForCodeCompletion(true)), +
[PATCH] D44954: [clangd] Add "member" symbols to the index
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE334017: [clangd] Add member symbols to the index (authored by malaperle, committed by ). Changed prior to commit: https://reviews.llvm.org/D44954?vs=149532=149970#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44954 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h clangd/index/SymbolYAML.cpp unittests/clangd/CodeCompleteTests.cpp unittests/clangd/FileIndexTests.cpp unittests/clangd/FindSymbolsTests.cpp unittests/clangd/SymbolCollectorTests.cpp Index: clangd/index/SymbolCollector.h === --- clangd/index/SymbolCollector.h +++ clangd/index/SymbolCollector.h @@ -18,13 +18,18 @@ namespace clang { namespace clangd { -/// \brief Collect top-level symbols from an AST. These are symbols defined -/// immediately inside a namespace or a translation unit scope. For example, -/// symbols in classes or functions are not collected. Note that this only -/// collects symbols that declared in at least one file that is not a main -/// file (i.e. the source file corresponding to a TU). These are symbols that -/// can be imported by other files by including the file where symbols are -/// declared. +/// \brief Collect declarations (symbols) from an AST. +/// It collects most declarations except: +/// - Implicit declarations +/// - Anonymous declarations (anonymous enum/class/struct, etc) +/// - Declarations in anonymous namespaces +/// - Local declarations (in function bodies, blocks, etc) +/// - Declarations in main files +/// - Template specializations +/// - Library-specific private declarations (e.g. private declaration generated +/// by protobuf compiler) +/// +/// See also shouldFilterDecl(). /// /// Clients (e.g. clangd) can use SymbolCollector together with /// index::indexTopLevelDecls to retrieve all symbols when the source file is Index: clangd/index/SymbolCollector.cpp === --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -9,6 +9,7 @@ #include "SymbolCollector.h" #include "../AST.h" +#include "../CodeComplete.h" #include "../CodeCompletionStrings.h" #include "../Logger.h" #include "../SourceCode.h" @@ -149,21 +150,20 @@ if (ND->isInAnonymousNamespace()) return true; - // We only want: - // * symbols in namespaces or translation unit scopes (e.g. no class - // members) - // * enum constants in unscoped enum decl (e.g. "red" in "enum {red};") - auto InTopLevelScope = hasDeclContext( - anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl())); - // Don't index template specializations. + // We want most things but not "local" symbols such as symbols inside + // FunctionDecl, BlockDecl, ObjCMethodDecl and OMPDeclareReductionDecl. + // FIXME: Need a matcher for ExportDecl in order to include symbols declared + // within an export. + auto InNonLocalContext = hasDeclContext(anyOf( + translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(), + enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(), + objcCategoryImplDecl(), objcImplementationDecl())); + // Don't index template specializations and expansions in main files. auto IsSpecialization = anyOf(functionDecl(isExplicitTemplateSpecialization()), cxxRecordDecl(isExplicitTemplateSpecialization()), varDecl(isExplicitTemplateSpecialization())); - if (match(decl(allOf(unless(isExpansionInMainFile()), - anyOf(InTopLevelScope, - hasDeclContext(enumDecl(InTopLevelScope, - unless(isScoped(), + if (match(decl(allOf(unless(isExpansionInMainFile()), InNonLocalContext, unless(IsSpecialization))), *ND, *ASTCtx) .empty()) @@ -377,6 +377,8 @@ Symbol S; S.ID = std::move(ID); std::tie(S.Scope, S.Name) = splitQualifiedName(QName); + + S.IsIndexedForCodeCompletion = isIndexedForCodeCompletion(ND, Ctx); S.SymInfo = index::getSymbolInfo(); std::string FileURI; if (auto DeclLoc = Index: clangd/index/Index.h === --- clangd/index/Index.h +++ clangd/index/Index.h @@ -149,9 +149,11 @@ // The number of translation units that reference this symbol from their main // file. This number is only meaningful if aggregated in an index. unsigned References = 0; - + /// Whether or not this symbol is meant to be used for the code completion. + /// See also isIndexedForCodeCompletion(). + bool IsIndexedForCodeCompletion = false; /// A brief description of the symbol that can be displayed in the completion - ///
[clang-tools-extra] r334017 - [clangd] Add "member" symbols to the index
Author: malaperle Date: Tue Jun 5 07:01:40 2018 New Revision: 334017 URL: http://llvm.org/viewvc/llvm-project?rev=334017=rev Log: [clangd] Add "member" symbols to the index Summary: This adds more symbols to the index: - member variables and functions - enum constants in scoped enums The code completion behavior should remain intact but workspace symbols should now provide much more useful symbols. Other symbols should be considered such as the ones in "main files" (files not being included) but this can be done separately as this introduces its fair share of problems. Signed-off-by: Marc-Andre Laperle Reviewers: ioeric, sammccall Reviewed By: ioeric, sammccall Subscribers: hokein, sammccall, jkorous, klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits Differential Revision: https://reviews.llvm.org/D44954 Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/CodeComplete.h clang-tools-extra/trunk/clangd/index/Index.h clang-tools-extra/trunk/clangd/index/MemIndex.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=334017=334016=334017=diff == --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Jun 5 07:01:40 2018 @@ -25,6 +25,7 @@ #include "Trace.h" #include "URI.h" #include "index/Index.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/LangOptions.h" #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" @@ -949,6 +950,7 @@ private: if (Opts.Limit) Req.MaxCandidateCount = Opts.Limit; Req.Query = Filter->pattern(); +Req.RestrictForCodeCompletion = true; Req.Scopes = getQueryScopes(Recorder->CCContext, Recorder->CCSema->getSourceManager()); log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])", @@ -1089,5 +1091,16 @@ SignatureHelp signatureHelp(PathRef File return Result; } +bool isIndexedForCodeCompletion(const NamedDecl , ASTContext ) { + using namespace clang::ast_matchers; + auto InTopLevelScope = hasDeclContext( + anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl())); + return !match(decl(anyOf(InTopLevelScope, + hasDeclContext( + enumDecl(InTopLevelScope, unless(isScoped()), +ND, ASTCtx) + .empty(); +} + } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/clangd/CodeComplete.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=334017=334016=334017=diff == --- clang-tools-extra/trunk/clangd/CodeComplete.h (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.h Tue Jun 5 07:01:40 2018 @@ -25,6 +25,7 @@ #include "clang/Tooling/CompilationDatabase.h" namespace clang { +class NamedDecl; class PCHContainerOperations; namespace clangd { @@ -82,6 +83,17 @@ SignatureHelp signatureHelp(PathRef File IntrusiveRefCntPtr VFS, std::shared_ptr PCHs); +// For index-based completion, we only consider: +// * symbols in namespaces or translation unit scopes (e.g. no class +// members, no locals) +// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};") +// * primary templates (no specializations) +// For the other cases, we let Clang do the completion because it does not +// need any non-local information and it will be much better at following +// lookup rules. Other symbols still appear in the index for other purposes, +// like workspace/symbols or textDocument/definition, but are not used for code +// completion. +bool isIndexedForCodeCompletion(const NamedDecl , ASTContext ); } // namespace clangd } // namespace clang Modified: clang-tools-extra/trunk/clangd/index/Index.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=334017=334016=334017=diff == --- clang-tools-extra/trunk/clangd/index/Index.h (original) +++ clang-tools-extra/trunk/clangd/index/Index.h Tue Jun 5 07:01:40 2018 @@ -149,9 +149,11 @@ struct Symbol { // The number of translation units that
[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/Quality.cpp:70 +return SymbolRelevanceSignals::ClassScope; + if (D.getLinkageInternal() < ExternalLinkage) +return SymbolRelevanceSignals::FileScope; sammccall wrote: > ioeric wrote: > > I wonder if this comparison is a common practice. Any reason not to use > > `!=`? > The orderedness in linkage is used e.g. in `isExternallyVisible()`. > I prefer `<` because while with this particular threshold value `!=` would be > equivalent, I'm not actually particularly sure it's the right threshold (vs > e.g. `ModuleLinkage`), and with any other threshold `<` would be required. sounds good. this would probably deserve a comment though as it's a little confusing... Comment at: clangd/Quality.cpp:71 + if (D.getLinkageInternal() < ExternalLinkage) +return SymbolRelevanceSignals::FileScope; + return SymbolRelevanceSignals::GlobalScope; sammccall wrote: > ioeric wrote: > > Does `FileScope` indicate that `D` is defined in the current *translation > > unit*? If so, I think this is a good signal (boosting symbols in the same > > TU), but something like `TUScope` might be a bit clearer? > File rather than TU is intended here. `AccessibleScope` is only an > approximate concept (it doesn't need to be precise as it's just a scoring > signal). > The idea is "this is a symbol likely to be only used in the same file that > declares it", and this is true of e.g. variables in anonymous namespaces, > despite the fact that you can technically put them in headers and reference > them in the main file. > > Added a comment to AccessibleScope to clarify this approximate nature. Cool. Thanks for the explanation! Comment at: unittests/clangd/QualityTests.cpp:1 //===-- SourceCodeTests.cpp *- C++ -*-===// // sammccall wrote: > ioeric wrote: > > Could you also add a test case for code completion? > The code completion scoring is tested in SymbolRelevanceSignalsSanity: file > scope is boosted compared to default when the query is code-complete, but not > when it's generic. > > What kind of test do you mean? I was thinking a test case that covers the changes in CodeComplete.cpp e.g. check that Relevance and Quality play well together, and locals/members are boosted? Would that make sense? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer
r.stahl updated this revision to Diff 149959. r.stahl added a comment. - add missing AnalysisConsumer diff - only collect const qualified vardecls in the extdef index and adjust test https://reviews.llvm.org/D46421 Files: include/clang/Basic/DiagnosticCrossTUKinds.td include/clang/CrossTU/CrossTranslationUnit.h include/clang/StaticAnalyzer/Core/AnalyzerOptions.h lib/CrossTU/CrossTranslationUnit.cpp lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/Inputs/ctu-other.cpp test/Analysis/Inputs/externalDefMap.txt test/Analysis/Inputs/externalFnMap.txt test/Analysis/ctu-main.cpp test/Analysis/func-mapping-test.cpp test/CMakeLists.txt test/lit.cfg.py tools/CMakeLists.txt tools/clang-extdef-mapping/CMakeLists.txt tools/clang-extdef-mapping/ClangExtDefMapGen.cpp tools/clang-func-mapping/CMakeLists.txt tools/clang-func-mapping/ClangFnMapGen.cpp tools/scan-build-py/README.md 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/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 @@ -96,7 +96,7 @@ class ClangIsCtuCapableTest(unittest.TestCase): def test_ctu_not_found(self): -is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping') +is_ctu = sut.is_ctu_capable('not-found-clang-extdef-mapping') self.assertFalse(is_ctu) 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 @@ -349,14 +349,14 @@ class MergeCtuMapTest(unittest.TestCase): def test_no_map_gives_empty(self): -pairs = sut.create_global_ctu_function_map([]) +pairs = sut.create_global_ctu_extdef_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) +pairs = sut.create_global_ctu_extdef_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) @@ -366,7 +366,7 @@ 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) +pairs = sut.create_global_ctu_extdef_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) @@ -376,28 +376,28 @@ 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) +pairs = sut.create_global_ctu_extdef_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) +pairs = sut.create_global_ctu_extdef_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): +class ExtdefMapSrcToAstTest(unittest.TestCase): def test_empty_gives_empty(self): -fun_ast_lst = sut.func_map_list_src_to_ast([]) +fun_ast_lst = sut.extdef_map_list_src_to_ast([]) self.assertFalse(fun_ast_lst) def test_sources_to_asts(self): fun_src_lst = ['c:@F@f1#I# ' + os.path.join(os.sep + 'path', 'f1.c'), 'c:@F@f2#I# ' + os.path.join(os.sep + 'path', 'f2.c')] -fun_ast_lst = sut.func_map_list_src_to_ast(fun_src_lst) +fun_ast_lst = sut.extdef_map_list_src_to_ast(fun_src_lst) self.assertTrue('c:@F@f1#I# ' + os.path.join('ast', 'path', 'f1.c.ast') in fun_ast_lst) @@ -408,7 +408,7 @@ def test_spaces_handled(self): fun_src_lst = ['c:@F@f1#I# ' +
[clang-tools-extra] r334014 - [clangd] Test tweaks (consistency, shorter, doc). NFC
Author: sammccall Date: Tue Jun 5 05:22:43 2018 New Revision: 334014 URL: http://llvm.org/viewvc/llvm-project?rev=334014=rev Log: [clangd] Test tweaks (consistency, shorter, doc). NFC Modified: clang-tools-extra/trunk/clangd/Quality.h clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp clang-tools-extra/trunk/unittests/clangd/TestTU.h Modified: clang-tools-extra/trunk/clangd/Quality.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.h?rev=334014=334013=334014=diff == --- clang-tools-extra/trunk/clangd/Quality.h (original) +++ clang-tools-extra/trunk/clangd/Quality.h Tue Jun 5 05:22:43 2018 @@ -61,11 +61,10 @@ llvm::raw_ostream <<(llvm::raw_ /// Attributes of a symbol-query pair that affect how much we like it. struct SymbolRelevanceSignals { - // 0-1 fuzzy-match score for unqualified name. Must be explicitly assigned. + /// 0-1 fuzzy-match score for unqualified name. Must be explicitly assigned. float NameMatch = 1; bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private). - /// Proximity between the best declaration and the query location. [0-1] score - /// where 1 is closest + /// Proximity between best declaration and the query. [0-1], 1 is closest. float ProximityScore = 0; void merge(const CodeCompletionResult ); Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=334014=334013=334014=diff == --- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Tue Jun 5 05:22:43 2018 @@ -60,44 +60,34 @@ TEST(QualityTests, SymbolQualitySignalEx TEST(QualityTests, SymbolRelevanceSignalExtraction) { TestTU Test; Test.HeaderCode = R"cpp( -int test_func_in_header(); -int test_func_in_header_and_cpp(); +int header(); +int header_main(); )cpp"; Test.Code = R"cpp( -int ::test_func_in_header_and_cpp() { -} -int test_func_in_cpp(); +int ::header_main() {} +int main(); [[deprecated]] -int test_deprecated() { return 0; } +int deprecated() { return 0; } )cpp"; auto AST = Test.build(); - SymbolRelevanceSignals Deprecated; - Deprecated.merge(CodeCompletionResult((AST, "test_deprecated"), -/*Priority=*/42, nullptr, false, -/*Accessible=*/false)); - EXPECT_EQ(Deprecated.NameMatch, SymbolRelevanceSignals().NameMatch); - EXPECT_TRUE(Deprecated.Forbidden); - - // Test proximity scores. - SymbolRelevanceSignals FuncInCpp; - FuncInCpp.merge(CodeCompletionResult((AST, "test_func_in_cpp"), - CCP_Declaration)); - /// Decls in the current file should get a proximity score of 1.0. - EXPECT_FLOAT_EQ(FuncInCpp.ProximityScore, 1.0); - - SymbolRelevanceSignals FuncInHeader; - FuncInHeader.merge(CodeCompletionResult((AST, "test_func_in_header"), - CCP_Declaration)); - /// Decls outside current file currently don't get a proximity score boost. - EXPECT_FLOAT_EQ(FuncInHeader.ProximityScore, 0.0); - - SymbolRelevanceSignals FuncInHeaderAndCpp; - FuncInHeaderAndCpp.merge(CodeCompletionResult( - (AST, "test_func_in_header_and_cpp"), CCP_Declaration)); - /// Decls in both header **and** the main file get the same boost. - EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.ProximityScore, 1.0); + SymbolRelevanceSignals Relevance; + Relevance.merge(CodeCompletionResult((AST, "deprecated"), + /*Priority=*/42, nullptr, false, + /*Accessible=*/false)); + EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals().NameMatch); + EXPECT_TRUE(Relevance.Forbidden); + + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "main"), 42)); + EXPECT_FLOAT_EQ(Relevance.ProximityScore, 1.0) << "Decl in current file"; + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "header"), 42)); + EXPECT_FLOAT_EQ(Relevance.ProximityScore, 0.0) << "Decl from header"; + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "header_main"), 42)); + EXPECT_FLOAT_EQ(Relevance.ProximityScore, 1.0) << "Current file and header"; } // Do the signals move the scores in the direction we expect? @@ -136,7 +126,7 @@ TEST(QualityTests, SymbolRelevanceSignal SymbolRelevanceSignals WithProximity; WithProximity.ProximityScore = 0.2f; - EXPECT_LT(Default.evaluate(), WithProximity.evaluate()); + EXPECT_GT(WithProximity.evaluate(), Default.evaluate()); } TEST(QualityTests, SortText) { Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.h URL:
[PATCH] D45722: [X86] Lowering SAD (sum of absolute differences) intrinsics to native IR (clang side)
mike.dvoretsky abandoned this revision. mike.dvoretsky added a comment. Closing this due to failure of https://reviews.llvm.org/D45723. https://reviews.llvm.org/D45722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r334013 - Silence a "truncation from double to float" diagnostic in MSVC; NFC.
Author: aaronballman Date: Tue Jun 5 05:14:47 2018 New Revision: 334013 URL: http://llvm.org/viewvc/llvm-project?rev=334013=rev Log: Silence a "truncation from double to float" diagnostic in MSVC; NFC. Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=334013=334012=334013=diff == --- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Tue Jun 5 05:14:47 2018 @@ -135,7 +135,7 @@ TEST(QualityTests, SymbolRelevanceSignal EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate()); SymbolRelevanceSignals WithProximity; - WithProximity.ProximityScore = 0.2; + WithProximity.ProximityScore = 0.2f; EXPECT_LT(Default.evaluate(), WithProximity.evaluate()); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45720: [X86] Lowering PACK*S (pack with saturation) intrinsics to native IR (clang side)
mike.dvoretsky abandoned this revision. mike.dvoretsky added a comment. Closing this due to failure of https://reviews.llvm.org/D45721. https://reviews.llvm.org/D45720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)
sammccall added inline comments. Comment at: clangd/Quality.cpp:70 +return SymbolRelevanceSignals::ClassScope; + if (D.getLinkageInternal() < ExternalLinkage) +return SymbolRelevanceSignals::FileScope; ioeric wrote: > I wonder if this comparison is a common practice. Any reason not to use `!=`? The orderedness in linkage is used e.g. in `isExternallyVisible()`. I prefer `<` because while with this particular threshold value `!=` would be equivalent, I'm not actually particularly sure it's the right threshold (vs e.g. `ModuleLinkage`), and with any other threshold `<` would be required. Comment at: clangd/Quality.cpp:71 + if (D.getLinkageInternal() < ExternalLinkage) +return SymbolRelevanceSignals::FileScope; + return SymbolRelevanceSignals::GlobalScope; ioeric wrote: > Does `FileScope` indicate that `D` is defined in the current *translation > unit*? If so, I think this is a good signal (boosting symbols in the same > TU), but something like `TUScope` might be a bit clearer? File rather than TU is intended here. `AccessibleScope` is only an approximate concept (it doesn't need to be precise as it's just a scoring signal). The idea is "this is a symbol likely to be only used in the same file that declares it", and this is true of e.g. variables in anonymous namespaces, despite the fact that you can technically put them in headers and reference them in the main file. Added a comment to AccessibleScope to clarify this approximate nature. Comment at: clangd/Quality.cpp:86 + if (SemaCCResult.Kind == CodeCompletionResult::RK_Declaration) +Scope = std::min(Scope, ComputeScope(*SemaCCResult.Declaration)); } ioeric wrote: > nit: does this assume that "smaller" scopes are better? If so, it might make > sense to document. There's no connection with "better" here, just that items are assumed to have maximum scope (global) until proven otherwise. There's no intent here (or with other signals) for merging conflicting results from different sources to do anything clever - it's fine to choose one arbitrarily. Comment at: unittests/clangd/QualityTests.cpp:1 //===-- SourceCodeTests.cpp *- C++ -*-===// // ioeric wrote: > Could you also add a test case for code completion? The code completion scoring is tested in SymbolRelevanceSignalsSanity: file scope is boosted compared to default when the query is code-complete, but not when it's generic. What kind of test do you mean? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)
sammccall updated this revision to Diff 149951. sammccall marked 3 inline comments as done. sammccall added a comment. Address review comments and sync. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47762 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp unittests/clangd/TestTU.cpp unittests/clangd/TestTU.h Index: unittests/clangd/TestTU.h === --- unittests/clangd/TestTU.h +++ unittests/clangd/TestTU.h @@ -53,6 +53,8 @@ const Symbol (const SymbolSlab &, llvm::StringRef QName); // Look up an AST symbol by qualified name, which must be unique and top-level. const NamedDecl (ParsedAST , llvm::StringRef QName); +// Look up a main-file AST symbol by unqualified name, which must be unique. +const NamedDecl (ParsedAST , llvm::StringRef Name); } // namespace clangd } // namespace clang Index: unittests/clangd/TestTU.cpp === --- unittests/clangd/TestTU.cpp +++ unittests/clangd/TestTU.cpp @@ -10,6 +10,7 @@ #include "TestFS.h" #include "index/FileIndex.h" #include "index/MemIndex.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Frontend/Utils.h" @@ -49,7 +50,6 @@ return MemIndex::build(headerSymbols()); } -// Look up a symbol by qualified name, which must be unique. const Symbol (const SymbolSlab , llvm::StringRef QName) { const Symbol *Result = nullptr; for (const Symbol : Slab) { @@ -92,5 +92,26 @@ return LookupDecl(*Scope, Components.back()); } +const NamedDecl (ParsedAST , llvm::StringRef Name) { + struct Visitor : RecursiveASTVisitor { +llvm::StringRef Name; +llvm::SmallVector Decls; +bool VisitNamedDecl(const NamedDecl *ND) { + if (auto *ID = ND->getIdentifier()) +if (ID->getName() == Name) + Decls.push_back(ND); + return true; +} + } Visitor; + Visitor.Name = Name; + for (Decl *D : AST.getLocalTopLevelDecls()) +Visitor.TraverseDecl(D); + if (Visitor.Decls.size() != 1) { +ADD_FAILURE() << Visitor.Decls.size() << " symbols named " << Name; +assert(Visitor.Decls.size() == 1); + } + return *Visitor.Decls.front(); +} + } // namespace clangd } // namespace clang Index: unittests/clangd/QualityTests.cpp === --- unittests/clangd/QualityTests.cpp +++ unittests/clangd/QualityTests.cpp @@ -70,6 +70,8 @@ [[deprecated]] int test_deprecated() { return 0; } + +namespace { struct X { void y() { int z; } }; } )cpp"; auto AST = Test.build(); @@ -79,6 +81,7 @@ /*Accessible=*/false)); EXPECT_EQ(Deprecated.NameMatch, SymbolRelevanceSignals().NameMatch); EXPECT_TRUE(Deprecated.Forbidden); + EXPECT_EQ(Deprecated.Scope, SymbolRelevanceSignals::GlobalScope); // Test proximity scores. SymbolRelevanceSignals FuncInCpp; @@ -98,6 +101,16 @@ (AST, "test_func_in_header_and_cpp"), CCP_Declaration)); /// Decls in both header **and** the main file get the same boost. EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.ProximityScore, 1.0); + + SymbolRelevanceSignals Relevance; + Relevance.merge(CodeCompletionResult((AST, "X"), 42)); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope); + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "y"), 42)); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::ClassScope); + Relevance = {}; + Relevance.merge(CodeCompletionResult((AST, "z"), 42)); + EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FunctionScope); } // Do the signals move the scores in the direction we expect? @@ -137,6 +150,12 @@ SymbolRelevanceSignals WithProximity; WithProximity.ProximityScore = 0.2; EXPECT_LT(Default.evaluate(), WithProximity.evaluate()); + + SymbolRelevanceSignals Scoped; + Scoped.Scope = SymbolRelevanceSignals::FileScope; + EXPECT_EQ(Scoped.evaluate(), Default.evaluate()); + Scoped.Query = SymbolRelevanceSignals::CodeComplete; + EXPECT_GT(Scoped.evaluate(), Default.evaluate()); } TEST(QualityTests, SortText) { Index: clangd/Quality.h === --- clangd/Quality.h +++ clangd/Quality.h @@ -68,7 +68,21 @@ /// where 1 is closest float ProximityScore = 0; + // An approximate measure of where we expect the symbol to be used. + enum AccessibleScope { +FunctionScope, +ClassScope, +FileScope, +GlobalScope, + } Scope = GlobalScope; + + enum QueryType { +CodeComplete, +Generic, + } Query = Generic; + void merge(const CodeCompletionResult ); + void merge(const Symbol ); // Condense these signals down to a single number, higher is better. float evaluate() const; Index:
[PATCH] D46602: [clang-tidy] Store checks profiling info as JSON files
lebedev.ri added a comment. In https://reviews.llvm.org/D46602#1122194, @aaron.ballman wrote: > LGTM! Thank you for the review. Waiting on @alexfh ... Repository: rL LLVM https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as JSON files
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tidy/ClangTidyProfiling.cpp:47 +void ClangTidyProfiling::printAsJSON(llvm::raw_ostream ) { + OS << "{\n"; lebedev.ri wrote: > aaron.ballman wrote: > > I'm not keen that we call this `printAsJSON()` when the docs talk about > > writing out YAML. While all JSON is valid YAML, that feels like trivia we > > shouldn't encode in the function name. What do you think to renaming to > > `printAsYAML()` instead? > But the actual printer is called `TG->printJSONValues()`.. > Let's go the other way around, and canonicalize to JSON. Even better! Repository: rL LLVM https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r333994 - [clang-tidy] fix broken test (no compile command) from r331763
Ah, I've been there. No worries! On Tue, Jun 5, 2018 at 1:08 PM Roman Lebedev wrote: > Ah, hm, yes, that was a further local problem in > https://reviews.llvm.org/D46602 > And i didn't double-check on master initially, because that causes all > the checks to be rebuilt :/ > > So thanks for the fix! > > Roman. > > > On Tue, Jun 5, 2018 at 1:40 PM, Sam McCall wrote: > > Hi Roman, > > > > Definitely something different in our configs, but it wasn't just me - a > > couple of colleagues have mentioned those tests as being broken for a > while! > > > > The problem is the lack of compilation database, so I can imagine it can > > depend on filesystem layout, e.g. whether you have a separate > build/source > > directory. (Potentially the CDB from llvm itself may be picked up or not) > > > > I can't tell from your lit output whether clang-tidy or FileCheck failed, > > what does this command return for you? > > > > bin/clang-tidy -enable-check-profile > -checks='-*,readability-function-size' > > ../src/llvm/tools/clang/tools/extra/test/clang-tid > > y/clang-tidy-enable-check-profile-one-tu.cpp -- > > > > (adjust path as needed) > > > > On Tue, Jun 5, 2018 at 12:30 PM Roman Lebedev > wrote: > >> > >> This is strange. > >> > >> First, i'm pretty sure the test worked for me before. > >> Second, this commit actually *breaks* those two tests for me: > >> > >> $ ninja check-clang-tools > >> [0/1] Running the Clang extra tools' regression tests > >> FAIL: Clang Tools :: > >> clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp (123 of 867) > >> TEST 'Clang Tools :: > >> clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp' FAILED > >> > >> Script: > >> -- > >> : 'RUN: at line 1'; clang-tidy -enable-check-profile > >> -checks='-*,readability-function-size' > >> > >> > /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> -- 2>&1 | FileCheck --match-full-lines > >> -implicit-check-not='{{warning:|error:}}' /build/clang > >> -tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> -- > >> Exit Code: 2 > >> > >> Command Output (stderr): > >> -- > >> FileCheck error: '-' is empty. > >> FileCheck command line: FileCheck --match-full-lines > >> -implicit-check-not={{warning:|error:}} > >> > >> > /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> > >> -- > >> > >> > >> FAIL: Clang Tools :: > >> clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp (125 of 867) > >> TEST 'Clang Tools :: > >> clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp' FAILED > >> > >> Script: > >> -- > >> : 'RUN: at line 1'; clang-tidy -enable-check-profile > >> -checks='-*,readability-function-size' > >> > >> > /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp > >> > >> > /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp > >> -- 2>&1 | > >> FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' > >> > >> > /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp > >> -- > >> Exit Code: 2 > >> > >> Command Output (stderr): > >> -- > >> FileCheck error: '-' is empty. > >> FileCheck command line: FileCheck --match-full-lines > >> -implicit-check-not={{warning:|error:}} > >> > >> > /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp > >> > >> -- > >> > >> Roman. > >> > >> On Tue, Jun 5, 2018 at 12:42 PM, Sam McCall via cfe-commits > >> wrote: > >> > Author: sammccall > >> > Date: Tue Jun 5 02:42:06 2018 > >> > New Revision: 333994 > >> > > >> > URL: http://llvm.org/viewvc/llvm-project?rev=333994=rev > >> > Log: > >> > [clang-tidy] fix broken test (no compile command) from r331763 > >> > > >> > Modified: > >> > > >> > > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> > > >> > > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp > >> > > >> > Modified: > >> > > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> > URL: > >> > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp?rev=333994=333993=333994=diff > >> > > >> > > == > >> > --- > >> > > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> > (original) > >> > +++ > >> > > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp > >> > Tue Jun 5 02:42:06 2018 > >> > @@ -1,4 +1,4 @@ > >> > -// RUN: clang-tidy -enable-check-profile > >> > -checks='-*,readability-function-size' %s 2>&1 | FileCheck > >> > --match-full-lines -implicit-check-not='{{warning:|error:}}' %s > >> > +// RUN: clang-tidy -enable-check-profile > >> >
[PATCH] D46602: [clang-tidy] Store checks profiling info as JSON files
lebedev.ri added inline comments. Comment at: clang-tidy/ClangTidyProfiling.cpp:47 +void ClangTidyProfiling::printAsJSON(llvm::raw_ostream ) { + OS << "{\n"; aaron.ballman wrote: > I'm not keen that we call this `printAsJSON()` when the docs talk about > writing out YAML. While all JSON is valid YAML, that feels like trivia we > shouldn't encode in the function name. What do you think to renaming to > `printAsYAML()` instead? But the actual printer is called `TG->printJSONValues()`.. Let's go the other way around, and canonicalize to JSON. Repository: rL LLVM https://reviews.llvm.org/D46602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46602: [clang-tidy] Store checks profiling info as JSON files
lebedev.ri updated this revision to Diff 149933. lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. Rebase ontop of https://reviews.llvm.org/rL333994, address review notes, canonicalize to `JSON`. Repository: rL LLVM https://reviews.llvm.org/D46602 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidy.h clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tidy/ClangTidyDiagnosticConsumer.h clang-tidy/ClangTidyProfiling.cpp clang-tidy/ClangTidyProfiling.h clang-tidy/tool/ClangTidyMain.cpp docs/ReleaseNotes.rst docs/clang-tidy/index.rst test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp Index: test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp === --- /dev/null +++ test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp @@ -0,0 +1,37 @@ +// RUN: rm -rf %T/out +// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' -store-check-profile=%T/out %s -- 2>&1 | not FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK-CONSOLE %s +// RUN: cat %T/out/*-clang-tidy-store-check-profile-one-tu.cpp.json | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK-FILE %s +// RUN: rm -rf %T/out +// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' -store-check-profile=%T/out %s -- 2>&1 +// RUN: cat %T/out/*-clang-tidy-store-check-profile-one-tu.cpp.json | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK-FILE %s + +// CHECK-CONSOLE-NOT: ===-=== +// CHECK-CONSOLE-NOT: {{.*}} --- Name --- +// CHECK-CONSOLE-NOT: {{.*}} readability-function-size +// CHECK-CONSOLE-NOT: {{.*}} Total +// CHECK-CONSOLE-NOT: ===-=== + +// CHECK-FILE: { +// CHECK-FILE-NEXT:"file": "{{.*}}clang-tidy-store-check-profile-one-tu.cpp", +// CHECK-FILE-NEXT:"timestamp": "{{[0-9]+}}-{{[0-9]+}}-{{[0-9]+}} {{[0-9]+}}:{{[0-9]+}}:{{[0-9]+}}.{{[0-9]+}}", +// CHECK-FILE-NEXT:"profile": { +// CHECK-FILE-NEXT: "time.clang-tidy.readability-function-size.wall": {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}}, +// CHECK-FILE-NEXT: "time.clang-tidy.readability-function-size.user": {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}}, +// CHECK-FILE-NEXT: "time.clang-tidy.readability-function-size.sys": {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}} +// CHECK-FILE-NEXT: } +// CHECK-FILE-NEXT: } + +// CHECK-FILE-NOT: { +// CHECK-FILE-NOT: "file": {{.*}}clang-tidy-store-check-profile-one-tu.cpp{{.*}}, +// CHECK-FILE-NOT: "timestamp": "{{[0-9]+}}-{{[0-9]+}}-{{[0-9]+}} {{[0-9]+}}:{{[0-9]+}}:{{[0-9]+}}.{{[0-9]+}}", +// CHECK-FILE-NOT: "profile": { +// CHECK-FILE-NOT: "time.clang-tidy.readability-function-size.wall": {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}}, +// CHECK-FILE-NOT: "time.clang-tidy.readability-function-size.user": {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}}, +// CHECK-FILE-NOT: "time.clang-tidy.readability-function-size.sys": {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}} +// CHECK-FILE-NOT: } +// CHECK-FILE-NOT: } + +class A { + A() {} + ~A() {} +}; Index: test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp === --- test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp +++ test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp @@ -1,22 +1,31 @@ // RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' %s %s -- 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' %s // CHECK: ===-=== -// CHECK-NEXT: {{.*}} --- Name --- +// CHECK-NEXT: clang-tidy checks profiling +// CHECK-NEXT: ===-=== +// CHECK-NEXT: Total Execution Time: {{.*}} seconds ({{.*}} wall clock) + +// CHECK: {{.*}} --- Name --- // CHECK-NEXT: {{.*}} readability-function-size // CHECK-NEXT: {{.*}} Total -// CHECK-NEXT: ===-=== // CHECK: ===-=== -// CHECK-NEXT: {{.*}} --- Name --- +// CHECK-NEXT: clang-tidy checks profiling +// CHECK-NEXT: ===-=== +// CHECK-NEXT: Total Execution Time: {{.*}} seconds ({{.*}} wall clock) + +// CHECK: {{.*}} --- Name --- // CHECK-NEXT: {{.*}} readability-function-size // CHECK-NEXT: {{.*}} Total -// CHECK-NEXT:
Re: [clang-tools-extra] r333994 - [clang-tidy] fix broken test (no compile command) from r331763
Ah, hm, yes, that was a further local problem in https://reviews.llvm.org/D46602 And i didn't double-check on master initially, because that causes all the checks to be rebuilt :/ So thanks for the fix! Roman. On Tue, Jun 5, 2018 at 1:40 PM, Sam McCall wrote: > Hi Roman, > > Definitely something different in our configs, but it wasn't just me - a > couple of colleagues have mentioned those tests as being broken for a while! > > The problem is the lack of compilation database, so I can imagine it can > depend on filesystem layout, e.g. whether you have a separate build/source > directory. (Potentially the CDB from llvm itself may be picked up or not) > > I can't tell from your lit output whether clang-tidy or FileCheck failed, > what does this command return for you? > > bin/clang-tidy -enable-check-profile -checks='-*,readability-function-size' > ../src/llvm/tools/clang/tools/extra/test/clang-tid > y/clang-tidy-enable-check-profile-one-tu.cpp -- > > (adjust path as needed) > > On Tue, Jun 5, 2018 at 12:30 PM Roman Lebedev wrote: >> >> This is strange. >> >> First, i'm pretty sure the test worked for me before. >> Second, this commit actually *breaks* those two tests for me: >> >> $ ninja check-clang-tools >> [0/1] Running the Clang extra tools' regression tests >> FAIL: Clang Tools :: >> clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp (123 of 867) >> TEST 'Clang Tools :: >> clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp' FAILED >> >> Script: >> -- >> : 'RUN: at line 1'; clang-tidy -enable-check-profile >> -checks='-*,readability-function-size' >> >> /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> -- 2>&1 | FileCheck --match-full-lines >> -implicit-check-not='{{warning:|error:}}' /build/clang >> -tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> -- >> Exit Code: 2 >> >> Command Output (stderr): >> -- >> FileCheck error: '-' is empty. >> FileCheck command line: FileCheck --match-full-lines >> -implicit-check-not={{warning:|error:}} >> >> /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> >> -- >> >> >> FAIL: Clang Tools :: >> clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp (125 of 867) >> TEST 'Clang Tools :: >> clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp' FAILED >> >> Script: >> -- >> : 'RUN: at line 1'; clang-tidy -enable-check-profile >> -checks='-*,readability-function-size' >> >> /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp >> >> /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp >> -- 2>&1 | >> FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' >> >> /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp >> -- >> Exit Code: 2 >> >> Command Output (stderr): >> -- >> FileCheck error: '-' is empty. >> FileCheck command line: FileCheck --match-full-lines >> -implicit-check-not={{warning:|error:}} >> >> /build/clang-tools-extra/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp >> >> -- >> >> Roman. >> >> On Tue, Jun 5, 2018 at 12:42 PM, Sam McCall via cfe-commits >> wrote: >> > Author: sammccall >> > Date: Tue Jun 5 02:42:06 2018 >> > New Revision: 333994 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=333994=rev >> > Log: >> > [clang-tidy] fix broken test (no compile command) from r331763 >> > >> > Modified: >> > >> > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> > >> > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp >> > >> > Modified: >> > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> > URL: >> > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp?rev=333994=333993=333994=diff >> > >> > == >> > --- >> > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> > (original) >> > +++ >> > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp >> > Tue Jun 5 02:42:06 2018 >> > @@ -1,4 +1,4 @@ >> > -// RUN: clang-tidy -enable-check-profile >> > -checks='-*,readability-function-size' %s 2>&1 | FileCheck >> > --match-full-lines -implicit-check-not='{{warning:|error:}}' %s >> > +// RUN: clang-tidy -enable-check-profile >> > -checks='-*,readability-function-size' %s -- 2>&1 | FileCheck >> > --match-full-lines -implicit-check-not='{{warning:|error:}}' %s >> > >> > // CHECK: >> > ===-=== >> > // CHECK-NEXT: {{.*}} --- Name --- >> > >> > Modified: >> > clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp >> > URL: >> >
[PATCH] D47724: [X86] Add back _mask, _maskz, and _mask3 builtins for some 512-bit fmadd/fmsub/fmaddsub/fmsubadd builtins.
tkrupa added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2388 + case X86::BI__builtin_ia32_vfmaddsubps512_maskz: + case X86::BI__builtin_ia32_vfmaddsubps512_mask3: ArgNum = 4; vfmsubps512_mask3, vfmsubpd512_mask3, vfmsubaddps512_mask3 and vfmsubaddpd512_mask3 are missing. Repository: rC Clang https://reviews.llvm.org/D47724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits