[libcxx] r248987 - Suppress array initialization warnings in std::experimental::apply tests
Author: ericwf Date: Thu Oct 1 02:05:38 2015 New Revision: 248987 URL: http://llvm.org/viewvc/llvm-project?rev=248987&view=rev Log: Suppress array initialization warnings in std::experimental::apply tests Added: libcxx/trunk/test/support/disable_missing_braces_warning.h Removed: libcxx/trunk/test/std/containers/sequences/array/suppress_array_warnings.h Modified: libcxx/trunk/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.data/data.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.data/data_const.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.fill/fill.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.size/size.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.special/swap.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.swap/swap.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.tuple/get.fail.cpp libcxx/trunk/test/std/containers/sequences/array/array.tuple/get.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.tuple/get_const.pass.cpp libcxx/trunk/test/std/containers/sequences/array/array.tuple/get_rv.pass.cpp libcxx/trunk/test/std/containers/sequences/array/at.pass.cpp libcxx/trunk/test/std/containers/sequences/array/begin.pass.cpp libcxx/trunk/test/std/containers/sequences/array/front_back.pass.cpp libcxx/trunk/test/std/containers/sequences/array/indexing.pass.cpp libcxx/trunk/test/std/experimental/utilities/tuple/tuple.apply/arg_type.pass.cpp libcxx/trunk/test/std/experimental/utilities/tuple/tuple.apply/extended_types.pass.cpp libcxx/trunk/test/std/experimental/utilities/tuple/tuple.apply/types.pass.cpp Modified: libcxx/trunk/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp?rev=248987&r1=248986&r2=248987&view=diff == --- libcxx/trunk/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp (original) +++ libcxx/trunk/test/std/containers/sequences/array/array.cons/initializer_list.pass.cpp Thu Oct 1 02:05:38 2015 @@ -14,7 +14,9 @@ #include #include -#include "../suppress_array_warnings.h" +// std::array is explicitly allowed to be initialized with A a = { init-list };. +// Disable the missing braces warning for this reason. +#include "disable_missing_braces_warning.h" int main() { Modified: libcxx/trunk/test/std/containers/sequences/array/array.data/data.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.data/data.pass.cpp?rev=248987&r1=248986&r2=248987&view=diff == --- libcxx/trunk/test/std/containers/sequences/array/array.data/data.pass.cpp (original) +++ libcxx/trunk/test/std/containers/sequences/array/array.data/data.pass.cpp Thu Oct 1 02:05:38 2015 @@ -14,7 +14,9 @@ #include #include -#include "../suppress_array_warnings.h" +// std::array is explicitly allowed to be initialized with A a = { init-list };. +// Disable the missing braces warning for this reason. +#include "disable_missing_braces_warning.h" int main() { Modified: libcxx/trunk/test/std/containers/sequences/array/array.data/data_const.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.data/data_const.pass.cpp?rev=248987&r1=248986&r2=248987&view=diff == --- libcxx/trunk/test/std/containers/sequences/array/array.data/data_const.pass.cpp (original) +++ libcxx/trunk/test/std/containers/sequences/array/array.data/data_const.pass.cpp Thu Oct 1 02:05:38 2015 @@ -14,7 +14,9 @@ #include #include -#include "../suppress_array_warnings.h" +// std::array is explicitly allowed to be initialized with A a = { init-list };. +// Disable the missing braces warning for this reason. +#include "disable_missing_braces_warning.h" int main() { Modified: libcxx/trunk/test/std/containers/sequences/array/array.fill/fill.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.fill/fill.pass.cpp?rev=248987&r1=248986&r2=248987&view=diff == --- libcxx/trunk/test/std/containers/sequences/array/array.fill/fill.pass.cpp (original) +++ libcxx/trunk/test/std/containers/sequences/array/array.fill/fill.pass.cpp Thu Oct 1 02:05:38 2015 @@ -14,7 +14,9 @@ #include #include -#include "../suppress_array_warnings.h" +// std::array is explicitly allowed to be initialized with A a = { init-list };. +// Disable the missing braces warni
[libcxx] r248988 - Fix initialzation order in dynarray
Author: ericwf Date: Thu Oct 1 02:29:38 2015 New Revision: 248988 URL: http://llvm.org/viewvc/llvm-project?rev=248988&view=rev Log: Fix initialzation order in dynarray Modified: libcxx/trunk/include/experimental/dynarray Modified: libcxx/trunk/include/experimental/dynarray URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/dynarray?rev=248988&r1=248987&r2=248988&view=diff == --- libcxx/trunk/include/experimental/dynarray (original) +++ libcxx/trunk/include/experimental/dynarray Thu Oct 1 02:29:38 2015 @@ -137,7 +137,7 @@ public: private: size_t __size_; value_type *__base_; -_LIBCPP_ALWAYS_INLINE dynarray () noexcept : __base_(nullptr), __size_(0) {} +_LIBCPP_ALWAYS_INLINE dynarray () noexcept : __size_(0), __base_(nullptr) {} static inline _LIBCPP_INLINE_VISIBILITY value_type* __allocate ( size_t count ) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
xazax.hun added a subscriber: xazax.hun. xazax.hun added a comment. If you check AvoidCStyleCastsCheck.cpp, it also suggest what kind of cast should you use to replace a C style cast. Probably, in some cases, the developers might be able to change the reinterpret_cast to something more type safe. It would be nice to make a suggestion in those cases. http://reviews.llvm.org/D13313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13331: [libcxx] Use newest supported language dialect when running the test suite.
EricWF created this revision. EricWF added reviewers: mclow.lists, danalbert, jroelofs. EricWF added a subscriber: cfe-commits. Currently the test suite defaults to C++11 mode if no standard version is supplied to LIT using `--param=std=c++XX`. This patch changes that behavior so that the newest possible dialect is selected instead. I have already patched the C++11 bot to explicitly specify `--param=std=c++11`. I'm just putting this up for review to see if anybody objects to this idea. http://reviews.llvm.org/D13331 Files: test/libcxx/test/config.py Index: test/libcxx/test/config.py === --- test/libcxx/test/config.py +++ test/libcxx/test/config.py @@ -344,7 +344,20 @@ # Try and get the std version from the command line. Fall back to # default given in lit.site.cfg is not present. If default is not # present then force c++11. -std = self.get_lit_conf('std', 'c++11') +std = self.get_lit_conf('std') +if not std: +# Choose the newest possible language dialect if none is given. +possible_stds = ['c++1z', 'c++14', 'c++11', 'c++03'] +for s in possible_stds: +if self.cxx.hasCompileFlag('-std=%s' % s): +std = s +self.lit_config.note( +'inferred language dialect as: %s' % std) +break +if not std: +self.lit_config.fatal( +'Failed to infer a supported language dialect from one of %r' +% possible_stds) self.cxx.compile_flags += ['-std={0}'.format(std)] self.config.available_features.add(std) # Configure include paths Index: test/libcxx/test/config.py === --- test/libcxx/test/config.py +++ test/libcxx/test/config.py @@ -344,7 +344,20 @@ # Try and get the std version from the command line. Fall back to # default given in lit.site.cfg is not present. If default is not # present then force c++11. -std = self.get_lit_conf('std', 'c++11') +std = self.get_lit_conf('std') +if not std: +# Choose the newest possible language dialect if none is given. +possible_stds = ['c++1z', 'c++14', 'c++11', 'c++03'] +for s in possible_stds: +if self.cxx.hasCompileFlag('-std=%s' % s): +std = s +self.lit_config.note( +'inferred language dialect as: %s' % std) +break +if not std: +self.lit_config.fatal( +'Failed to infer a supported language dialect from one of %r' +% possible_stds) self.cxx.compile_flags += ['-std={0}'.format(std)] self.config.available_features.add(std) # Configure include paths ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r248989 - Manually suppress -Wnonnull when it occurs in an unevaluated context
Author: ericwf Date: Thu Oct 1 02:41:07 2015 New Revision: 248989 URL: http://llvm.org/viewvc/llvm-project?rev=248989&view=rev Log: Manually suppress -Wnonnull when it occurs in an unevaluated context Modified: libcxx/trunk/test/std/depr/depr.c.headers/stdlib_h.pass.cpp libcxx/trunk/test/std/language.support/support.runtime/cstdlib.pass.cpp Modified: libcxx/trunk/test/std/depr/depr.c.headers/stdlib_h.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/depr/depr.c.headers/stdlib_h.pass.cpp?rev=248989&r1=248988&r2=248989&view=diff == --- libcxx/trunk/test/std/depr/depr.c.headers/stdlib_h.pass.cpp (original) +++ libcxx/trunk/test/std/depr/depr.c.headers/stdlib_h.pass.cpp Thu Oct 1 02:41:07 2015 @@ -12,6 +12,12 @@ #include #include +// As of 1/10/2015 clang emits a -Wnonnull warnings even if the warning occurs +// in an unevaluated context. For this reason we manually suppress the warning. +#if defined(__clang__) +#pragma clang diagnostic ignored "-Wnonnull" +#endif + #ifndef EXIT_FAILURE #error EXIT_FAILURE not defined #endif Modified: libcxx/trunk/test/std/language.support/support.runtime/cstdlib.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/language.support/support.runtime/cstdlib.pass.cpp?rev=248989&r1=248988&r2=248989&view=diff == --- libcxx/trunk/test/std/language.support/support.runtime/cstdlib.pass.cpp (original) +++ libcxx/trunk/test/std/language.support/support.runtime/cstdlib.pass.cpp Thu Oct 1 02:41:07 2015 @@ -13,6 +13,12 @@ #include #include +// As of 1/10/2015 clang emits a -Wnonnull warnings even if the warning occurs +// in an unevaluated context. For this reason we manually suppress the warning. +#if defined(__clang__) +#pragma clang diagnostic ignored "-Wnonnull" +#endif + #ifndef EXIT_FAILURE #error EXIT_FAILURE not defined #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12993: [analyzer] Add TK_EntireMemSpace invalidation trait.
seaneveson added a comment. Could you update this patch to the latest trunk please? There's a conflict with http://reviews.llvm.org/rL248516. Thanks. http://reviews.llvm.org/D12993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13318: Add a utility function to add target information to a command line
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: include/clang/Tooling/Tooling.h:437 @@ +436,3 @@ +/// +/// \note This will not set \c CommandLine[0] to \c InvokedAs. +void addTargetAndModeForProgramName(std::vector &CommandLine, Add that for tools, CommandLine[0] will need to be a tool path relatively to which the builtin headers can be found. http://reviews.llvm.org/D13318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13332: [libcxx] Remove uses of std::unique_ptr from test to prevent warning.
EricWF created this revision. EricWF added a reviewer: mclow.lists. EricWF added subscribers: cfe-commits, mclow.lists. constructing a std::unique_ptr will always cause a warning because the deleter (`std::default_delete`) will call `delete ptr` instead of `delete [] ptr`. This is a bug in the standard, not libc++. I don't see a reason why removing the test cases will decrease coverage because std::unique_ptr and std::unique_ptr select the same primary template. However since @mclow.lists wrote this test I want him to approve this patch. http://reviews.llvm.org/D13332 Files: test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp Index: test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp === --- test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp +++ test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp @@ -21,18 +21,10 @@ std::tuple> up; std::tuple> sp; std::tuple> wp; -// std::tuple> ap; } { std::tuple> up; std::tuple> sp; std::tuple> wp; -// std::tuple> ap; } -{ -std::tuple> up; -std::tuple> sp; -std::tuple> wp; -// std::tuple> ap; -} -} \ No newline at end of file +} Index: test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp === --- test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp +++ test/std/utilities/tuple/tuple.general/tuple.smartptr.pass.cpp @@ -21,18 +21,10 @@ std::tuple> up; std::tuple> sp; std::tuple> wp; -// std::tuple> ap; } { std::tuple> up; std::tuple> sp; std::tuple> wp; -// std::tuple> ap; } -{ -std::tuple> up; -std::tuple> sp; -std::tuple> wp; -// std::tuple> ap; -} -} \ No newline at end of file +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13318: Add a utility function to add target information to a command line
klimek added a comment. In http://reviews.llvm.org/D13318#257091, @echristo wrote: > This seems a little odd, could you explain in a bit more detail? Me not > understanding I imagine. :) Seems to be explained well in the comments? If the compilation database contains: i686-linux-android-g++ Then might only be correctly parseable with the target implied; usually the driver figures that out from argv[0], but for tools, argv[0] is the tool name. http://reviews.llvm.org/D13318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r248993 - Attempt to prevent flaky thread.mutex tests by once again increasing timing tolerances
Author: ericwf Date: Thu Oct 1 03:34:37 2015 New Revision: 248993 URL: http://llvm.org/viewvc/llvm-project?rev=248993&view=rev Log: Attempt to prevent flaky thread.mutex tests by once again increasing timing tolerances Modified: libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_duration.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_time_point.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/lock_shared.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_for.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_for.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_until.pass.cpp libcxx/trunk/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_until.pass.cpp libcxx/trunk/test/support/test_macros.h Modified: libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp?rev=248993&r1=248992&r2=248993&view=diff == --- libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp (original) +++ libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp Thu Oct 1 03:34:37 2015 @@ -35,10 +35,10 @@ ms WaitTime = ms(250); // Thread sanitizer causes more overhead and will sometimes cause this test // to fail. To prevent this we give Thread sanitizer more time to complete the // test. -#if !TEST_HAS_FEATURE(thread_sanitizer) +#if !defined(TEST_HAS_SANITIZERS) ms Tolerance = ms(50); #else -ms Tolerance = ms(100); +ms Tolerance = ms(50 * 5); #endif std::shared_timed_mutex m; Modified: libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_duration.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_duration.pass.cpp?rev=248993&r1=248992&r2=248993&view=diff == --- libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_duration.pass.cpp (original) +++ libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_duration.pass.cpp Thu Oct 1 03:34:37 2015 @@ -38,10 +38,10 @@ ms WaitTime = ms(250); // Thread sanitizer causes more overhead and will sometimes cause this test // to fail. To prevent this we give Thread sanitizer more time to complete the // test. -#if !TEST_HAS_FEATURE(thread_sanitizer) +#if !defined(TEST_HAS_SANITIZERS) ms Tolerance = ms(50); #else -ms Tolerance = ms(100); +ms Tolerance = ms(50 * 5); #endif Modified: libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_time_point.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_time_point.pass.cpp?rev=248993&r1=248992&r2=248993&view=diff == --- libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_time_point.pass.cpp (original) +++ libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_time_point.pass.cpp Thu Oct 1 03:34:37 2015 @@ -38,13 +38,12 @@ ms WaitTime = ms(250); // Thread sanitiz
Re: [PATCH] D13292: Add support for 'cbegin()' and 'cend()' on modernize-loop-convert.
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. ... Which indicates that the tests might need better names and more comments, so I don't mess them up next time :D http://reviews.llvm.org/D13292 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13249: Divide TraverseInitListExpr to a different function for each form.
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D13249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r248984 - Test fix
On 1 October 2015 at 05:19, Piotr Padlewski via cfe-commits wrote: > Author: prazek > Date: Wed Sep 30 23:19:45 2015 > New Revision: 248984 > > URL: http://llvm.org/viewvc/llvm-project?rev=248984&view=rev > Log: > Test fix Almost there... :) http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/6671/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Avirtual-function-calls.cpp ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13062: [libcxx] Fix PR24779 -- Prevent evaluation of std::is_default_constructible in clearly ill-formed tuple constructor.
EricWF abandoned this revision. EricWF added a comment. This change is garbage. Sorry for the noise. http://reviews.llvm.org/D13062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11397: [libcxx] Add first bits of
EricWF added a comment. @mclow.lists ping. I would like to keep moving on the LFTS v1 work. http://reviews.llvm.org/D11397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13292: Add support for 'cbegin()' and 'cend()' on modernize-loop-convert.
angelgarcia updated this revision to Diff 36198. angelgarcia added a comment. Yes, right now it is pretty hard to figure out what some of the tests are for, they are a bit messy. I plan to do something about it, but for now I added a comment on that test. http://reviews.llvm.org/D13292 Files: clang-tidy/modernize/LoopConvertCheck.cpp test/clang-tidy/Inputs/modernize-loop-convert/structures.h test/clang-tidy/modernize-loop-convert-basic.cpp Index: test/clang-tidy/modernize-loop-convert-basic.cpp === --- test/clang-tidy/modernize-loop-convert-basic.cpp +++ test/clang-tidy/modernize-loop-convert-basic.cpp @@ -104,6 +104,14 @@ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Elem : ConstArr) // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem, Elem + Elem); + + const NonTriviallyCopyable NonCopy[N]{}; + for (int I = 0; I < N; ++I) { +printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (const auto & Elem : NonCopy) + // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem.X, Elem.X + Elem.X); } struct HasArr { @@ -209,6 +217,13 @@ // CHECK-FIXES: for (auto & P : *Ps) // CHECK-FIXES-NEXT: printf("s has value %d\n", P.X); + for (S::const_iterator It = Ss.cbegin(), E = Ss.cend(); It != E; ++It) { +printf("s has value %d\n", (*It).X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto Elem : Ss) + // CHECK-FIXES-NEXT: printf("s has value %d\n", Elem.X); + for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { printf("s has value %d\n", It->X); } @@ -459,8 +474,8 @@ const int N = 6; dependent V; dependent *Pv; -const dependent Constv; -const dependent *Pconstv; +const dependent Constv; +const dependent *Pconstv; transparent> Cv; @@ -516,50 +531,51 @@ // CHECK-FIXES-NEXT: Sum += Elem + 2; } +// Ensure that 'const auto &' is used with containers of non-trivial types. void constness() { int Sum = 0; for (int I = 0, E = Constv.size(); I < E; ++I) { -printf("Fibonacci number is %d\n", Constv[I]); -Sum += Constv[I] + 2; +printf("Fibonacci number is %d\n", Constv[I].X); +Sum += Constv[I].X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : Constv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : Constv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; for (int I = 0, E = Constv.size(); I < E; ++I) { -printf("Fibonacci number is %d\n", Constv.at(I)); -Sum += Constv.at(I) + 2; +printf("Fibonacci number is %d\n", Constv.at(I).X); +Sum += Constv.at(I).X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : Constv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : Constv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; for (int I = 0, E = Pconstv->size(); I < E; ++I) { -printf("Fibonacci number is %d\n", Pconstv->at(I)); -Sum += Pconstv->at(I) + 2; +printf("Fibonacci number is %d\n", Pconstv->at(I).X); +Sum += Pconstv->at(I).X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : *Pconstv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : *Pconstv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; // This test will fail if size() isn't called repeatedly, since it // returns unsigned int, and 0 is deduced to be signed int. // FIXME: Insert the necessary explicit conversion, or write out the types // explicitly. for (int I = 0; I < Pconstv->size(); ++I) { -printf("Fibonacci number is %d\n", (*Pconstv).at(I)); -Sum += (*Pconstv)[I] + 2; +printf("Fibonacci number is %d\n", (*Pconstv).at(I).X); +Sum += (*Pconstv)[I].X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : *Pconstv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : *Pconstv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; } -void ConstRef(const dependent& ConstVRef) { +v
[clang-tools-extra] r248994 - Add support for 'cbegin()' and 'cend()' on modernize-loop-convert.
Author: angelgarcia Date: Thu Oct 1 03:57:11 2015 New Revision: 248994 URL: http://llvm.org/viewvc/llvm-project?rev=248994&view=rev Log: Add support for 'cbegin()' and 'cend()' on modernize-loop-convert. Summary: This fixes https://llvm.org/bugs/show_bug.cgi?id=22196 . Also add a non-trivially copyable type to fix some tests that were meant to be about using const-refs, but were changed in r248438. Reviewers: klimek Subscribers: alexfh, cfe-commits Differential Revision: http://reviews.llvm.org/D13292 Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=248994&r1=248993&r2=248994&view=diff == --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Thu Oct 1 03:57:11 2015 @@ -112,8 +112,9 @@ StatementMatcher makeArrayLoopMatcher() /// - If the end iterator variable 'g' is defined, it is the same as 'f'. StatementMatcher makeIteratorLoopMatcher() { StatementMatcher BeginCallMatcher = - cxxMemberCallExpr(argumentCountIs(0), -callee(cxxMethodDecl(hasName("begin" + cxxMemberCallExpr( + argumentCountIs(0), + callee(cxxMethodDecl(anyOf(hasName("begin"), hasName("cbegin") .bind(BeginCallName); DeclarationMatcher InitDeclMatcher = @@ -127,7 +128,8 @@ StatementMatcher makeIteratorLoopMatcher varDecl(hasInitializer(anything())).bind(EndVarName); StatementMatcher EndCallMatcher = cxxMemberCallExpr( - argumentCountIs(0), callee(cxxMethodDecl(hasName("end"; + argumentCountIs(0), + callee(cxxMethodDecl(anyOf(hasName("end"), hasName("cend"); StatementMatcher IteratorBoundMatcher = expr(anyOf(ignoringParenImpCasts( @@ -296,7 +298,8 @@ static const Expr *getContainerFromBegin return nullptr; StringRef Name = Member->getMemberDecl()->getName(); StringRef TargetName = IsBegin ? "begin" : "end"; - if (Name != TargetName) + StringRef ConstTargetName = IsBegin ? "cbegin" : "cend"; + if (Name != TargetName && Name != ConstTargetName) return nullptr; const Expr *SourceExpr = Member->getBase(); @@ -423,7 +426,7 @@ void LoopConvertCheck::storeOptions(Clan Options.store(Opts, "MinConfidence", Confs[static_cast(MinConfidence)]); SmallVector Styles{"camelBack", "CamelCase", "lower_case", -"UPPER_CASE"}; + "UPPER_CASE"}; Options.store(Opts, "NamingStyle", Styles[static_cast(NamingStyle)]); } Modified: clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h?rev=248994&r1=248993&r2=248994&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h (original) +++ clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h Thu Oct 1 03:57:11 2015 @@ -16,11 +16,20 @@ struct MutableVal { int X; }; +struct NonTriviallyCopyable { + NonTriviallyCopyable() = default; + // Define this constructor to make this class non-trivially copyable. + NonTriviallyCopyable(const NonTriviallyCopyable& Ntc); + int X; +}; + struct S { typedef MutableVal *iterator; typedef const MutableVal *const_iterator; const_iterator begin() const; const_iterator end() const; + const_iterator cbegin() const; + const_iterator cend() const; iterator begin(); iterator end(); }; Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=248994&r1=248993&r2=248994&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Thu Oct 1 03:57:11 2015 @@ -104,6 +104,14 @@ void constArray() { // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Elem : ConstArr) // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem, Elem + Elem); + + const NonTriviallyCopyable NonCopy[N]{}; + for (int I = 0; I < N; ++I) { +printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X); + } +
Re: [PATCH] D13292: Add support for 'cbegin()' and 'cend()' on modernize-loop-convert.
This revision was automatically updated to reflect the committed changes. Closed by commit rL248994: Add support for 'cbegin()' and 'cend()' on modernize-loop-convert. (authored by angelgarcia). Changed prior to commit: http://reviews.llvm.org/D13292?vs=36198&id=36200#toc Repository: rL LLVM http://reviews.llvm.org/D13292 Files: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Index: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp === --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp @@ -104,6 +104,14 @@ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Elem : ConstArr) // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem, Elem + Elem); + + const NonTriviallyCopyable NonCopy[N]{}; + for (int I = 0; I < N; ++I) { +printf("2 * %d = %d\n", NonCopy[I].X, NonCopy[I].X + NonCopy[I].X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (const auto & Elem : NonCopy) + // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem.X, Elem.X + Elem.X); } struct HasArr { @@ -209,6 +217,13 @@ // CHECK-FIXES: for (auto & P : *Ps) // CHECK-FIXES-NEXT: printf("s has value %d\n", P.X); + for (S::const_iterator It = Ss.cbegin(), E = Ss.cend(); It != E; ++It) { +printf("s has value %d\n", (*It).X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto Elem : Ss) + // CHECK-FIXES-NEXT: printf("s has value %d\n", Elem.X); + for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { printf("s has value %d\n", It->X); } @@ -459,8 +474,8 @@ const int N = 6; dependent V; dependent *Pv; -const dependent Constv; -const dependent *Pconstv; +const dependent Constv; +const dependent *Pconstv; transparent> Cv; @@ -516,50 +531,51 @@ // CHECK-FIXES-NEXT: Sum += Elem + 2; } +// Ensure that 'const auto &' is used with containers of non-trivial types. void constness() { int Sum = 0; for (int I = 0, E = Constv.size(); I < E; ++I) { -printf("Fibonacci number is %d\n", Constv[I]); -Sum += Constv[I] + 2; +printf("Fibonacci number is %d\n", Constv[I].X); +Sum += Constv[I].X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : Constv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : Constv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; for (int I = 0, E = Constv.size(); I < E; ++I) { -printf("Fibonacci number is %d\n", Constv.at(I)); -Sum += Constv.at(I) + 2; +printf("Fibonacci number is %d\n", Constv.at(I).X); +Sum += Constv.at(I).X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : Constv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : Constv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; for (int I = 0, E = Pconstv->size(); I < E; ++I) { -printf("Fibonacci number is %d\n", Pconstv->at(I)); -Sum += Pconstv->at(I) + 2; +printf("Fibonacci number is %d\n", Pconstv->at(I).X); +Sum += Pconstv->at(I).X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : *Pconstv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const auto & Elem : *Pconstv) + // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem.X); + // CHECK-FIXES-NEXT: Sum += Elem.X + 2; // This test will fail if size() isn't called repeatedly, since it // returns unsigned int, and 0 is deduced to be signed int. // FIXME: Insert the necessary explicit conversion, or write out the types // explicitly. for (int I = 0; I < Pconstv->size(); ++I) { -printf("Fibonacci number is %d\n", (*Pconstv).at(I)); -Sum += (*Pconstv)[I] + 2; +printf("Fibonacci number is %d\n", (*Pconstv).at(I).X); +Sum += (*Pconstv)[I].X + 2; } // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead - // CHECK-FIXES: for (auto Elem : *Pconstv) - // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", Elem); - // CHECK-FIXES-NEXT: Sum += Elem + 2; + // CHECK-FIXES: for (const au
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. The patch looks good with one comment. I'll fix it and submit the patch for you. Thank you for working on this! Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:548 @@ +547,3 @@ + } + + if (const auto *Loc = Result.Nodes.getNodeAs("typeLoc")) { Note, that I suggested to use `Loc->getType()->getDecl()`, which uses `TypeLoc::getType()` and `Type::getDecl()`. http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
alexfh added inline comments. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:548 @@ +547,3 @@ + } + + if (const auto *Loc = Result.Nodes.getNodeAs("typeLoc")) { alexfh wrote: > Note, that I suggested to use `Loc->getType()->getDecl()`, which uses > `TypeLoc::getType()` and `Type::getDecl()`. Ah, I see now: the problem is not `TypeLoc` here, it's that there's no `Type::getDecl`. So your version is probably the best we can do here. http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r248996 - [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
Author: alexfh Date: Thu Oct 1 04:19:40 2015 New Revision: 248996 URL: http://llvm.org/viewvc/llvm-project?rev=248996&view=rev Log: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck This diff requires http://reviews.llvm.org/D13079 to be applied first. I wasn't sure about how to make patch series in Phabricator, and I wanted to keep the two separate for clarity. It looks like that most cases can be supported with this patch. I'm not totally sure about the actual coverage though. I think that the matchers are very generic, but I'm still not totally fluent with the AST. Patch by Beren Minor! Differential revision: http://reviews.llvm.org/D13081 Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/ clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/system/ clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/user-header.h Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=248996&r1=248995&r2=248996&view=diff == --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp Thu Oct 1 04:19:40 2015 @@ -136,13 +136,13 @@ void IdentifierNamingCheck::storeOptions } void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) { - // FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking - // and replacement. There is a lot of missing cases, such as references to a - // class name (as in 'const int CMyClass::kClassConstant = 4;'), to an - // enclosing context (namespace, class, etc). - Finder->addMatcher(namedDecl().bind("decl"), this); - Finder->addMatcher(declRefExpr().bind("declref"), this); + Finder->addMatcher(usingDecl().bind("using"), this); + Finder->addMatcher(declRefExpr().bind("declRef"), this); + Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this); + Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this); + Finder->addMatcher(typeLoc().bind("typeLoc"), this); + Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); } static bool matchesStyle(StringRef Name, @@ -504,27 +504,121 @@ static StyleKind findStyleKind( static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, const NamedDecl *Decl, SourceRange Range, const SourceManager *SM) { + // Do nothin if the provided range is invalid + if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid()) +return; + + // Try to insert the identifier location in the Usages map, and bail out if it + // is already in there auto &Failure = Failures[Decl]; if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second) return; - Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) && - SM->isInMainFile(Range.getEnd()) && - !Range.getBegin().isMacroID() && + Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() && !Range.getEnd().isMacroID(); } void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { - if (const auto *DeclRef = Result.Nodes.getNodeAs("declref")) { + if (const auto *Decl = + Result.Nodes.getNodeAs("classRef")) { +if (Decl->isImplicit()) + return; + +addUsage(NamingCheckFailures, Decl->getParent(), + Decl->getNameInfo().getSourceRange(), Result.SourceManager); +return; + } + + if (const auto *Decl = + Result.Nodes.getNodeAs("classRef")) { +if (Decl->isImplicit()) + return; + +SourceRange Range = Decl->getNameInfo().getSourceRange(); +if (Range.getBegin().isInvalid()) + return; +// The first token that will be found is the ~ (or the equivalent trigraph), +// we want instead to replace the next token, that will be the identifier. +Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd()); + +addUsage(NamingCheckFailures, Decl->getParent(), Range, + Result.SourceManager); +return; + } + + if (const auto *Loc = Result.Nodes.getNodeAs("typeLoc")) { +NamedDecl *Decl = nullptr; +if (const auto &Ref = Loc->getAs()) { + Decl = Ref.getDecl(); +} else if (const auto &Ref = Loc->getAs()) { + Decl = Ref.getDecl(); +} else if (const auto &Ref = Loc->getAs()) { + Decl = Ref.getDecl(); +
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
This revision was automatically updated to reflect the committed changes. Closed by commit rL248996: [clang-tidy] Implement FixitHints for identifier references in… (authored by alexfh). Changed prior to commit: http://reviews.llvm.org/D13081?vs=35974&id=36202#toc Repository: rL LLVM http://reviews.llvm.org/D13081 Files: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/user-header.h clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/user-header.h === --- clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/user-header.h +++ clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/user-header.h @@ -0,0 +1,17 @@ +#ifndef USER_HEADER_H +#define USER_HEADER_H + +#define USER_MACRO(m) int m = 0 + +namespace USER_NS { +class object { + int member; +}; + +float global; + +void function() { +} +} + +#endif // USER_HEADER_H Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h === --- clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h +++ clang-tools-extra/trunk/test/clang-tidy/Inputs/readability-identifier-naming/system/system-header.h @@ -0,0 +1,17 @@ +#ifndef SYSTEM_HEADER_H +#define SYSTEM_HEADER_H + +#define SYSTEM_MACRO(m) int m = 0 + +namespace SYSTEM_NS { +class structure { + int member; +}; + +float global; + +void function() { +} +} + +#endif // SYSTEM_HEADER_H Index: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp === --- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp @@ -62,25 +62,44 @@ // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \ // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \ // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \ -// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing - -// FIXME: There should be more test cases for checking that references to class -// FIXME: name, declaration contexts, forward declarations, etc, are correctly -// FIXME: checked and renamed +// RUN: ]}' -- -std=c++11 -fno-delayed-template-parsing \ +// RUN: -I%S/Inputs/readability-identifier-naming \ +// RUN: -isystem %S/Inputs/readability-identifier-naming/system // clang-format off +#include +#include "user-header.h" +// NO warnings or fixes expected from declarations within header files without +// the -header-filter= option + namespace FOO_NS { // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming] // CHECK-FIXES: {{^}}namespace foo_ns {{{$}} inline namespace InlineNamespace { // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for inline namespace 'InlineNamespace' // CHECK-FIXES: {{^}}inline namespace inline_namespace {{{$}} +SYSTEM_NS::structure g_s1; +// NO warnings or fixes expected as SYSTEM_NS and structure are declared in a header file + +USER_NS::object g_s2; +// NO warnings or fixes expected as USER_NS and object are declared in a header file + +SYSTEM_MACRO(var1); +// NO warnings or fixes expected as var1 is from macro expansion + +USER_MACRO(var2); +// NO warnings or fixes expected as var2 is declared in a macro expansion + +int global; +#define USE_IN_MACRO(m) auto use_##m = m +USE_IN_MACRO(global); +// NO warnings or fixes expected as global is used in a macro expansion + #define BLA int FOO_bar BLA; -// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for global variable 'FOO_bar' -// NO fix expected as FOO_bar is from macro expansion +// NO warnings or fixes expected as FOO_bar is from macro expansion enum my_enumeration { // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for enum 'my_enumeration' @@ -104,6 +123,13 @@ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for class 'my_class' // CHECK-FIXES: {{^}}class CMyClass {{{$}} my_class(); +// CHECK-FIXES: {{^}}CMyClass();{{$}} + +~ + my_class(); +// (space in destructor token test, we could check trigraph but they will be deprecated) +// CHECK-FIXES: {{^}}~{{$}} +// CHECK-FIXES: {{^}} CMyClass();{{$}} const int MEMBER_one_1 = ConstExpr_variable; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for constant member 'MEMBER_one_1' @@ -137,15 +163,36 @@ const int my_clas
[clang-tools-extra] r248997 - [clang-tidy] fix add_new_check.py
Author: alexfh Date: Thu Oct 1 04:23:20 2015 New Revision: 248997 URL: http://llvm.org/viewvc/llvm-project?rev=248997&view=rev Log: [clang-tidy] fix add_new_check.py Before this check, I would get the following error: Updating ./misc/CMakeLists.txt... Creating ./misc/NoReinterpret_castCheck.h... Creating ./misc/NoReinterpret_castCheck.cpp... Updating ./misc/MiscTidyModule.cpp... Creating ../test/clang-tidy/misc-no-reinterpret_cast.cpp... Creating ../docs/clang-tidy/checks/misc-no-reinterpret_cast.rst... Traceback (most recent call last): File "./add_new_check.py", line 271, in main() File "./add_new_check.py", line 267, in main update_checks_list(module_path) File "./add_new_check.py", line 220, in update_checks_list os.listdir('docs/clang-tidy/checks'))) OSError: [Errno 2] No such file or directory: 'docs/clang-tidy/checks' Patch by Matthias Gehre! Differential revision: http://reviews.llvm.org/D13272 Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=248997&r1=248996&r2=248997&view=diff == --- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original) +++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Thu Oct 1 04:23:20 2015 @@ -217,7 +217,7 @@ def update_checks_list(module_path): checks = map(lambda s: ' ' + s.replace('.rst', '\n'), filter(lambda s: s.endswith('.rst') and s != 'list.rst', - os.listdir('docs/clang-tidy/checks'))) + os.listdir(os.path.join(module_path, '../../docs/clang-tidy/checks' checks.sort() print('Updating %s...' % filename) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13272: [clang-tidy] fix add_new_check.py
alexfh added a comment. In http://reviews.llvm.org/D13272#256305, @mgehre wrote: > Yes, please. Btw, I'm going to submit some diffs for clang-tidy checks (for > the CppCoreGuidelines). Which reviewers should I set on them? Please always add me to the reviewers and cfe-commits to Subscribers. Also note, that in October I'll be mostly unavailable, but other clang-tidy contributors can do the review and help getting patches in: sbenza, bkramer, aaron.ballman. Patches are always welcome ;) Repository: rL LLVM http://reviews.llvm.org/D13272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
alexfh added a comment. Also note, that in October I'll be mostly unavailable, but other clang-tidy contributors can do the review and help getting patches in: sbenza, bkramer, aaron.ballman. Repository: rL LLVM http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13081: [clang-tidy] Implement FixitHints for identifier references in IdentifierNamingCheck
berenm added a comment. Alright, thank you for your time! Repository: rL LLVM http://reviews.llvm.org/D13081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm updated this revision to Diff 36207. berenm added a comment. Fix arcanist insisting on creating the diff against old revision... http://reviews.llvm.org/D12362 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/WhitespaceManager.cpp lib/Format/WhitespaceManager.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -8645,6 +8645,189 @@ Alignment); } +TEST_F(FormatTest, AlignConsecutiveDeclarations) { + FormatStyle Alignment = getLLVMStyle(); + Alignment.AlignConsecutiveDeclarations = false; + verifyFormat("float const a = 5;\n" + "int oneTwoThree = 123;", + Alignment); + verifyFormat("int a = 5;\n" + "float const oneTwoThree = 123;", + Alignment); + + Alignment.AlignConsecutiveDeclarations = true; + verifyFormat("float const a = 5;\n" + "int oneTwoThree = 123;", + Alignment); + verifyFormat("int a = method();\n" + "float const oneTwoThree = 133;", + Alignment); + verifyFormat("int i = 1, j = 10;\n" + "something = 2000;", + Alignment); + verifyFormat("something = 2000;\n" + "int i = 1, j = 10;\n", + Alignment); + verifyFormat("float something = 2000;\n" + "double another = 911;\n" + "inti = 1, j = 10;\n" + "const int *oneMore = 1;\n" + "unsigned i = 2;", + Alignment); + verifyFormat("float a = 5;\n" + "int one = 1;\n" + "method();\n" + "const double oneTwoThree = 123;\n" + "const unsigned int oneTwo = 12;", + Alignment); + verifyFormat("int oneTwoThree{0}; // comment\n" + "unsigned oneTwo; // comment", + Alignment); + EXPECT_EQ("float const a = 5;\n" +"\n" +"int oneTwoThree = 123;", +format("float const a = 5;\n" + "\n" + "int oneTwoThree= 123;", + Alignment)); + EXPECT_EQ("float a = 5;\n" +"int one = 1;\n" +"\n" +"unsigned oneTwoThree = 123;", +format("floata = 5;\n" + "int one = 1;\n" + "\n" + "unsigned oneTwoThree = 123;", + Alignment)); + EXPECT_EQ("float a = 5;\n" +"int one = 1;\n" +"\n" +"unsigned oneTwoThree = 123;\n" +"int oneTwo = 12;", +format("floata = 5;\n" + "int one = 1;\n" + "\n" + "unsigned oneTwoThree = 123;\n" + "int oneTwo = 12;", + Alignment)); + Alignment.AlignConsecutiveAssignments = true; + verifyFormat("float something = 2000;\n" + "double another = 911;\n" + "inti = 1, j = 10;\n" + "const int *oneMore = 1;\n" + "unsigned i = 2;", + Alignment); + verifyFormat("int oneTwoThree = {0}; // comment\n" + "unsigned oneTwo = 0; // comment", + Alignment); + EXPECT_EQ("void SomeFunction(int parameter = 0) {\n" +" int const i = 1;\n" +" int * j = 2;\n" +" int big = 1;\n" +"\n" +" unsigned oneTwoThree = 123;\n" +" int oneTwo = 12;\n" +" method();\n" +" float k = 2;\n" +" int ll = 1;\n" +"}", +format("void SomeFunction(int parameter= 0) {\n" + " int const i= 1;\n" + " int *j=2;\n" + " int big = 1;\n" + "\n" + "unsigned oneTwoThree =123;\n" + "int oneTwo = 12;\n" + " method();\n" + "float k= 2;\n" + "int ll=1;\n" + "}", + Alignment)); + Alignment.AlignConsecutiveAssignments = false; + Alignment.AlignEscapedNewlinesLeft = true; + verifyFormat("#define A \\\n" + " int = 12; \\\n" + " float b = 23;\\\n" + " const int ccc = 234; \\\n" + " unsigned dd = 2345;", + Alignment); + Alignment.AlignEscapedNewlinesLeft = false; + Alignment.ColumnLimit = 30; + verifyFormat("#define A\\\n" + " int = 12; \\\n" + " float b = 23; \\\n" + " const int ccc = 234; \\\n" + " i
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm marked 7 inline comments as done. Comment at: unittests/Format/FormatTest.cpp:8699 @@ +8698,3 @@ + "int one = 1;\n" + "\n" + "unsigned oneTwoThree = 123;", I think I actually managed to do something about that. I added a test case as well to check that AlignConsecutive* respect the column limit. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm marked 2 inline comments as done. berenm added a comment. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12839: Extend MoveConstructorInitCheck to also flag constructor arguments passed by value and can be moved assigned to fields.
alexfh added a comment. The patch doesn't apply cleanly. Please update it. http://reviews.llvm.org/D12839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. This looks awesome. Do you have commit access? Comment at: unittests/Format/FormatTest.cpp:8699 @@ +8698,3 @@ + "int one = 1;\n" + "\n" + "unsigned oneTwoThree = 123;", berenm wrote: > I think I actually managed to do something about that. > > I added a test case as well to check that AlignConsecutive* respect the > column limit. Very nice! http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
berenm added a comment. Thanks, I don't have commit access. http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12362: [clang-format] Add support of consecutive declarations alignment
djasper closed this revision. djasper added a comment. Submitted as r248999. Thank you! http://reviews.llvm.org/D12362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r248999 - [clang-format] Add support of consecutive declarations alignment
Author: djasper Date: Thu Oct 1 05:06:54 2015 New Revision: 248999 URL: http://llvm.org/viewvc/llvm-project?rev=248999&view=rev Log: [clang-format] Add support of consecutive declarations alignment This allows clang-format to align identifiers in consecutive declarations. This is useful for increasing the readability of the code in the same way the alignment of assignations is. The code is a slightly modified version of the consecutive assignment alignment code. Currently only the identifiers are aligned, and there is no support of alignment of the pointer star or reference symbol. The patch also solve the issue of alignments not being possible due to the ColumnLimit for both the existing AlignConsecutiveAligments and the new AlignConsecutiveDeclarations. Patch by Beren Minor, thank you. Review: http://reviews.llvm.org/D12362 Modified: cfe/trunk/include/clang/Format/Format.h cfe/trunk/lib/Format/Format.cpp cfe/trunk/lib/Format/WhitespaceManager.cpp cfe/trunk/lib/Format/WhitespaceManager.h cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/include/clang/Format/Format.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=248999&r1=248998&r2=248999&view=diff == --- cfe/trunk/include/clang/Format/Format.h (original) +++ cfe/trunk/include/clang/Format/Format.h Thu Oct 1 05:06:54 2015 @@ -64,6 +64,17 @@ struct FormatStyle { /// \endcode bool AlignConsecutiveAssignments; + /// \brief If \c true, aligns consecutive declarations. + /// + /// This will align the declaration names of consecutive lines. This + /// will result in formattings like + /// \code + /// int = 12; + /// float b = 23; + /// std::string ccc = 23; + /// \endcode + bool AlignConsecutiveDeclarations; + /// \brief If \c true, aligns escaped newlines as far left as possible. /// Otherwise puts them into the right-most column. bool AlignEscapedNewlinesLeft; @@ -495,6 +506,7 @@ struct FormatStyle { return AccessModifierOffset == R.AccessModifierOffset && AlignAfterOpenBracket == R.AlignAfterOpenBracket && AlignConsecutiveAssignments == R.AlignConsecutiveAssignments && + AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations && AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft && AlignOperands == R.AlignOperands && AlignTrailingComments == R.AlignTrailingComments && Modified: cfe/trunk/lib/Format/Format.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=248999&r1=248998&r2=248999&view=diff == --- cfe/trunk/lib/Format/Format.cpp (original) +++ cfe/trunk/lib/Format/Format.cpp Thu Oct 1 05:06:54 2015 @@ -201,6 +201,8 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=248999&r1=248998&r2=248999&view=diff == --- cfe/trunk/lib/Format/WhitespaceManager.cpp (original) +++ cfe/trunk/lib/Format/WhitespaceManager.cpp Thu Oct 1 05:06:54 2015 @@ -29,13 +29,15 @@ WhitespaceManager::Change::Change( bool CreateReplacement, const SourceRange &OriginalWhitespaceRange, unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn, unsigned NewlinesBefore, StringRef PreviousLinePostfix, -StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective) +StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective, +bool IsStartOfDeclName) : CreateReplacement(CreateReplacement), OriginalWhitespaceRange(OriginalWhitespaceRange), StartOfTokenColumn(StartOfTokenColumn), NewlinesBefore(NewlinesBefore), PreviousLinePostfix(PreviousLinePostfix), CurrentLinePrefix(CurrentLinePrefix), Kind(Kind), - ContinuesPPDirective(ContinuesPPDirective), IndentLevel(IndentLevel), + ContinuesPPDirective(ContinuesPPDirective), + IsStartOfDeclName(IsStartOfDeclName), IndentLevel(IndentLevel), Spaces(Spaces), IsTrailingComment(false), TokenLength(0), PreviousEndOfTokenColumn(0), EscapedNewlineColumn(0), StartOfBlockComment(nullptr), IndentationOffset(0) {} @@ -52,19 +54,21 @@ void WhitespaceManager::replaceWhitespac if (Tok.Finalized) return; Tok.Decision = (Newlines > 0) ? FD_Break : FD_Continue; - Changes.push_back(Change(true, Tok.WhitespaceRange, IndentLevel, Spaces, - StartOfTokenColumn, Newlines, "", "", - Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst)); + Changes.push_back( + Change(true, Tok.WhitespaceRange, IndentLevel, Spaces, StartOfTokenColumn, + Newlines, "", "", Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst, + Tok.is(TT_StartOfName) |
[PATCH] D13336: [MSVC] 'property' with an empty array in array subscript expression.
ABataev created this revision. ABataev added a reviewer: rnk. ABataev added a subscriber: cfe-commits. MSVC supports 'property' attribute and allows to apply it to the declaration of an empty array in a class or structure definition. For example: ``` __declspec(property(get=GetX, put=PutX)) int x[]; ``` The above statement indicates that x[] can be used with one or more array indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b), and p->x[a][b] = i will be turned into p->PutX(a, b, i); http://reviews.llvm.org/D13336 Files: include/clang/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaPseudoObject.cpp test/CodeGenCXX/ms-property.cpp Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -3958,7 +3958,21 @@ // operand might be an overloadable type, in which case the overload // resolution for the operator overload should get the first crack // at the overload. - if (base->getType()->isNonOverloadPlaceholderType()) { + // MSDN, property (C++) + // https://msdn.microsoft.com/en-us/library/yhfk0thd(v=vs.120).aspx + // This attribute can also be used in the declaration of an empty array in a + // class or structure definition. For example: + // __declspec(property(get=GetX, put=PutX)) int x[]; + // The above statement indicates that x[] can be used with one or more array + // indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b), + // and p->x[a][b] = i will be turned into p->PutX(a, b, i); + auto *MSProp = dyn_cast(base->IgnoreParens()); + if (MSProp && MSProp->getPropertyDecl()->getType()->isArrayType()) { +auto I = MSPropArgsMap.find(MSProp); +if (I == MSPropArgsMap.end()) + I = MSPropArgsMap.insert(std::make_pair(MSProp, MSPropArgsTy())).first; +I->second.push_back(idx); + } else if (base->getType()->isNonOverloadPlaceholderType()) { ExprResult result = CheckPlaceholderExpr(base); if (result.isInvalid()) return ExprError(); base = result.get(); @@ -3969,6 +3983,9 @@ idx = result.get(); } + if (MSProp && MSProp->getPropertyDecl()->getType()->isArrayType()) +return base; + // Build an unanalyzed expression if either operand is type-dependent. if (getLangOpts().CPlusPlus && (base->isTypeDependent() || idx->isTypeDependent())) { Index: lib/Sema/SemaPseudoObject.cpp === --- lib/Sema/SemaPseudoObject.cpp +++ lib/Sema/SemaPseudoObject.cpp @@ -1432,6 +1432,11 @@ } MultiExprArg ArgExprs; + auto I = S.MSPropArgsMap.find(RefExpr); + if (I != S.MSPropArgsMap.end()) { +ArgExprs = I->second; +S.MSPropArgsMap.erase(I); + } return S.ActOnCallExpr(S.getCurScope(), GetterExpr.get(), RefExpr->getSourceRange().getBegin(), ArgExprs, RefExpr->getSourceRange().getEnd()); @@ -1461,7 +1466,12 @@ return ExprError(); } - SmallVector ArgExprs; + Sema::MSPropArgsTy ArgExprs; + auto I = S.MSPropArgsMap.find(RefExpr); + if (I != S.MSPropArgsMap.end()) { +ArgExprs = I->second; +S.MSPropArgsMap.erase(I); + } ArgExprs.push_back(op); return S.ActOnCallExpr(S.getCurScope(), SetterExpr.get(), RefExpr->getSourceRange().getBegin(), ArgExprs, Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -463,6 +463,10 @@ typedef llvm::SmallVector DeleteLocs; llvm::MapVector DeleteExprs; + /// \brief Map of additional arguments for ms property calls. + typedef llvm::SmallVector MSPropArgsTy; + llvm::DenseMap MSPropArgsMap; + typedef llvm::SmallPtrSet RecordDeclSetTy; /// PureVirtualClassDiagSet - a set of class declarations which we have Index: test/CodeGenCXX/ms-property.cpp === --- test/CodeGenCXX/ms-property.cpp +++ test/CodeGenCXX/ms-property.cpp @@ -0,0 +1,35 @@ +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s + +class S { +public: + __declspec(property(get=GetX,put=PutX)) int x[]; + int GetX(int i, int j) { return i+j; } + void PutX(int i, int j, int k) { j = i = k; } +}; + +template +class St { +public: + __declspec(property(get=GetX,put=PutX)) T x[]; + T GetX(T i, T j) { return i+j; } + void PutX(T i, T j, T k) { j = i = k; } +}; + +// CHECK-LABEL: main +int main() +{ + S *p1 = 0; + St *p2 = 0; + // CHECK: call i32 @"\01?GetX@S@@QEAAHHH@Z"(%class.S* %{{.+}}, i32 223, i32 11) + int j = p1->x[223][11]; + // CHECK: [[J:%.+]] = load i32, i32* % + // CHECK-NEXT: call void @"\01?PutX@S@@QEAAXHHH@Z"(%class.S* %{{.+}}, i32 23, i32 1, i32 [[J]]) + p1->x[23][1] = j; + // CHECK: call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %{{.+}}, float 2.23e+02, float 1.10e+01) + float j1 = p2-
[PATCH] D13337: [libcxx] Attempt to fix __throw_future_error in C++03
EricWF created this revision. EricWF added a reviewer: mclow.lists. EricWF added a subscriber: cfe-commits. Hi Marshall, Could you please test this patch and see if you run into the same linker errors we talked about? I can't reproduce on linux or OS X. Hopefully you can't find any problems and we can fix the C++03 bot. http://reviews.llvm.org/D13337 Files: include/future Index: include/future === --- include/future +++ include/future @@ -512,9 +512,8 @@ virtual ~future_error() _NOEXCEPT; }; -template -_LIBCPP_ALWAYS_INLINE -void __throw_future_error() +inline _LIBCPP_ALWAYS_INLINE +void __throw_future_error(future_errc _Ev) { #ifndef _LIBCPP_NO_EXCEPTIONS throw future_error(make_error_code(_Ev)); @@ -657,7 +656,7 @@ { unique_lock __lk(this->__mut_); if (this->__has_value()) -__throw_future_error(); +__throw_future_error(future_errc::promise_already_satisfied); ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg)); this->__state_ |= base::__constructed | base::ready; __cv_.notify_all(); @@ -674,7 +673,7 @@ { unique_lock __lk(this->__mut_); if (this->__has_value()) -__throw_future_error(); +__throw_future_error(future_errc::promise_already_satisfied); ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg)); this->__state_ |= base::__constructed; __thread_local_data()->__make_ready_at_thread_exit(this); @@ -733,7 +732,7 @@ { unique_lock __lk(this->__mut_); if (this->__has_value()) -__throw_future_error(); +__throw_future_error(future_errc::promise_already_satisfied); __value_ = _VSTD::addressof(__arg); this->__state_ |= base::__constructed | base::ready; __cv_.notify_all(); @@ -745,7 +744,7 @@ { unique_lock __lk(this->__mut_); if (this->__has_value()) -__throw_future_error(); +__throw_future_error(future_errc::promise_already_satisfied); __value_ = _VSTD::addressof(__arg); this->__state_ |= base::__constructed; __thread_local_data()->__make_ready_at_thread_exit(this); @@ -1142,7 +1141,7 @@ : __state_(__state) { if (__state_->__has_future_attached()) -__throw_future_error(); +__throw_future_error(future_errc::future_already_retrieved); __state_->__add_shared(); __state_->__set_future_attached(); } @@ -1244,7 +1243,7 @@ : __state_(__state) { if (__state_->__has_future_attached()) -__throw_future_error(); +__throw_future_error(future_errc::future_already_retrieved); __state_->__add_shared(); __state_->__set_future_attached(); } @@ -1445,7 +1444,7 @@ promise<_Rp>::get_future() { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); return future<_Rp>(__state_); } @@ -1454,7 +1453,7 @@ promise<_Rp>::set_value(const _Rp& __r) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_value(__r); } @@ -1465,7 +1464,7 @@ promise<_Rp>::set_value(_Rp&& __r) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_value(_VSTD::move(__r)); } @@ -1476,7 +1475,7 @@ promise<_Rp>::set_exception(exception_ptr __p) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_exception(__p); } @@ -1485,7 +1484,7 @@ promise<_Rp>::set_value_at_thread_exit(const _Rp& __r) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_value_at_thread_exit(__r); } @@ -1496,7 +1495,7 @@ promise<_Rp>::set_value_at_thread_exit(_Rp&& __r) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_value_at_thread_exit(_VSTD::move(__r)); } @@ -1507,7 +1506,7 @@ promise<_Rp>::set_exception_at_thread_exit(exception_ptr __p) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_exception_at_thread_exit(__p); } @@ -1605,7 +1604,7 @@ promise<_Rp&>::get_future() { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); return future<_Rp&>(__state_); } @@ -1614,7 +1613,7 @@ promise<_Rp&>::set_value(_Rp& __r) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_value(__r); } @@ -1623,7 +1622,7 @@ promise<_Rp&>::set_exception(exception_ptr __p) { if (__state_ == nullptr) -__throw_future_error(); +__throw_future_error(future_errc::no_state); __state_->set_exception(__p); } @@
r249001 - C++11 rangify for loops, NFC.
Author: yrnkrn Date: Thu Oct 1 06:19:28 2015 New Revision: 249001 URL: http://llvm.org/viewvc/llvm-project?rev=249001&view=rev Log: C++11 rangify for loops, NFC. Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=249001&r1=249000&r2=249001&view=diff == --- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original) +++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Thu Oct 1 06:19:28 2015 @@ -246,10 +246,8 @@ void InitHeaderSearch::AddDefaultCInclud if (CIncludeDirs != "") { SmallVector dirs; CIncludeDirs.split(dirs, ":"); -for (SmallVectorImpl::iterator i = dirs.begin(); - i != dirs.end(); - ++i) - AddPath(*i, ExternCSystem, false); +for (StringRef dir : dirs) + AddPath(dir, ExternCSystem, false); return; } @@ -559,39 +557,33 @@ void InitHeaderSearch::Realize(const Lan SearchList.reserve(IncludePath.size()); // Quoted arguments go first. - for (path_iterator it = IncludePath.begin(), ie = IncludePath.end(); - it != ie; ++it) { -if (it->first == Quoted) - SearchList.push_back(it->second); - } + for (auto &Include : IncludePath) +if (Include.first == Quoted) + SearchList.push_back(Include.second); + // Deduplicate and remember index. RemoveDuplicates(SearchList, 0, Verbose); unsigned NumQuoted = SearchList.size(); - for (path_iterator it = IncludePath.begin(), ie = IncludePath.end(); - it != ie; ++it) { -if (it->first == Angled || it->first == IndexHeaderMap) - SearchList.push_back(it->second); - } + for (auto &Include : IncludePath) +if (Include.first == Angled || Include.first == IndexHeaderMap) + SearchList.push_back(Include.second); RemoveDuplicates(SearchList, NumQuoted, Verbose); unsigned NumAngled = SearchList.size(); - for (path_iterator it = IncludePath.begin(), ie = IncludePath.end(); - it != ie; ++it) { -if (it->first == System || it->first == ExternCSystem || -(!Lang.ObjC1 && !Lang.CPlusPlus && it->first == CSystem)|| -(/*FIXME !Lang.ObjC1 && */Lang.CPlusPlus && it->first == CXXSystem) || -(Lang.ObjC1 && !Lang.CPlusPlus && it->first == ObjCSystem) || -(Lang.ObjC1 && Lang.CPlusPlus && it->first == ObjCXXSystem)) - SearchList.push_back(it->second); - } - - for (path_iterator it = IncludePath.begin(), ie = IncludePath.end(); - it != ie; ++it) { -if (it->first == After) - SearchList.push_back(it->second); - } + for (auto &Include : IncludePath) +if (Include.first == System || Include.first == ExternCSystem || +(!Lang.ObjC1 && !Lang.CPlusPlus && Include.first == CSystem) || +(/*FIXME !Lang.ObjC1 && */ Lang.CPlusPlus && + Include.first == CXXSystem) || +(Lang.ObjC1 && !Lang.CPlusPlus && Include.first == ObjCSystem) || +(Lang.ObjC1 && Lang.CPlusPlus && Include.first == ObjCXXSystem)) + SearchList.push_back(Include.second); + + for (auto &Include : IncludePath) +if (Include.first == After) + SearchList.push_back(Include.second); // Remove duplicates across both the Angled and System directories. GCC does // this and failing to remove duplicates across these two groups breaks ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13339: Allow a ToolChain to compute the path of a compiler-rt's component.
vkalintiris created this revision. vkalintiris added reviewers: atanasyan, rsmith. vkalintiris added a subscriber: cfe-commits. Herald added subscribers: srhines, danalbert, tberghammer. This patch moves getCompilerRT() from the clang::driver::tools namespace to the ToolChain class. This is needed for multilib toolchains that need to place their libraries in Clang's resource directory with a layout that is different from the default one. http://reviews.llvm.org/D13339 Files: include/clang/Driver/ToolChain.h lib/Driver/SanitizerArgs.cpp lib/Driver/ToolChain.cpp lib/Driver/Tools.cpp Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -2402,60 +2402,11 @@ } } -// Until ARM libraries are build separately, we have them all in one library -static StringRef getArchNameForCompilerRTLib(const ToolChain &TC, - const ArgList &Args) { - const llvm::Triple &Triple = TC.getTriple(); - bool IsWindows = Triple.isOSWindows(); - - if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86) -return "i386"; - - if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb) -return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows) - ? "armhf" - : "arm"; - - return TC.getArchName(); -} - -static SmallString<128> getCompilerRTLibDir(const ToolChain &TC) { - // The runtimes are located in the OS-specific resource directory. - SmallString<128> Res(TC.getDriver().ResourceDir); - const llvm::Triple &Triple = TC.getTriple(); - // TC.getOS() yield "freebsd10.0" whereas "freebsd" is expected. - StringRef OSLibName = - (Triple.getOS() == llvm::Triple::FreeBSD) ? "freebsd" : TC.getOS(); - llvm::sys::path::append(Res, "lib", OSLibName); - return Res; -} - -SmallString<128> tools::getCompilerRT(const ToolChain &TC, const ArgList &Args, - StringRef Component, bool Shared) { - const char *Env = TC.getTriple().getEnvironment() == llvm::Triple::Android -? "-android" -: ""; - - bool IsOSWindows = TC.getTriple().isOSWindows(); - bool IsITANMSVCWindows = TC.getTriple().isWindowsMSVCEnvironment() || - TC.getTriple().isWindowsItaniumEnvironment(); - StringRef Arch = getArchNameForCompilerRTLib(TC, Args); - const char *Prefix = IsITANMSVCWindows ? "" : "lib"; - const char *Suffix = - Shared ? (IsOSWindows ? ".dll" : ".so") : (IsITANMSVCWindows ? ".lib" : ".a"); - - SmallString<128> Path = getCompilerRTLibDir(TC); - llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" + -Arch + Env + Suffix); - - return Path; -} - static const char *getCompilerRTArgString(const ToolChain &TC, const llvm::opt::ArgList &Args, StringRef Component, bool Shared = false) { - return Args.MakeArgString(getCompilerRT(TC, Args, Component, Shared)); + return Args.MakeArgString(TC.getCompilerRT(Args, Component, Shared)); } // This adds the static libclang_rt.builtins-arch.a directly to the command line @@ -2569,7 +2520,7 @@ static bool addSanitizerDynamicList(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs, StringRef Sanitizer) { - SmallString<128> SanRT = getCompilerRT(TC, Args, Sanitizer); + SmallString<128> SanRT = TC.getCompilerRT(Args, Sanitizer); if (llvm::sys::fs::exists(SanRT + ".syms")) { CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + SanRT + ".syms")); return true; Index: lib/Driver/ToolChain.cpp === --- lib/Driver/ToolChain.cpp +++ lib/Driver/ToolChain.cpp @@ -23,7 +23,9 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/TargetRegistry.h" + using namespace clang::driver; +using namespace clang::driver::tools; using namespace clang; using namespace llvm::opt; @@ -265,6 +267,44 @@ llvm_unreachable("Invalid tool kind."); } +static StringRef getArchNameForCompilerRTLib(const ToolChain &TC, + const ArgList &Args) { + const llvm::Triple &Triple = TC.getTriple(); + bool IsWindows = Triple.isOSWindows(); + + if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86) +return "i386"; + + if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb) +return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows) + ? "armhf" + : "arm"; + + return TC.getArchName(); +} + +SmallString<128> ToolChain::getCompilerRT(const ArgList &Args, +
[PATCH] D13340: Add support for the new mips-mti-linux toolchain.
vkalintiris created this revision. vkalintiris added reviewers: atanasyan, dsanders, rsmith. vkalintiris added a subscriber: cfe-commits. Herald added subscribers: dschuff, srhines, danalbert, tberghammer, jfb. This new toolchain uses primarily LLVM-based tools, eg. compiler-rt, lld, libcxx, etc. Because of this, it doesn't require neither an existing GCC installation nor a GNU environment. Ideally, in a follow-up patch we would like to add a new --{llvm|clang}-toolchain option (similar to --gcc-toolchain) in order to allow the use of this toolchain with independent Clang builds. For the time being, we use the --sysroot option just to test the correctness of the paths generated by the driver. http://reviews.llvm.org/D13340 Files: include/clang/Driver/ToolChain.h lib/Driver/Driver.cpp lib/Driver/ToolChain.cpp lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h lib/Driver/Tools.cpp test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mips-r2-hard-musl/lib/linux/libclang_rt.builtins-mips.a test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mips-r2-hard-musl/lib/linux/libclang_rt.builtins-mips.so test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mipsel-r2-hard-musl/lib/linux/libclang_rt.builtins-mipsel.a test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mipsel-r2-hard-musl/lib/linux/libclang_rt.builtins-mipsel.so test/Driver/Inputs/mips_mti_linux/sysroot/mips-r2-hard-musl/usr/lib/crt1.o test/Driver/Inputs/mips_mti_linux/sysroot/mips-r2-hard-musl/usr/lib/crti.o test/Driver/Inputs/mips_mti_linux/sysroot/mips-r2-hard-musl/usr/lib/crtn.o test/Driver/Inputs/mips_mti_linux/sysroot/mipsel-r2-hard-musl/usr/lib/crt1.o test/Driver/Inputs/mips_mti_linux/sysroot/mipsel-r2-hard-musl/usr/lib/crti.o test/Driver/Inputs/mips_mti_linux/sysroot/mipsel-r2-hard-musl/usr/lib/crtn.o test/Driver/mips-mti-linux.c Index: test/Driver/mips-mti-linux.c === --- /dev/null +++ test/Driver/mips-mti-linux.c @@ -0,0 +1,42 @@ +// Check frontend and linker invocations on GPL-free MIPS toolchain. +// +// FIXME: Using --sysroot with this toolchain/triple isn't supported. We use +//it here to test that we are producing the correct paths/flags. +//Ideally, we'd like to have an --llvm-toolchain option similar to +//the --gcc-toolchain one. + +// = Big-endian, mips32r2, hard float +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=mips-mti-linux -mips32r2 -mhard-float \ +// RUN: --sysroot=%S/Inputs/mips_mti_linux/sysroot \ +// RUN: | FileCheck --check-prefix=CHECK-BE-HF-32R2 %s +// +// CHECK-BE-HF-32R2: "{{.*}}clang" {{.*}} "-triple" "mips-mti-linux" +// CHECK-BE-HF-32R2-SAME: "-fuse-init-array" "-target-cpu" "mips32r2" +// CHECK-BE-HF-32R2-SAME: "-isysroot" "{{.*}}mips_mti_linux/sysroot" +// CHECK-BE-HF-32R2: "lld" "-flavor" "gnu" "-target" "mips-mti-linux" +// CHECK-BE-HF-32R2-SAME: "--sysroot=[[SYSROOT:[^"]+]]" {{.*}} "-dynamic-linker" "/lib/ld-musl-mips.so.1" +// CHECK-BE-HF-32R2-SAME: "[[SYSROOT]]/mips-r2-hard-musl/usr/lib/crt1.o" +// CHECK-BE-HF-32R2-SAME: "[[SYSROOT]]/mips-r2-hard-musl/usr/lib/crti.o" +// CHECK-BE-HF-32R2-SAME: "-L[[SYSROOT]]/mips-r2-hard-musl/usr/lib" +// CHECK-BE-HF-32R2-SAME: "{{[^"]+}}/mips-r2-hard-musl/lib/linux/libclang_rt.builtins-mips.a" +// CHECK-BE-HF-32R2-SAME: "-lc" +// CHECK-BE-HF-32R2-SAME: "[[SYSROOT]]/mips-r2-hard-musl/usr/lib/crtn.o" + +// = Little-endian, mips32r2, hard float +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=mips-mti-linux -mips32r2 -EL -mhard-float \ +// RUN: --sysroot=%S/Inputs/mips_mti_linux/sysroot \ +// RUN: | FileCheck --check-prefix=CHECK-LE-HF-32R2 %s +// +// CHECK-LE-HF-32R2: "{{.*}}clang" {{.*}} "-triple" "mipsel-mti-linux" +// CHECK-LE-HF-32R2-SAME: "-fuse-init-array" "-target-cpu" "mips32r2" +// CHECK-LE-HF-32R2-SAME: "-isysroot" "{{.*}}mips_mti_linux/sysroot" +// CHECK-LE-HF-32R2: "lld" "-flavor" "gnu" "-target" "mipsel-mti-linux" +// CHECK-LE-HF-32R2-SAME: "--sysroot=[[SYSROOT:[^"]+]]" {{.*}} "-dynamic-linker" "/lib/ld-musl-mipsel.so.1" +// CHECK-LE-HF-32R2-SAME: "[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib/crt1.o" +// CHECK-LE-HF-32R2-SAME: "[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib/crti.o" +// CHECK-LE-HF-32R2-SAME: "-L[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib" +// CHECK-LE-HF-32R2-SAME: "{{[^"]+}}/mipsel-r2-hard-musl/lib/linux/libclang_rt.builtins-mipsel.a" +// CHECK-LE-HF-32R2-SAME: "-lc" +// CHECK-LE-HF-32R2-SAME: "[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib/crtn.o" Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -4559,12 +4559,16 @@ } // -fuse-cxa-atexit is default. - if (!Args.hasFlag(options::OPT_fuse_cxa_atexit, -options::OPT_fno_use_cxa_atexit, -!IsWindowsCygnus && !IsWindowsGNU && -getToolChain().getTriple().getOS()
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
aaron.ballman added a subscriber: aaron.ballman. aaron.ballman added reviewers: aaron.ballman, alexfh. aaron.ballman added a comment. As a slightly more broad question: I think we should have a user-customizable way to categorize these checks so that you can enable/disable them with finer-grained control. Some of the existing checkers already cover the Cpp guidelines, and we'll likely be adding plenty more. There's quite likely overlap with Google and LLVM checkers, etc. It would be really nice if we had a way to say: -checks=-*, CppGuidelines or -checks=-*, CERT, etc. (I'm not suggesting this as part of this patch, but I think it is an idea we should consider exploring because style guidelines abound: the new C++ ones, MISRA, CERT, joint strike fighter, etc. User-customizable categorization would really help for this sort of thing. This would help assuage my issue with the checker being on by default in misc-* -- it could be off in misc-* but on in cppcoreguidelines-*, for instance.) ~Aaron Comment at: clang-tidy/misc/NoReinterpretCastCheck.cpp:29 @@ +28,3 @@ + Result.Nodes.getNodeAs("cast"); + diag(MatchedCast->getOperatorLoc(), + "do not use reinterpret_cast (C++ Core Guidelines, rule Type.1)"); I am worried about the amount of chattiness for this diagnostic and the fact that it does not provide the user with any idea as to what to do instead. The C-style cast checker will suggest that users don't use C-style casts, which suggests there's no way to appease this checker. The style guide suggests using variant, but that is not a workable solution for projects that don't have a variant type. FWIW, I've run into the same thing for CERT rules like: DCL50-CPP. Do not define a C-style variadic function MSC50-CPP. Do not use std::rand() for generating pseudorandom numbers (to a lesser extent, because exists now.) I don't have a particularly good solution to the issue though. I'm not opposed to the checker, but I am opposed to the checker being on by default for misc-* checkers. http://reviews.llvm.org/D13313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13342: Prevent loop-convert from leaving empty lines after removing an alias declaration.
angelgarcia created this revision. angelgarcia added a reviewer: klimek. angelgarcia added subscribers: alexfh, cfe-commits. This fixes https://llvm.org/bugs/show_bug.cgi?id=17716. http://reviews.llvm.org/D13342 Files: clang-tidy/modernize/LoopConvertCheck.cpp clang-tidy/modernize/LoopConvertCheck.h test/clang-tidy/modernize-loop-convert-extra.cpp Index: test/clang-tidy/modernize-loop-convert-extra.cpp === --- test/clang-tidy/modernize-loop-convert-extra.cpp +++ test/clang-tidy/modernize-loop-convert-extra.cpp @@ -53,7 +53,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : Arr) // CHECK-FIXES-NOT: Val &{{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; // The container was not only used to initialize a temporary loop variable for @@ -89,7 +89,7 @@ } // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : V) - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; // The same with a call to at() @@ -100,7 +100,7 @@ } // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : *Pv) - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; for (int I = 0; I < N; ++I) { @@ -166,8 +166,17 @@ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & Elem : IntArr) // CHECK-FIXES-NEXT: IntRef Int(Elem); -} + // Ensure that removing the alias doesn't leave empty lines behind. + for (int I = 0; I < N; ++I) { +auto &X = IntArr[I]; +X = 0; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto & X : IntArr) { + // CHECK-FIXES-NEXT: {{^X = 0;$}} + // CHECK-FIXES-NEXT: {{^ }$}} +} void refs_and_vals() { // The following tests check that the transform correctly preserves the @@ -186,7 +195,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Alias : S_const) // CHECK-FIXES-NOT: MutableVal {{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: Alias.X = 0; for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { @@ -197,7 +206,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Alias : Ss) // CHECK-FIXES-NOT: MutableVal {{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: Alias.X = 0; for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { @@ -208,7 +217,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & Alias : Ss) // CHECK-FIXES-NOT: MutableVal &{{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: Alias.X = 0; dependent Dep, Other; @@ -863,7 +872,7 @@ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto R5 : Arr) // CHECK-FIXES-NEXT: auto G5 = [&]() - // CHECK-FIXES: int J5 = 8 + R5; + // CHECK-FIXES-NEXT: int J5 = 8 + R5; // Alias by reference. for (int I = 0; I < N; ++I) { @@ -875,7 +884,7 @@ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & R6 : Arr) // CHECK-FIXES-NEXT: auto G6 = [&]() - // CHECK-FIXES: int J6 = -1 + R6; + // CHECK-FIXES-NEXT: int J6 = -1 + R6; } void iterators() { @@ -953,7 +962,8 @@ E Ee{ { { g( { Array[I] } ) } } }; } // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead - // CHECK-FIXES: int A{ Elem }; + // CHECK-FIXES: for (auto & Elem : Array) + // CHECK-FIXES-NEXT: int A{ Elem }; // CHECK-FIXES-NEXT: int B{ g(Elem) }; // CHECK-FIXES-NEXT: int C{ g( { Elem } ) }; // CHECK-FIXES-NEXT: D Dd{ { g( { Elem } ) } }; Index: clang-tidy/modernize/LoopConvertCheck.h === --- clang-tidy/modernize/LoopConvertCheck.h +++ clang-tidy/modernize/LoopConvertCheck.h @@ -34,6 +34,8 @@ std::string ContainerString; }; + void getAliasRange(SourceManager &SM, SourceRange &DeclRange); + void doConversion(ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer, const UsageResult &Usages, const DeclStmt *AliasDecl, bool AliasUseRequired, Index: clang-tidy/modernize/LoopConvertCheck.cpp === --- clang-tidy/modernize/LoopConvertCheck.cpp +++ clang-tidy/modernize/LoopConvertCheck.cpp @@ -442,6 +442,30 @@ Finder->addMatcher(makePseudoArrayLoopMatcher(), this); } +/// \brief Given the range of a sin
Re: [PATCH] D13342: Prevent loop-convert from leaving empty lines after removing an alias declaration.
klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:455-457 @@ +454,5 @@ +/// +/// FIXME: if 'next_instruction' is a closing brace ('}'), after the replacement +/// it will be over-indented. But then, who would declare an alias and do +/// nothing with it before it goes out of the scope? +void LoopConvertCheck::getAliasRange(SourceManager &SM, SourceRange &Range) { Replace this with something like: We need to delete a potential newline after the deleted alias, as clang-format will leave empty lines untouched. For all other formatting we rely on clang-format to fix it. http://reviews.llvm.org/D13342 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13342: Prevent loop-convert from leaving empty lines after removing an alias declaration.
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. apart from that LG http://reviews.llvm.org/D13342 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13342: Prevent loop-convert from leaving empty lines after removing an alias declaration.
angelgarcia updated this revision to Diff 36234. angelgarcia added a comment. Change a comment. http://reviews.llvm.org/D13342 Files: clang-tidy/modernize/LoopConvertCheck.cpp clang-tidy/modernize/LoopConvertCheck.h test/clang-tidy/modernize-loop-convert-extra.cpp Index: test/clang-tidy/modernize-loop-convert-extra.cpp === --- test/clang-tidy/modernize-loop-convert-extra.cpp +++ test/clang-tidy/modernize-loop-convert-extra.cpp @@ -53,7 +53,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : Arr) // CHECK-FIXES-NOT: Val &{{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; // The container was not only used to initialize a temporary loop variable for @@ -89,7 +89,7 @@ } // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : V) - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; // The same with a call to at() @@ -100,7 +100,7 @@ } // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : *Pv) - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; for (int I = 0; I < N; ++I) { @@ -166,8 +166,17 @@ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & Elem : IntArr) // CHECK-FIXES-NEXT: IntRef Int(Elem); -} + // Ensure that removing the alias doesn't leave empty lines behind. + for (int I = 0; I < N; ++I) { +auto &X = IntArr[I]; +X = 0; + } + // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto & X : IntArr) { + // CHECK-FIXES-NEXT: {{^X = 0;$}} + // CHECK-FIXES-NEXT: {{^ }$}} +} void refs_and_vals() { // The following tests check that the transform correctly preserves the @@ -186,7 +195,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Alias : S_const) // CHECK-FIXES-NOT: MutableVal {{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: Alias.X = 0; for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { @@ -197,7 +206,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto Alias : Ss) // CHECK-FIXES-NOT: MutableVal {{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: Alias.X = 0; for (S::iterator It = Ss.begin(), E = Ss.end(); It != E; ++It) { @@ -208,7 +217,7 @@ // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & Alias : Ss) // CHECK-FIXES-NOT: MutableVal &{{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: Alias.X = 0; dependent Dep, Other; @@ -863,7 +872,7 @@ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto R5 : Arr) // CHECK-FIXES-NEXT: auto G5 = [&]() - // CHECK-FIXES: int J5 = 8 + R5; + // CHECK-FIXES-NEXT: int J5 = 8 + R5; // Alias by reference. for (int I = 0; I < N; ++I) { @@ -875,7 +884,7 @@ // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & R6 : Arr) // CHECK-FIXES-NEXT: auto G6 = [&]() - // CHECK-FIXES: int J6 = -1 + R6; + // CHECK-FIXES-NEXT: int J6 = -1 + R6; } void iterators() { @@ -953,7 +962,8 @@ E Ee{ { { g( { Array[I] } ) } } }; } // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead - // CHECK-FIXES: int A{ Elem }; + // CHECK-FIXES: for (auto & Elem : Array) + // CHECK-FIXES-NEXT: int A{ Elem }; // CHECK-FIXES-NEXT: int B{ g(Elem) }; // CHECK-FIXES-NEXT: int C{ g( { Elem } ) }; // CHECK-FIXES-NEXT: D Dd{ { g( { Elem } ) } }; Index: clang-tidy/modernize/LoopConvertCheck.h === --- clang-tidy/modernize/LoopConvertCheck.h +++ clang-tidy/modernize/LoopConvertCheck.h @@ -34,6 +34,8 @@ std::string ContainerString; }; + void getAliasRange(SourceManager &SM, SourceRange &DeclRange); + void doConversion(ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer, const UsageResult &Usages, const DeclStmt *AliasDecl, bool AliasUseRequired, Index: clang-tidy/modernize/LoopConvertCheck.cpp === --- clang-tidy/modernize/LoopConvertCheck.cpp +++ clang-tidy/modernize/LoopConvertCheck.cpp @@ -442,6 +442,30 @@ Finder->addMatcher(makePseudoArrayLoopMatcher(), this); } +/// \brief Given the range of a single declaration, such as: +/// \code +/// unsigned &ThisIsADeclarationThatCanSpanS
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
alexfh added a comment. In http://reviews.llvm.org/D13313#257476, @aaron.ballman wrote: > As a slightly more broad question: I think we should have a user-customizable > way to categorize these checks so that you can enable/disable them with > finer-grained control. Some of the existing checkers already cover the Cpp > guidelines, and we'll likely be adding plenty more. There's quite likely > overlap with Google and LLVM checkers, etc. It would be really nice if we had > a way to say: -checks=-*, CppGuidelines or -checks=-*, CERT, etc. > > (I'm not suggesting this as part of this patch, but I think it is an idea we > should consider exploring because style guidelines abound: the new C++ ones, > MISRA, CERT, joint strike fighter, etc. User-customizable categorization > would really help for this sort of thing. This would help assuage my issue > with the checker being on by default in misc-* -- it could be off in misc-* > but on in cppcoreguidelines-*, for instance.) > > ~Aaron One way we could get CppCoreGuidelines checks available for easy enabling as a whole is to create a separate module and register all relevant checks there with names relevant to the CppCoreGuidelines (e.g. register the `clang::tidy::google::ExplicitConstructorCheck` there as "cppcoreguidelines-rc-explicit"). If a checks needs to be slightly modified in order to be closer to a specific rule, we might add some check options and configure proper defaults in the `CppCoreGuidelinesModule::getModuleOptions()`. If a larger change in behavior is needed, we could inherit from existing checks as well. http://reviews.llvm.org/D13313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r249005 - Revert "Decorating virtual functions load with invariant.load" and fix
Author: rengolin Date: Thu Oct 1 07:58:41 2015 New Revision: 249005 URL: http://llvm.org/viewvc/llvm-project?rev=249005&view=rev Log: Revert "Decorating virtual functions load with invariant.load" and fix This reverts commit r248982 as it was breaking the ARM buildbots and the fix didn't work. This reverts commit r248984, the fix that didn't work. Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=249005&r1=249004&r2=249005&view=diff == --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Thu Oct 1 07:58:41 2015 @@ -1609,16 +1609,7 @@ llvm::Value *ItaniumCXXABI::getVirtualFu uint64_t VTableIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(GD); llvm::Value *VFuncPtr = CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfn"); - auto *Inst = CGF.Builder.CreateAlignedLoad(VFuncPtr, CGF.getPointerAlign()); - - // It's safe to add "invariant.load" without -fstrict-vtable-pointers, but it - // would not help in devirtualization. - if (CGM.getCodeGenOpts().OptimizationLevel > 0 && - CGM.getCodeGenOpts().StrictVTablePointers) -Inst->setMetadata(llvm::LLVMContext::MD_invariant_load, - llvm::MDNode::get(CGM.getLLVMContext(), -llvm::ArrayRef())); - return Inst; + return CGF.Builder.CreateAlignedLoad(VFuncPtr, CGF.getPointerAlign()); } llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall( Modified: cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp?rev=249005&r1=249004&r2=249005&view=diff == --- cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp (original) +++ cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp Thu Oct 1 07:58:41 2015 @@ -1,5 +1,4 @@ // RUN: %clang_cc1 %s -triple %itanium_abi_triple -std=c++11 -emit-llvm -o - | FileCheck %s -// RUN: %clang_cc1 %s -triple %itanium_abi_triple -std=c++11 -emit-llvm -o - -fstrict-vtable-pointers -O1 | FileCheck --check-prefix=CHECK-INVARIANT %s // PR5021 namespace PR5021 { @@ -43,14 +42,10 @@ namespace VirtualNoreturn { [[noreturn]] virtual void f(); }; - // CHECK-LABEL: @_ZN15VirtualNoreturn1f - // CHECK-INVARIANT-LABEL: define void @_ZN15VirtualNoreturn1f + // CHECK: @_ZN15VirtualNoreturn1f void f(A *p) { p->f(); // CHECK: call {{.*}}void %{{[^#]*$}} // CHECK-NOT: unreachable -// CHECK-INVARIANT: load {{.*}} !invariant.load ![[EMPTY_NODE:[0-9]]] } } - -// CHECK-INVARIANT: ![[EMPTY_NODE]] = !{} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
alexfh added a comment. In http://reviews.llvm.org/D13313#256982, @vsk wrote: > The patch lgtm. Has there been a discussion on cfe-dev about adopting these > guidelines? > > I count well over 500 uses of reinterpret_cast in llvm+clang, so we might not > all be on the same page on this. I don't think LLVM can adopt a significant part of C++ Core Guidelines. Nevertheless, it makes sense to have these rules implemented as clang-tidy checks for the projects that decide to adopt them. http://reviews.llvm.org/D13313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r248984 - Test fix
Right, I reverted both commits on r249005. Please, let me know if you need help testing on ARM before the next commit. This looks like it could be tested on any 32-bit platform, though, so you should be able to get it passing on ARM if you test and make it pass on x86. On 1 October 2015 at 09:44, Renato Golin wrote: > On 1 October 2015 at 05:19, Piotr Padlewski via cfe-commits > wrote: >> Author: prazek >> Date: Wed Sep 30 23:19:45 2015 >> New Revision: 248984 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=248984&view=rev >> Log: >> Test fix > > Almost there... :) > > http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/6671/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Avirtual-function-calls.cpp ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r249006 - Prevent loop-convert from leaving empty lines after removing an alias declaration.
Author: angelgarcia Date: Thu Oct 1 08:08:21 2015 New Revision: 249006 URL: http://llvm.org/viewvc/llvm-project?rev=249006&view=rev Log: Prevent loop-convert from leaving empty lines after removing an alias declaration. Summary: This fixes https://llvm.org/bugs/show_bug.cgi?id=17716. Reviewers: klimek Subscribers: cfe-commits, alexfh Differential Revision: http://reviews.llvm.org/D13342 Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=249006&r1=249005&r2=249006&view=diff == --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Thu Oct 1 08:08:21 2015 @@ -442,6 +442,30 @@ void LoopConvertCheck::registerMatchers( Finder->addMatcher(makePseudoArrayLoopMatcher(), this); } +/// \brief Given the range of a single declaration, such as: +/// \code +/// unsigned &ThisIsADeclarationThatCanSpanSeveralLinesOfCode = +/// InitializationValues[I]; +/// next_instruction; +/// \endcode +/// Finds the range that has to be erased to remove this declaration without +/// leaving empty lines, by extending the range until the beginning of the +/// next instruction. +/// +/// We need to delete a potential newline after the deleted alias, as +/// clang-format will leave empty lines untouched. For all other formatting we +/// rely on clang-format to fix it. +void LoopConvertCheck::getAliasRange(SourceManager &SM, SourceRange &Range) { + bool Invalid = false; + const char *TextAfter = + SM.getCharacterData(Range.getEnd().getLocWithOffset(1), &Invalid); + if (Invalid) +return; + unsigned Offset = std::strspn(TextAfter, " \t\r\n"); + Range = + SourceRange(Range.getBegin(), Range.getEnd().getLocWithOffset(Offset)); +} + /// \brief Computes the changes needed to convert a given for loop, and /// applies them. void LoopConvertCheck::doConversion( @@ -460,7 +484,7 @@ void LoopConvertCheck::doConversion( AliasVarIsRef = AliasVar->getType()->isReferenceType(); // We keep along the entire DeclStmt to keep the correct range here. -const SourceRange &ReplaceRange = AliasDecl->getSourceRange(); +SourceRange ReplaceRange = AliasDecl->getSourceRange(); std::string ReplacementText; if (AliasUseRequired) { @@ -470,6 +494,9 @@ void LoopConvertCheck::doConversion( // in a for loop's init clause. Need to put this ';' back while removing // the declaration of the alias variable. This is probably a bug. ReplacementText = ";"; +} else { + // Avoid leaving empty lines or trailing whitespaces. + getAliasRange(Context->getSourceManager(), ReplaceRange); } Diag << FixItHint::CreateReplacement( Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h?rev=249006&r1=249005&r2=249006&view=diff == --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h Thu Oct 1 08:08:21 2015 @@ -34,6 +34,8 @@ private: std::string ContainerString; }; + void getAliasRange(SourceManager &SM, SourceRange &DeclRange); + void doConversion(ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer, const UsageResult &Usages, const DeclStmt *AliasDecl, bool AliasUseRequired, Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp?rev=249006&r1=249005&r2=249006&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-extra.cpp Thu Oct 1 08:08:21 2015 @@ -53,7 +53,7 @@ void aliasing() { // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & T : Arr) // CHECK-FIXES-NOT: Val &{{[a-z_]+}} = - // CHECK-FIXES: {} + // CHECK-FIXES-NEXT: {} // CHECK-FIXES-NEXT: int Y = T.X; // The container was not only used to initialize a temporary loop variable for @@ -89,7 +89,7 @@ void aliasing() { } // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES:
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
alexfh added a comment. A high-level comment: the "misc" module is the place for checks that we didn't find (or create) a better category for. Here it's clear that we need a separate category, so we need a `CppCoreGuidelinesModule` and the `cppcoreguidelines-` check prefix. I also suggest using anchor names (converted to lower case) in the document as check names, this one would be `cppcoreguidelines-pro-type-reinterpretcast`. Comment at: test/clang-tidy/misc-no-reinterpret-cast.cpp:7 @@ +6,1 @@ +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast (C++ Core Guidelines, rule Type.1) [misc-no-reinterpret-cast] \ No newline at end of file Please add a newline at the end of file to pacify various diff tools. http://reviews.llvm.org/D13313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13339: Allow a ToolChain to compute the path of a compiler-rt's component.
atanasyan accepted this revision. atanasyan added a comment. This revision is now accepted and ready to land. LGTM with a nit. Comment at: include/clang/Driver/ToolChain.h:253 @@ -252,1 +252,3 @@ + virtual SmallString<128> getCompilerRT(const llvm::opt::ArgList &Args, + StringRef Component, Return `std::string` for consistency with other methods. http://reviews.llvm.org/D13339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13340: Add support for the new mips-mti-linux toolchain.
atanasyan added inline comments. Comment at: include/clang/Driver/ToolChain.h:92 @@ -91,2 +91,3 @@ MultilibSet Multilibs; + Multilib SelectedMultilib; This field is used by the `MipsToolChain` class only. If so, move it to that class. Comment at: include/clang/Driver/ToolChain.h:147 @@ -145,1 +146,3 @@ + const Multilib &getSelectedMultilib() const { return SelectedMultilib; } + Do you need public access to this member function? Comment at: lib/Driver/Driver.cpp:2127 @@ +2126,3 @@ + // Allow the discovery of tools prefixed with LLVM's default target triple. + std::string LLVMDefaultTargetTriple = llvm::sys::getDefaultTargetTriple(); + if (LLVMDefaultTargetTriple != DefaultTargetTriple) Just curious when is DefaultTargetTriple not equal to LLVMDefaultTargetTriple? Comment at: lib/Driver/Driver.cpp:2225 @@ -2219,1 +2224,3 @@ TC = new toolchains::HexagonToolChain(*this, Target, Args); + else if (Target.getVendor() == llvm::Triple::MipsTechnologies) +TC = new toolchains::MipsToolChain(*this, Target, Args); The `mips-mti-linux-gnu` triple is used by Codescape toolchain too. After this change if user provides `-target mips-mti-linux-gnu` command line option, the `MipsToolChain` will be used. As far as I understand you have to put `GCCInstallation.isValid()` checking to the `MipsToolChain` class methods to allow working with both GNU and non-GNU toolchains. IMHO it does not make the code clear. Maybe use the `MipsToolChain` class for the non-GNU toolchain only. Comment at: lib/Driver/ToolChains.cpp:2210 @@ -2172,2 +2209,3 @@ const Generic_GCC::GCCVersion &V = GCCInstallation.getVersion(); + const llvm::Triple &TT = getTriple(); bool UseInitArrayDefault = Let's make this change by a separate commit to reduce number of unrelated changes. Comment at: lib/Driver/ToolChains.cpp:2231 @@ +2230,3 @@ + // If we did find a valid GCC installation, we don't have anything else to do. + if (GCCInstallation.isValid()) +return; When is GCCInstallation invalid in case of using this toolchain? Comment at: lib/Driver/ToolChains.cpp:2243 @@ +2242,3 @@ + tools::mips::getMipsCPUAndABI(Args, Triple, CPUName, ABIName); + LibSuffix = llvm::StringSwitch(ABIName) + .Case("o32", "") Similar code exists in the `getLinuxDynamicLinker` routine. Let's factor out it into the separate function say `tools::mips::getMipsABILibSuffix`. Comment at: lib/Driver/Tools.cpp:8115 @@ -8110,2 +8114,3 @@ const llvm::Triple::ArchType Arch = ToolChain.getArch(); + const llvm::Triple &TT = ToolChain.getTriple(); Let's make this change by a separate commit to reduce number of unrelated changes. http://reviews.llvm.org/D13340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13344: Keep the IfStmt node even if the condition is invalid
ogoffart created this revision. ogoffart added a reviewer: cfe-commits. This is quite useful for IDE's or other tools which would like to recover as much as possible even if there are a few errors in the code, and discarding the full 'if' bodies is unfortunate. http://reviews.llvm.org/D13344 Files: lib/Sema/SemaStmt.cpp test/Misc/ast-dump-invalid.cpp Index: test/Misc/ast-dump-invalid.cpp === --- test/Misc/ast-dump-invalid.cpp +++ test/Misc/ast-dump-invalid.cpp @@ -18,3 +18,26 @@ // CHECK-NEXT: `-CXXUnresolvedConstructExpr {{.*}} 'T' // CHECK-NEXT: |-DeclRefExpr {{.*}} 'T' lvalue ParmVar {{.*}} 'i' 'T' // CHECK-NEXT: `-DeclRefExpr {{.*}} 'T' lvalue ParmVar {{.*}} 'j' 'T' + + +namespace TestInvalidIf { +int g(int i) { + if (invalid_condition) +return 4; + else +return i; +} +} +// CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidIf +// CHECK-NEXT: `-FunctionDecl +// CHECK-NEXT: |-ParmVarDecl +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: `-IfStmt {{.*}} +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-ReturnStmt {{.*}} +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 4 +// CHECK-NEXT: `-ReturnStmt {{.*}} +// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' +// CHECK-NEXT: `-DeclRefExpr {{.*}} 'int' lvalue ParmVar {{.*}} 'i' 'int' + Index: lib/Sema/SemaStmt.cpp === --- lib/Sema/SemaStmt.cpp +++ lib/Sema/SemaStmt.cpp @@ -483,13 +483,6 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar, Stmt *thenStmt, SourceLocation ElseLoc, Stmt *elseStmt) { - // If the condition was invalid, discard the if statement. We could recover - // better by replacing it with a valid expr, but don't do that yet. - if (!CondVal.get() && !CondVar) { -getCurFunction()->setHasDroppedStmt(); -return StmtError(); - } - ExprResult CondResult(CondVal.release()); VarDecl *ConditionVar = nullptr; @@ -501,18 +494,17 @@ return StmtError(); } Expr *ConditionExpr = CondResult.getAs(); - if (!ConditionExpr) -return StmtError(); + if (ConditionExpr) { +DiagnoseUnusedExprResult(thenStmt); - DiagnoseUnusedExprResult(thenStmt); +if (!elseStmt) { + DiagnoseEmptyStmtBody(ConditionExpr->getLocEnd(), thenStmt, +diag::warn_empty_if_body); +} - if (!elseStmt) { -DiagnoseEmptyStmtBody(ConditionExpr->getLocEnd(), thenStmt, - diag::warn_empty_if_body); +DiagnoseUnusedExprResult(elseStmt); } - DiagnoseUnusedExprResult(elseStmt); - return new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr, thenStmt, ElseLoc, elseStmt); } Index: test/Misc/ast-dump-invalid.cpp === --- test/Misc/ast-dump-invalid.cpp +++ test/Misc/ast-dump-invalid.cpp @@ -18,3 +18,26 @@ // CHECK-NEXT: `-CXXUnresolvedConstructExpr {{.*}} 'T' // CHECK-NEXT: |-DeclRefExpr {{.*}} 'T' lvalue ParmVar {{.*}} 'i' 'T' // CHECK-NEXT: `-DeclRefExpr {{.*}} 'T' lvalue ParmVar {{.*}} 'j' 'T' + + +namespace TestInvalidIf { +int g(int i) { + if (invalid_condition) +return 4; + else +return i; +} +} +// CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidIf +// CHECK-NEXT: `-FunctionDecl +// CHECK-NEXT: |-ParmVarDecl +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: `-IfStmt {{.*}} +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-ReturnStmt {{.*}} +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 4 +// CHECK-NEXT: `-ReturnStmt {{.*}} +// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' +// CHECK-NEXT: `-DeclRefExpr {{.*}} 'int' lvalue ParmVar {{.*}} 'i' 'int' + Index: lib/Sema/SemaStmt.cpp === --- lib/Sema/SemaStmt.cpp +++ lib/Sema/SemaStmt.cpp @@ -483,13 +483,6 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar, Stmt *thenStmt, SourceLocation ElseLoc, Stmt *elseStmt) { - // If the condition was invalid, discard the if statement. We could recover - // better by replacing it with a valid expr, but don't do that yet. - if (!CondVal.get() && !CondVar) { -getCurFunction()->setHasDroppedStmt(); -return StmtError(); - } - ExprResult CondResult(CondVal.release()); VarDecl *ConditionVar = nullptr; @@ -501,18 +494,17 @@ return StmtError(); } Expr *ConditionExpr = CondResult.getAs(); - if (!ConditionExpr) -return StmtError(); + if (ConditionExpr) { +DiagnoseUnusedExprResult(thenStmt); - DiagnoseUnusedExprResult(thenStmt); +if (!elseStmt) { + DiagnoseEmptyStmtBody(Condi
r249009 - [PowerPC] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros on all PPC cores
Author: hfinkel Date: Thu Oct 1 08:39:49 2015 New Revision: 249009 URL: http://llvm.org/viewvc/llvm-project?rev=249009&view=rev Log: [PowerPC] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros on all PPC cores We support all __sync_val_compare_and_swap_* builtins (only 64-bit on 64-bit targets) on all cores, and should define the corresponding __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros, just as GCC does. As it turns out, this is really important because they're needed to prevent a bad ODR violation with libstdc++'s std::shared_ptr (this is well explained in PR12730). We were doing this only for P8, but this is necessary on all PPC systems. Modified: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/Preprocessor/predefined-arch-macros.c Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=249009&r1=249008&r2=249009&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Thu Oct 1 08:39:49 2015 @@ -1220,14 +1220,12 @@ void PPCTargetInfo::getTargetDefines(con Builder.defineMacro("__CRYPTO__"); if (HasHTM) Builder.defineMacro("__HTM__"); - if (getTriple().getArch() == llvm::Triple::ppc64le || - (defs & ArchDefinePwr8) || (CPU == "pwr8")) { -Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1"); -Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2"); -Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4"); -if (PointerWidth == 64) - Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8"); - } + + Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1"); + Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2"); + Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4"); + if (PointerWidth == 64) +Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8"); // FIXME: The following are not yet generated here by Clang, but are //generated by GCC: Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=249009&r1=249008&r2=249009&view=diff == --- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original) +++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Thu Oct 1 08:39:49 2015 @@ -1675,6 +1675,9 @@ // // CHECK_PPC_CRYPTO_M64: #define __CRYPTO__ // +// RUN: %clang -mcpu=ppc64 -E -dM %s -o - 2>&1 \ +// RUN: -target powerpc64-unknown-unknown \ +// RUN: | FileCheck %s -check-prefix=CHECK_PPC_GCC_ATOMICS // RUN: %clang -mcpu=pwr8 -E -dM %s -o - 2>&1 \ // RUN: -target powerpc64-unknown-unknown \ // RUN: | FileCheck %s -check-prefix=CHECK_PPC_GCC_ATOMICS ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13203: [Clang] - Massaging code to fix MSVS 2015 win32-release configuration
klimek added a subscriber: klimek. klimek added a comment. Not sure whether it makes sense to work around compiler bugs in CL. I assume this happens with clang from trunk? Is there a bug filed against CL? http://reviews.llvm.org/D13203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13346: Update clang-tidy documentation.
angelgarcia created this revision. angelgarcia added a reviewer: klimek. angelgarcia added subscribers: alexfh, cfe-commits. Improve modernize-use-auto documentation (https://llvm.org/bugs/show_bug.cgi?id=24962). Add documentation for modernize-make-unique. http://reviews.llvm.org/D13346 Files: docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/modernize-make-unique.rst docs/clang-tidy/checks/modernize-use-auto.rst Index: docs/clang-tidy/checks/modernize-use-auto.rst === --- docs/clang-tidy/checks/modernize-use-auto.rst +++ docs/clang-tidy/checks/modernize-use-auto.rst @@ -108,6 +108,39 @@ list. Otherwise, use of ``auto`` would cause the type of the variable to be deduced as``std::initializer_list``. +New expressions +--- + +Frequently, when a pointer is declared and initialized with ``new``, the +pointee type has to be written twice: in the declaration type and in the +``new`` expression. In this cases, the declaration type can be replaced with +``auto`` improving readability and maintainability. + +.. code-block:: c++ + + TypeName *my_pointer = new TypeName(my_param); + + // becomes + + auto my_pointer = new TypeName(my_param); + +The check will also replace the declaration type in multiple declarations, if +the following conditions are satisfied: + +* All declared variables have the same type (i.e. all of them are pointers to + the same type). +* All declared variables are initialized with a ``new`` expression. +* The types of all the new expressions are the same than the pointee of the + declaration type. + +.. code-block:: c++ + + TypeName *my_first_pointer = new TypeName, *my_second_pointer = new TypeName; + + // becomes + + auto my_first_pointer = new TypeName, my_second_pointer = new TypeName; + Known Limitations - * If the initializer is an explicit conversion constructor, the check will not Index: docs/clang-tidy/checks/modernize-make-unique.rst === --- /dev/null +++ docs/clang-tidy/checks/modernize-make-unique.rst @@ -0,0 +1,15 @@ +modernize-make-unique += + +This check finds the creation of ``std::unique_ptr`` objects by explicitly +calling the constructor and a ``new`` expression, and replaces it with a call +to ``std::make_unique``, introduced in C++14. + +.. code-block:: c++ + + auto my_ptr = std::unique_ptr(new MyPair(1, 2)); + + // becomes + + auto my_ptr = std::make_unique(1, 2); + Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -42,6 +42,7 @@ misc-unused-parameters misc-unused-raii modernize-loop-convert + modernize-make-unique modernize-pass-by-value modernize-replace-auto-ptr modernize-shrink-to-fit Index: docs/clang-tidy/checks/modernize-use-auto.rst === --- docs/clang-tidy/checks/modernize-use-auto.rst +++ docs/clang-tidy/checks/modernize-use-auto.rst @@ -108,6 +108,39 @@ list. Otherwise, use of ``auto`` would cause the type of the variable to be deduced as``std::initializer_list``. +New expressions +--- + +Frequently, when a pointer is declared and initialized with ``new``, the +pointee type has to be written twice: in the declaration type and in the +``new`` expression. In this cases, the declaration type can be replaced with +``auto`` improving readability and maintainability. + +.. code-block:: c++ + + TypeName *my_pointer = new TypeName(my_param); + + // becomes + + auto my_pointer = new TypeName(my_param); + +The check will also replace the declaration type in multiple declarations, if +the following conditions are satisfied: + +* All declared variables have the same type (i.e. all of them are pointers to + the same type). +* All declared variables are initialized with a ``new`` expression. +* The types of all the new expressions are the same than the pointee of the + declaration type. + +.. code-block:: c++ + + TypeName *my_first_pointer = new TypeName, *my_second_pointer = new TypeName; + + // becomes + + auto my_first_pointer = new TypeName, my_second_pointer = new TypeName; + Known Limitations - * If the initializer is an explicit conversion constructor, the check will not Index: docs/clang-tidy/checks/modernize-make-unique.rst === --- /dev/null +++ docs/clang-tidy/checks/modernize-make-unique.rst @@ -0,0 +1,15 @@ +modernize-make-unique += + +This check finds the creation of ``std::unique_ptr`` objects by explicitly +calling the constructor and a ``new`` expression, and replaces it with a call +to ``std::make_unique``, introduced in C++14. + +.. code-block:: c++ + + auto my_ptr = std::unique_ptr(new MyPair(1, 2));
Re: [PATCH] D13344: Keep the IfStmt node even if the condition is invalid
milianw added a comment. Is there still an error reported for the invalid condition? Anyhow, from my POV this is excellent and should fix a bunch of issues I've seen when using clang-c in KDevelop. I never got around to investigating it, but it always was related to conditionals. I'm pretty sure this patch fixes it then. @ogoffart: For Manuel to see the difference, could you share the before/after screenshots of your code browser? That makes the impact of this patch pretty clear. http://reviews.llvm.org/D13344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13304: Avoid inlining in throw statement
mcrosier added a comment. FWIW, a llvm based solution was discussed in http://reviews.llvm.org/D12979, but the clang solution is obviously more robust and easier to implement. Comment at: lib/CodeGen/CodeGenFunction.h:287 @@ +286,3 @@ + // True if the current insertion point is in cold regions (e.g., exception + // handling regions). As of now, this flag is ture only when handling throw + // statement. ture -> true http://reviews.llvm.org/D13304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13346: Update clang-tidy documentation.
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG http://reviews.llvm.org/D13346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r249017 - Update clang-tidy documentation.
Author: angelgarcia Date: Thu Oct 1 09:50:40 2015 New Revision: 249017 URL: http://llvm.org/viewvc/llvm-project?rev=249017&view=rev Log: Update clang-tidy documentation. Summary: Improve modernize-use-auto documentation (https://llvm.org/bugs/show_bug.cgi?id=24962). Add documentation for modernize-make-unique. Reviewers: klimek Subscribers: cfe-commits, alexfh Differential Revision: http://reviews.llvm.org/D13346 Added: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=249017&r1=249016&r2=249017&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Oct 1 09:50:40 2015 @@ -42,6 +42,7 @@ List of clang-tidy Checks misc-unused-parameters misc-unused-raii modernize-loop-convert + modernize-make-unique modernize-pass-by-value modernize-replace-auto-ptr modernize-shrink-to-fit Added: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst?rev=249017&view=auto == --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst (added) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-make-unique.rst Thu Oct 1 09:50:40 2015 @@ -0,0 +1,15 @@ +modernize-make-unique += + +This check finds the creation of ``std::unique_ptr`` objects by explicitly +calling the constructor and a ``new`` expression, and replaces it with a call +to ``std::make_unique``, introduced in C++14. + +.. code-block:: c++ + + auto my_ptr = std::unique_ptr(new MyPair(1, 2)); + + // becomes + + auto my_ptr = std::make_unique(1, 2); + Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst?rev=249017&r1=249016&r2=249017&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-auto.rst Thu Oct 1 09:50:40 2015 @@ -108,6 +108,39 @@ conditions are satisfied: list. Otherwise, use of ``auto`` would cause the type of the variable to be deduced as``std::initializer_list``. +New expressions +--- + +Frequently, when a pointer is declared and initialized with ``new``, the +pointee type has to be written twice: in the declaration type and in the +``new`` expression. In this cases, the declaration type can be replaced with +``auto`` improving readability and maintainability. + +.. code-block:: c++ + + TypeName *my_pointer = new TypeName(my_param); + + // becomes + + auto my_pointer = new TypeName(my_param); + +The check will also replace the declaration type in multiple declarations, if +the following conditions are satisfied: + +* All declared variables have the same type (i.e. all of them are pointers to + the same type). +* All declared variables are initialized with a ``new`` expression. +* The types of all the new expressions are the same than the pointee of the + declaration type. + +.. code-block:: c++ + + TypeName *my_first_pointer = new TypeName, *my_second_pointer = new TypeName; + + // becomes + + auto my_first_pointer = new TypeName, my_second_pointer = new TypeName; + Known Limitations - * If the initializer is an explicit conversion constructor, the check will not ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13203: [Clang] - Massaging code to fix MSVS 2015 win32-release configuration
yaron.keren added a subscriber: yaron.keren. yaron.keren added a comment. clang can't generate Visual C++ debug info except line numbers yet, so building with VC is essential to development on Windows. While most people and our bots are still using VC 2013 going forward supporting VC 2015 (even with bugs) is worth a small patch. http://reviews.llvm.org/D13203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13344: Keep the IfStmt node even if the condition is invalid
klimek added a reviewer: rsmith. klimek added a comment. +Richard, whom we need to validate AST changes to make sure they're benign. http://reviews.llvm.org/D13344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r249019 - [VFS] Remove unused setters. NFC.
Author: d0k Date: Thu Oct 1 10:02:15 2015 New Revision: 249019 URL: http://llvm.org/viewvc/llvm-project?rev=249019&view=rev Log: [VFS] Remove unused setters. NFC. Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=249019&r1=249018&r2=249019&view=diff == --- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original) +++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Thu Oct 1 10:02:15 2015 @@ -63,8 +63,6 @@ public: uint32_t getUser() const { return User; } uint32_t getGroup() const { return Group; } uint64_t getSize() const { return Size; } - void setType(llvm::sys::fs::file_type v) { Type = v; } - void setPermissions(llvm::sys::fs::perms p) { Perms = p; } /// @} /// @name Status queries /// These are static queries in llvm::sys::fs. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13330: Implement __attribute__((unique_instantiation))
aaron.ballman added a subscriber: aaron.ballman. aaron.ballman added reviewers: rsmith, aaron.ballman. Comment at: include/clang/Basic/Attr.td:1462 @@ +1461,3 @@ +def UniqueInstantiation : InheritableAttr { + let Spellings = [GCC<"unique_instantiation">]; + let Subjects = SubjectList<[CXXRecord]>; This is not a GCC attribute, so it should not be spelled as such. Since this only applies to C++ code, I would recommend a C++11 spelling (in the clang namespace). If you think this is something C++03 and earlier code would really benefit from, then you could also add a GNU-style spelling. Comment at: include/clang/Basic/Attr.td:1464 @@ +1463,3 @@ + let Subjects = SubjectList<[CXXRecord]>; + let Documentation = [Undocumented]; +} Please, no undocumented attributes. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2443 @@ -2442,1 +2442,3 @@ ":must be between 1 and %2}2">; +def err_unique_instantiation_wrong_decl : Error< + "unique_instantiation attribute on something that is not a explicit template declaration or instantiation.">; Would this make more sense as an option in warn_attribute_wrong_decl_type instead? (there's an err_ version as well if you wish to keep this an error). Also, please ensure that the attribute is quoted in the diagnostic -- it makes things less confusing for the user. Comment at: lib/AST/ASTContext.cpp:8244 @@ -8242,2 +8243,3 @@ case TSK_ExplicitInstantiationDefinition: -return GVA_StrongODR; +CTSD = dyn_cast(FD->getDeclContext()); +if (!CTSD || !CTSD->hasAttr()) I think this would be easier to read (and not have to hoist a declaration out of the switch) as: ``` if (const auto *CTSD = dyn_cast<>()) { if (CTSD->hasAttr<>()) return GVA_StrongExternal; } return GVA_StrongODR; ``` Comment at: lib/AST/ASTContext.cpp:8353 @@ -8342,2 +8352,3 @@ case TSK_ExplicitInstantiationDefinition: -return GVA_StrongODR; +CTSD = dyn_cast(VD->getDeclContext()); +if (!CTSD || !CTSD->hasAttr()) Similar suggestion here. Comment at: lib/Sema/SemaDeclAttr.cpp:4535 @@ +4534,3 @@ + const AttributeList &Attr) { + ClassTemplateSpecializationDecl *CTSD; + if ((CTSD = dyn_cast(D))) { The declaration does not need to be hoisted here. Comment at: lib/Sema/SemaDeclAttr.cpp:4539 @@ +4538,3 @@ +// by an ExplicitInstantiationDeclaration. +if (CTSD->getSpecializationKind() == TSK_ExplicitInstantiationDefinition) { + if (!CTSD->getPreviousDecl()) Why is this required as part of the feature design? It seems restrictive. Comment at: lib/Sema/SemaDeclAttr.cpp:4546 @@ +4545,3 @@ +// have performed the same check on the previous declaration here. +CXXRecordDecl *Previous = CTSD->getPreviousDecl(); +if (Previous) { Is this something that can be handled by mergeDeclAttribute()? I'm not certain how that interplays with templates specifically, but usually we do this sort of logic within a Sema::mergeFooAttr() function. Repository: rL LLVM http://reviews.llvm.org/D13330 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
aaron.ballman added a comment. In http://reviews.llvm.org/D13313#257506, @alexfh wrote: > In http://reviews.llvm.org/D13313#257476, @aaron.ballman wrote: > > > As a slightly more broad question: I think we should have a > > user-customizable way to categorize these checks so that you can > > enable/disable them with finer-grained control. Some of the existing > > checkers already cover the Cpp guidelines, and we'll likely be adding > > plenty more. There's quite likely overlap with Google and LLVM checkers, > > etc. It would be really nice if we had a way to say: -checks=-*, > > CppGuidelines or -checks=-*, CERT, etc. > > > > (I'm not suggesting this as part of this patch, but I think it is an idea > > we should consider exploring because style guidelines abound: the new C++ > > ones, MISRA, CERT, joint strike fighter, etc. User-customizable > > categorization would really help for this sort of thing. This would help > > assuage my issue with the checker being on by default in misc-* -- it could > > be off in misc-* but on in cppcoreguidelines-*, for instance.) > > > > ~Aaron > > > One way we could get CppCoreGuidelines checks available for easy enabling as > a whole is to create a separate module and register all relevant checks there > with names relevant to the CppCoreGuidelines (e.g. register the > `clang::tidy::google::ExplicitConstructorCheck` there as > "cppcoreguidelines-rc-explicit"). If a checks needs to be slightly modified > in order to be closer to a specific rule, we might add some check options and > configure proper defaults in the > `CppCoreGuidelinesModule::getModuleOptions()`. If a larger change in behavior > is needed, we could inherit from existing checks as well. That's a good idea that I hadn't considered before. I may give this a shot for the CERT checks as well. Thank you for the suggestion! ~Aaron http://reviews.llvm.org/D13313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13318: Add a utility function to add target information to a command line
echristo added a comment. BTW as Manuel is happy it's fine with me. Still curious though. http://reviews.llvm.org/D13318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13336: [MSVC] 'property' with an empty array in array subscript expression.
rnk added a comment. I think fundamentally we are doing too much declspec property lowering in Sema. We might want to back up and figure out how to do it in IRGen. Right now we have bugs like this, which are probably more important than new functionality: https://llvm.org/bugs/show_bug.cgi?id=24132 http://reviews.llvm.org/D13336 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13349: Casting boolean to an integer vector in OpenCL should set all bits if boolean is true
neil.hickey created this revision. neil.hickey added a reviewer: cfe-commits. Changing behaviour of casting a true boolean to an integer vector for OpenCL. The spec (6.2.2) states that if the boolean is true, every bit in the result vector should be set. This change will treat the i1 value as signed for the purposes of performing the cast to integer, and therefore sign extend into the result. http://reviews.llvm.org/D13349 Files: lib/CodeGen/CGExprScalar.cpp test/CodeGenOpenCL/bool_cast.cl Index: test/CodeGenOpenCL/bool_cast.cl === --- /dev/null +++ test/CodeGenOpenCL/bool_cast.cl @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 %s -emit-llvm -o - -O0 | FileCheck %s + +typedef unsigned char uchar4 __attribute((ext_vector_type(4))); +typedef unsigned int int4 __attribute((ext_vector_type(4))); + +void kernel ker() { + bool t = true; + int4 vec4 = (int4)t; +// CHECK: {{%.*}} = load i8, i8* %t, align 1 +// CHECK: {{%.*}} = trunc i8 {{%.*}} to i1 +// CHECK: {{%.*}} = sext i1 {{%.*}} to i32 +// CHECK: {{%.*}} = insertelement <4 x i32> undef, i32 {{%.*}}, i32 0 +// CHECK: {{%.*}} = shufflevector <4 x i32> {{%.*}}, <4 x i32> undef, <4 x i32> zeroinitializer +// CHECK: store <4 x i32> {{%.*}}, <4 x i32>* %vec4, align 16 + int i = (int)t; +// CHECK: {{%.*}} = load i8, i8* %t, align 1 +// CHECK: {{%.*}} = trunc i8 {{%.*}} to i1 +// CHECK: {{%.*}} = zext i1 {{%.*}} to i32 +// CHECK: store i32 {{%.*}}, i32* %i, align 4 + + uchar4 vc; + vc = (uchar4)true; +// CHECK: store <4 x i8> , <4 x i8>* %vc, align 4 + unsigned char c; + c = (unsigned char)true; +// CHECK: store i8 1, i8* %c, align 1 +} Index: lib/CodeGen/CGExprScalar.cpp === --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -151,6 +151,9 @@ Value *EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy, SourceLocation Loc); + Value *EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy, + SourceLocation Loc, bool TreatBooleanAsSigned); + /// Emit a conversion from the specified complex type to the specified /// destination type, where the destination type is an LLVM scalar type. Value *EmitComplexToScalarConversion(CodeGenFunction::ComplexPairTy Src, @@ -733,6 +736,13 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType, QualType DstType, SourceLocation Loc) { + return EmitScalarConversion(Src, SrcType, DstType, Loc, false); +} + +Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType, + QualType DstType, + SourceLocation Loc, + bool TreatBooleanAsSigned) { SrcType = CGF.getContext().getCanonicalType(SrcType); DstType = CGF.getContext().getCanonicalType(DstType); if (SrcType == DstType) return Src; @@ -807,7 +817,7 @@ if (DstType->isExtVectorType() && !SrcType->isVectorType()) { // Cast the scalar to element type QualType EltTy = DstType->getAs()->getElementType(); -llvm::Value *Elt = EmitScalarConversion(Src, SrcType, EltTy, Loc); +llvm::Value *Elt = EmitScalarConversion(Src, SrcType, EltTy, Loc, CGF.getContext().getLangOpts().OpenCL); // Splat the element across to all elements unsigned NumElements = cast(DstTy)->getNumElements(); @@ -847,6 +857,9 @@ if (isa(SrcTy)) { bool InputSigned = SrcType->isSignedIntegerOrEnumerationType(); +if (SrcType->isBooleanType() && TreatBooleanAsSigned) { + InputSigned = true; +} if (isa(DstTy)) Res = Builder.CreateIntCast(Src, DstTy, InputSigned, "conv"); else if (InputSigned) @@ -1531,10 +1544,14 @@ } case CK_VectorSplat: { llvm::Type *DstTy = ConvertType(DestTy); -Value *Elt = Visit(const_cast(E)); -Elt = EmitScalarConversion(Elt, E->getType(), +// Need an IgnoreImpCasts here as by default a boolean will be promoted to +// an int, which will not perform the sign extension, so if we know we are +// going to cast to a vector we have to strip the implicit cast off. +Value *Elt = Visit(const_cast(E->IgnoreImpCasts())); +Elt = EmitScalarConversion(Elt, E->IgnoreImpCasts()->getType(), DestTy->getAs()->getElementType(), - CE->getExprLoc()); + CE->getExprLoc(), + CGF.getContext().getLangOpts().OpenCL); // Splat the element across to all elements unsigned NumElements = cast(DstTy)->getNumElements(); Index: test/CodeGenOpenCL/bool_cast.cl === --- /dev/null +++ test/CodeGenOpenCL/bool_cast.cl @@ -0,0
Re: [PATCH] D13100: [mips] Separated mips specific -Wa options, so that they are not checked on other platforms.
s.egerton updated this revision to Diff 36252. s.egerton added a comment. Responded to comments made on the mailing list. http://reviews.llvm.org/D13100 Files: lib/Driver/Tools.cpp Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -2351,10 +2351,35 @@ continue; } - bool IsMips = C.getDefaultToolChain().getArch() == llvm::Triple::mips || -C.getDefaultToolChain().getArch() == llvm::Triple::mipsel || -C.getDefaultToolChain().getArch() == llvm::Triple::mips64 || -C.getDefaultToolChain().getArch() == llvm::Triple::mips64el; + switch (C.getDefaultToolChain().getArch()) { + default: +break; + case llvm::Triple::mips: + case llvm::Triple::mipsel: + case llvm::Triple::mips64: + case llvm::Triple::mips64el: +if (Value == "--trap") { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("+use-tcc-in-div"); + continue; +} +if (Value == "--break") { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("-use-tcc-in-div"); + continue; +} +if (Value.startswith("-msoft-float")) { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("+soft-float"); + continue; +} +if (Value.startswith("-mhard-float")) { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("-soft-float"); + continue; +} +break; + } if (Value == "-force_cpusubtype_ALL") { // Do nothing, this is the default and we don't support anything else. @@ -2381,18 +2406,6 @@ } else if (Value.startswith("-mcpu") || Value.startswith("-mfpu") || Value.startswith("-mhwdiv") || Value.startswith("-march")) { // Do nothing, we'll validate it later. - } else if (IsMips && Value == "--trap") { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("+use-tcc-in-div"); - } else if (IsMips && Value == "--break") { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("-use-tcc-in-div"); - } else if (IsMips && Value.startswith("-msoft-float")) { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("+soft-float"); - } else if (IsMips && Value.startswith("-mhard-float")) { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("-soft-float"); } else { D.Diag(diag::err_drv_unsupported_option_argument) << A->getOption().getName() << Value; Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -2351,10 +2351,35 @@ continue; } - bool IsMips = C.getDefaultToolChain().getArch() == llvm::Triple::mips || -C.getDefaultToolChain().getArch() == llvm::Triple::mipsel || -C.getDefaultToolChain().getArch() == llvm::Triple::mips64 || -C.getDefaultToolChain().getArch() == llvm::Triple::mips64el; + switch (C.getDefaultToolChain().getArch()) { + default: +break; + case llvm::Triple::mips: + case llvm::Triple::mipsel: + case llvm::Triple::mips64: + case llvm::Triple::mips64el: +if (Value == "--trap") { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("+use-tcc-in-div"); + continue; +} +if (Value == "--break") { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("-use-tcc-in-div"); + continue; +} +if (Value.startswith("-msoft-float")) { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("+soft-float"); + continue; +} +if (Value.startswith("-mhard-float")) { + CmdArgs.push_back("-target-feature"); + CmdArgs.push_back("-soft-float"); + continue; +} +break; + } if (Value == "-force_cpusubtype_ALL") { // Do nothing, this is the default and we don't support anything else. @@ -2381,18 +2406,6 @@ } else if (Value.startswith("-mcpu") || Value.startswith("-mfpu") || Value.startswith("-mhwdiv") || Value.startswith("-march")) { // Do nothing, we'll validate it later. - } else if (IsMips && Value == "--trap") { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("+use-tcc-in-div"); - } else if (IsMips && Value == "--break") { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("-use-tcc-in-div"); - } else if (IsMips && Value.startswith("-msoft-float")) { -CmdArgs.push_back("-target-feature"); -CmdArgs.push_back("+soft-float"); - } else if (IsMips && Valu
Re: [PATCH] D13340: Add support for the new mips-mti-linux toolchain.
vkalintiris updated this revision to Diff 36257. vkalintiris marked 8 inline comments as done. vkalintiris added a comment. Thanks. The review comments are addressed in this update. http://reviews.llvm.org/D13340 Files: lib/Driver/Driver.cpp lib/Driver/ToolChain.cpp lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h lib/Driver/Tools.cpp lib/Driver/Tools.h test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mips-r2-hard-musl/lib/linux/libclang_rt.builtins-mips.a test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mips-r2-hard-musl/lib/linux/libclang_rt.builtins-mips.so test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mipsel-r2-hard-musl/lib/linux/libclang_rt.builtins-mipsel.a test/Driver/Inputs/mips_mti_linux/lib/clang/3.8.0/mipsel-r2-hard-musl/lib/linux/libclang_rt.builtins-mipsel.so test/Driver/Inputs/mips_mti_linux/sysroot/mips-r2-hard-musl/usr/lib/crt1.o test/Driver/Inputs/mips_mti_linux/sysroot/mips-r2-hard-musl/usr/lib/crti.o test/Driver/Inputs/mips_mti_linux/sysroot/mips-r2-hard-musl/usr/lib/crtn.o test/Driver/Inputs/mips_mti_linux/sysroot/mipsel-r2-hard-musl/usr/lib/crt1.o test/Driver/Inputs/mips_mti_linux/sysroot/mipsel-r2-hard-musl/usr/lib/crti.o test/Driver/Inputs/mips_mti_linux/sysroot/mipsel-r2-hard-musl/usr/lib/crtn.o test/Driver/mips-mti-linux.c Index: test/Driver/mips-mti-linux.c === --- /dev/null +++ test/Driver/mips-mti-linux.c @@ -0,0 +1,42 @@ +// Check frontend and linker invocations on GPL-free MIPS toolchain. +// +// FIXME: Using --sysroot with this toolchain/triple isn't supported. We use +//it here to test that we are producing the correct paths/flags. +//Ideally, we'd like to have an --llvm-toolchain option similar to +//the --gcc-toolchain one. + +// = Big-endian, mips32r2, hard float +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=mips-mti-linux -mips32r2 -mhard-float \ +// RUN: --sysroot=%S/Inputs/mips_mti_linux/sysroot \ +// RUN: | FileCheck --check-prefix=CHECK-BE-HF-32R2 %s +// +// CHECK-BE-HF-32R2: "{{.*}}clang" {{.*}} "-triple" "mips-mti-linux" +// CHECK-BE-HF-32R2-SAME: "-fuse-init-array" "-target-cpu" "mips32r2" +// CHECK-BE-HF-32R2-SAME: "-isysroot" "{{.*}}mips_mti_linux/sysroot" +// CHECK-BE-HF-32R2: "lld" "-flavor" "gnu" "-target" "mips-mti-linux" +// CHECK-BE-HF-32R2-SAME: "--sysroot=[[SYSROOT:[^"]+]]" {{.*}} "-dynamic-linker" "/lib/ld-musl-mips.so.1" +// CHECK-BE-HF-32R2-SAME: "[[SYSROOT]]/mips-r2-hard-musl/usr/lib/crt1.o" +// CHECK-BE-HF-32R2-SAME: "[[SYSROOT]]/mips-r2-hard-musl/usr/lib/crti.o" +// CHECK-BE-HF-32R2-SAME: "-L[[SYSROOT]]/mips-r2-hard-musl/usr/lib" +// CHECK-BE-HF-32R2-SAME: "{{[^"]+}}/mips-r2-hard-musl/lib/linux/libclang_rt.builtins-mips.a" +// CHECK-BE-HF-32R2-SAME: "-lc" +// CHECK-BE-HF-32R2-SAME: "[[SYSROOT]]/mips-r2-hard-musl/usr/lib/crtn.o" + +// = Little-endian, mips32r2, hard float +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=mips-mti-linux -mips32r2 -EL -mhard-float \ +// RUN: --sysroot=%S/Inputs/mips_mti_linux/sysroot \ +// RUN: | FileCheck --check-prefix=CHECK-LE-HF-32R2 %s +// +// CHECK-LE-HF-32R2: "{{.*}}clang" {{.*}} "-triple" "mipsel-mti-linux" +// CHECK-LE-HF-32R2-SAME: "-fuse-init-array" "-target-cpu" "mips32r2" +// CHECK-LE-HF-32R2-SAME: "-isysroot" "{{.*}}mips_mti_linux/sysroot" +// CHECK-LE-HF-32R2: "lld" "-flavor" "gnu" "-target" "mipsel-mti-linux" +// CHECK-LE-HF-32R2-SAME: "--sysroot=[[SYSROOT:[^"]+]]" {{.*}} "-dynamic-linker" "/lib/ld-musl-mipsel.so.1" +// CHECK-LE-HF-32R2-SAME: "[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib/crt1.o" +// CHECK-LE-HF-32R2-SAME: "[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib/crti.o" +// CHECK-LE-HF-32R2-SAME: "-L[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib" +// CHECK-LE-HF-32R2-SAME: "{{[^"]+}}/mipsel-r2-hard-musl/lib/linux/libclang_rt.builtins-mipsel.a" +// CHECK-LE-HF-32R2-SAME: "-lc" +// CHECK-LE-HF-32R2-SAME: "[[SYSROOT]]/mipsel-r2-hard-musl/usr/lib/crtn.o" Index: lib/Driver/Tools.h === --- lib/Driver/Tools.h +++ lib/Driver/Tools.h @@ -267,6 +267,8 @@ void getMipsCPUAndABI(const llvm::opt::ArgList &Args, const llvm::Triple &Triple, StringRef &CPUName, StringRef &ABIName); +std::string getMipsABILibSuffix(const llvm::opt::ArgList &Args, +const llvm::Triple &Triple); bool hasMipsAbiArg(const llvm::opt::ArgList &Args, const char *Value); bool isUCLibc(const llvm::opt::ArgList &Args); bool isNaN2008(const llvm::opt::ArgList &Args, const llvm::Triple &Triple); Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -1077,6 +1077,16 @@ // FIXME: Warn on inconsistent use of -march and -mabi. } +std::string mips::getMipsABILibSuffix(const ArgList &Args, +
Re: [PATCH] D13340: Add support for the new mips-mti-linux toolchain.
vkalintiris added inline comments. Comment at: include/clang/Driver/ToolChain.h:147 @@ -145,1 +146,3 @@ + const Multilib &getSelectedMultilib() const { return SelectedMultilib; } + atanasyan wrote: > Do you need public access to this member function? I discarded this member function after the move of `SelectedMultilib` to the `MipsToolChain` class. Comment at: lib/Driver/Driver.cpp:2127 @@ +2126,3 @@ + // Allow the discovery of tools prefixed with LLVM's default target triple. + std::string LLVMDefaultTargetTriple = llvm::sys::getDefaultTargetTriple(); + if (LLVMDefaultTargetTriple != DefaultTargetTriple) atanasyan wrote: > Just curious when is DefaultTargetTriple not equal to LLVMDefaultTargetTriple? In this case, `DefaultTargetTriple` is the triple specified with `--target=`. LLVMDefaultTargetTriple is the value specified with the CMake variable `-DLLVM_DEFAULT_TARGET_TRIPLE=` during configuration time. For example, using `--target=mips64el-mti-linux` will search for files prefixed with either `mips64el-mti-linux-{as,ld}` and `mips-mti-linux-{as,ld}` in our toolchain where we specify `-DLLVM_DEFAULT_TARGET_TRIPLE=mips-mti-linux.` Comment at: lib/Driver/Driver.cpp:2225 @@ -2219,1 +2224,3 @@ TC = new toolchains::HexagonToolChain(*this, Target, Args); + else if (Target.getVendor() == llvm::Triple::MipsTechnologies) +TC = new toolchains::MipsToolChain(*this, Target, Args); atanasyan wrote: > The `mips-mti-linux-gnu` triple is used by Codescape toolchain too. After > this change if user provides `-target mips-mti-linux-gnu` command line > option, the `MipsToolChain` will be used. As far as I understand you have to > put `GCCInstallation.isValid()` checking to the `MipsToolChain` class methods > to allow working with both GNU and non-GNU toolchains. IMHO it does not make > the code clear. Maybe use the `MipsToolChain` class for the non-GNU toolchain > only. Shall we change the `MipsToolChain` class name to reflect these changes? Maybe `MipsNonGNUToolChain`? It would be strange to keep the same name, because most of the MIPS toolchains wouldn't be instantiated by `MipsToolChain`. Comment at: lib/Driver/ToolChains.cpp:2231 @@ +2230,3 @@ + // If we did find a valid GCC installation, we don't have anything else to do. + if (GCCInstallation.isValid()) +return; atanasyan wrote: > When is GCCInstallation invalid in case of using this toolchain? I changed that, according to your comments earlier. Now we instantiate this class only for triples that don't have an environment. http://reviews.llvm.org/D13340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13203: [Clang] - Massaging code to fix MSVS 2015 win32-release configuration
majnemer added a subscriber: majnemer. majnemer added a comment. In http://reviews.llvm.org/D13203#257539, @klimek wrote: > Not sure whether it makes sense to work around compiler bugs in CL. I assume > this happens with clang from trunk? Is there a bug filed against CL? We do this with some regularity for MSVC and GCC. Comment at: ASTContext.cpp:367-369 @@ -366,4 +366,5 @@ OriginalDeclForRC = I; - RawCommentAndCacheFlags Raw; + RedeclComments[I] = RawCommentAndCacheFlags(); + RawCommentAndCacheFlags &Raw = RedeclComments[I]; if (RC) { Raw.setRaw(RC); We need a comment detailing that this is a workaround for MSVC 2015 so that it can be removed when we drop support for 2015. http://reviews.llvm.org/D13203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13276: Don't adjust field offsets for external record layouts
majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. LGTM with a test added, see test/CodeGenCXX/override-layout.cpp for an example of how to test external record layout. http://reviews.llvm.org/D13276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13351: [Power PC] add soft float support for ppc32
spetrovic created this revision. spetrovic added a reviewer: hfinkel. spetrovic added a subscriber: cfe-commits. This patch enables soft float support for ppc32 architecture and fixes the ABI for variadic functions. This is first in set of patches for soft float support in LLVM. http://reviews.llvm.org/D13351 Files: include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/CodeGen/TargetInfo.cpp lib/Driver/Tools.cpp lib/Driver/Tools.h test/CodeGen/ppc-sfvarargs.c test/Driver/ppc-features.cpp Index: test/Driver/ppc-features.cpp === --- test/Driver/ppc-features.cpp +++ test/Driver/ppc-features.cpp @@ -12,6 +12,10 @@ // RUN: not %clang -target mips64-linux-gnu -faltivec -fsyntax-only %s 2>&1 | FileCheck %s // RUN: not %clang -target sparc-unknown-solaris -faltivec -fsyntax-only %s 2>&1 | FileCheck %s +// check soft float option only for ppc32 +// RUN: %clang -target powerpc-unknown-linux-gnu %s -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTFLOAT %s +// CHECK-SOFTFLOAT: "-target-feature" "+soft-float" + // CHECK: invalid argument '-faltivec' only allowed with 'ppc/ppc64/ppc64le' // Check that -fno-altivec and -mno-altivec correctly disable the altivec Index: test/CodeGen/ppc-sfvarargs.c === --- test/CodeGen/ppc-sfvarargs.c +++ test/CodeGen/ppc-sfvarargs.c @@ -0,0 +1,17 @@ +// RUN: %clang -O0 --target=powerpc-unknown-linux-gnu -EB -msoft-float -S -emit-llvm %s -o - | FileCheck %s + +#include +void test(char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + va_arg(ap, double); + va_end(ap); +} + +void foo() { + double a; + test("test",a); +} +// CHECK: %{{[0-9]+}} = add i8 %numUsedRegs, 1 +// CHECK: %{{[0-9]+}} = and i8 %{{[0-9]+}}, -2 +// CHECK: %{{[0-9]+}} = mul i8 %{{[0-9]+}}, 4 \ No newline at end of file Index: lib/Driver/Tools.h === --- lib/Driver/Tools.h +++ lib/Driver/Tools.h @@ -728,6 +728,10 @@ FloatABI getARMFloatABI(const ToolChain &TC, const llvm::opt::ArgList &Args); } + +namespace ppc { + StringRef getPPCFloatABI(const llvm::opt::ArgList &Args); +} namespace XCore { // For XCore, we do not need to instantiate tools for PreProcess, PreCompile and // Compile. Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -1344,15 +1344,44 @@ // TODO: Change the LLVM backend option maybe? if (Name == "mfcrf") Name = "mfocrf"; - + Features.push_back(Args.MakeArgString((IsNegative ? "-" : "+") + Name)); } + StringRef FloatABI = ppc::getPPCFloatABI(Args); + if (FloatABI == "soft") +Features.push_back("+soft-float"); + // Altivec is a bit weird, allow overriding of the Altivec feature here. AddTargetFeature(Args, Features, options::OPT_faltivec, options::OPT_fno_altivec, "altivec"); } +StringRef ppc::getPPCFloatABI(const ArgList &Args) { + StringRef FloatABI; + if (Arg *A = Args.getLastArg(options::OPT_msoft_float, + options::OPT_mhard_float, + options::OPT_mfloat_abi_EQ)) { +if (A->getOption().matches(options::OPT_msoft_float)) + FloatABI = "soft"; +else if (A->getOption().matches(options::OPT_mhard_float)) + FloatABI = "hard"; +else { + FloatABI = A->getValue(); + if (FloatABI != "soft" && FloatABI != "hard") { +FloatABI = "hard"; + } +} + } + + // If unspecified, choose the default based on the platform. + if (FloatABI.empty()) { +FloatABI = "hard"; + } + + return FloatABI; +} + void Clang::AddPPCTargetArgs(const ArgList &Args, ArgStringList &CmdArgs) const { // Select the ABI to use. @@ -1389,10 +1418,26 @@ if (StringRef(A->getValue()) != "altivec") ABIName = A->getValue(); + StringRef FloatABI = ppc::getPPCFloatABI(Args); + + if (FloatABI == "soft") { +// Floating point operations and argument passing are soft. +CmdArgs.push_back("-msoft-float"); +CmdArgs.push_back("-mfloat-abi"); +CmdArgs.push_back("soft"); + } + else { +// Floating point operations and argument passing are hard. +assert(FloatABI == "hard" && "Invalid float abi!"); +CmdArgs.push_back("-mfloat-abi"); +CmdArgs.push_back("hard"); + } + if (ABIName) { CmdArgs.push_back("-target-abi"); CmdArgs.push_back(ABIName); } + } bool ppc::hasPPCAbiArg(const ArgList &Args, const char *Value) { Index: lib/CodeGen/TargetInfo.cpp === --- lib/CodeGen/TargetInfo.cpp +++ lib/CodeGen/TargetInfo.cpp @@ -3422,6 +3422,8 @@ bool isI64 = Ty->isIntegerType() && getContext().getTypeSize(Ty) == 64; bool isInt = Ty->isIntegerType() || Ty->isPointerType()
[PATCH] D13352: [PATCH] Add a CERT category for clang-tidy checkers
aaron.ballman created this revision. aaron.ballman added reviewers: alexfh, sbenza. aaron.ballman added a subscriber: cfe-commits. CERT produces a set of secure coding rules and recommendations for both C (https://www.securecoding.cert.org/confluence/display/c/SEI+CERT+C+Coding+Standard) and C++ (https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=637). One of the tasks we've been doing lately is mapping existing checks to our rules, as well as coming up with new checks where there is insufficient existing coverage for a rule. This patch adds a new module so that users can enable CERT-specific checkers by using -checks=-*,cert-*. Currently, this is remapping existing checkers under a new name that matches the CERT guideline the checker matches. However, this also is a convenient place for us to hang CERT-specific rules that do not apply elsewhere. This patch does not come with any tests because the only thing we could test is the name change for reported diagnostics, and I wasn't certain whether that was worth testing. One thing this patch does not do is enable tests for static analyzer checkers under new names. For instance, I would like to have a way to map clang-analyzer-unix.Malloc to cert-mem34-c, but that seems slightly more involved, and so I intend to do this in a follow-up patch. ~Aaron http://reviews.llvm.org/D13352 Files: clang-tidy/CMakeLists.txt clang-tidy/Makefile clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/cert/Makefile clang-tidy/tool/CMakeLists.txt clang-tidy/tool/ClangTidyMain.cpp clang-tidy/tool/Makefile Index: clang-tidy/tool/Makefile === --- clang-tidy/tool/Makefile +++ clang-tidy/tool/Makefile @@ -16,8 +16,9 @@ include $(CLANG_LEVEL)/../../Makefile.config LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader support mc option -USEDLIBS = clangTidy.a clangTidyLLVMModule.a clangTidyGoogleModule.a \ - clangTidyMiscModule.a clangTidyModernizeModule.a clangTidyReadability.a \ +USEDLIBS = clangTidy.a clangTidyCERTModule.a clangTidyLLVMModule.a \ + clangTidyGoogleModule.a \ clangTidyMiscModule.a \ + clangTidyModernizeModule.a clangTidyReadability.a \ clangTidyUtils.a clangStaticAnalyzerFrontend.a \ clangStaticAnalyzerCheckers.a clangStaticAnalyzerCore.a \ clangFormat.a clangASTMatchers.a clangTooling.a clangToolingCore.a \ Index: clang-tidy/tool/CMakeLists.txt === --- clang-tidy/tool/CMakeLists.txt +++ clang-tidy/tool/CMakeLists.txt @@ -10,6 +10,7 @@ clangASTMatchers clangBasic clangTidy + clangTidyCERTModule clangTidyGoogleModule clangTidyLLVMModule clangTidyMiscModule Index: clang-tidy/tool/ClangTidyMain.cpp === --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -347,6 +347,11 @@ return 0; } +// This anchor is used to force the linker to link the CERTModule. +extern volatile int CERTModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination = +CERTModuleAnchorSource; + // This anchor is used to force the linker to link the LLVMModule. extern volatile int LLVMModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination = Index: clang-tidy/Makefile === --- clang-tidy/Makefile +++ clang-tidy/Makefile @@ -11,6 +11,6 @@ LIBRARYNAME := clangTidy include $(CLANG_LEVEL)/../../Makefile.config -DIRS = utils readability llvm google misc modernize tool +DIRS = utils cert readability llvm google misc modernize tool include $(CLANG_LEVEL)/Makefile Index: clang-tidy/CMakeLists.txt === --- clang-tidy/CMakeLists.txt +++ clang-tidy/CMakeLists.txt @@ -26,6 +26,7 @@ ) add_subdirectory(tool) +add_subdirectory(cert) add_subdirectory(llvm) add_subdirectory(google) add_subdirectory(misc) Index: clang-tidy/cert/Makefile === --- clang-tidy/cert/Makefile +++ clang-tidy/cert/Makefile @@ -0,0 +1,12 @@ +##===- clang-tidy/misc/Makefile *- Makefile -*-===## +# +# The LLVM Compiler Infrastructure +# +# This file is distributed under the University of Illinois Open Source +# License. See LICENSE.TXT for details. +# +##===--===## +CLANG_LEVEL := ../../../.. +LIBRARYNAME := clangTidyCERTModule + +include $(CLANG_LEVEL)/Makefile Index: clang-tidy/cert/CMakeLists.txt === --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -0,0 +1,12 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTi
Re: [PATCH] D13339: Allow a ToolChain to compute the path of a compiler-rt's component.
This revision was automatically updated to reflect the committed changes. Closed by commit rL249030: Allow a ToolChain to compute the path of a compiler-rt's component. (authored by vkalintiris). Changed prior to commit: http://reviews.llvm.org/D13339?vs=36225&id=36265#toc Repository: rL LLVM http://reviews.llvm.org/D13339 Files: cfe/trunk/include/clang/Driver/ToolChain.h cfe/trunk/lib/Driver/SanitizerArgs.cpp cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/lib/Driver/Tools.cpp Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp === --- cfe/trunk/lib/Driver/SanitizerArgs.cpp +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp @@ -609,13 +609,11 @@ if (TC.getTriple().isOSWindows() && needsUbsanRt()) { // Instruct the code generator to embed linker directives in the object file // that cause the required runtime libraries to be linked. -CmdArgs.push_back( -Args.MakeArgString("--dependent-lib=" + - tools::getCompilerRT(TC, Args, "ubsan_standalone"))); +CmdArgs.push_back(Args.MakeArgString( +"--dependent-lib=" + TC.getCompilerRT(Args, "ubsan_standalone"))); if (types::isCXX(InputType)) CmdArgs.push_back(Args.MakeArgString( - "--dependent-lib=" + - tools::getCompilerRT(TC, Args, "ubsan_standalone_cxx"))); + "--dependent-lib=" + TC.getCompilerRT(Args, "ubsan_standalone_cxx"))); } } Index: cfe/trunk/lib/Driver/ToolChain.cpp === --- cfe/trunk/lib/Driver/ToolChain.cpp +++ cfe/trunk/lib/Driver/ToolChain.cpp @@ -23,7 +23,9 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/TargetRegistry.h" + using namespace clang::driver; +using namespace clang::driver::tools; using namespace clang; using namespace llvm::opt; @@ -265,6 +267,43 @@ llvm_unreachable("Invalid tool kind."); } +static StringRef getArchNameForCompilerRTLib(const ToolChain &TC, + const ArgList &Args) { + const llvm::Triple &Triple = TC.getTriple(); + bool IsWindows = Triple.isOSWindows(); + + if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86) +return "i386"; + + if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb) +return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows) + ? "armhf" + : "arm"; + + return TC.getArchName(); +} + +std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component, + bool Shared) const { + const llvm::Triple &TT = getTriple(); + const char *Env = + (TT.getEnvironment() == llvm::Triple::Android) ? "-android" : ""; + bool IsITANMSVCWindows = + TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment(); + + StringRef Arch = getArchNameForCompilerRTLib(*this, Args); + const char *Prefix = IsITANMSVCWindows ? "" : "lib"; + const char *Suffix = Shared ? (Triple.isOSWindows() ? ".dll" : ".so") + : (IsITANMSVCWindows ? ".lib" : ".a"); + + SmallString<128> Path(getDriver().ResourceDir); + StringRef OSLibName = Triple.isOSFreeBSD() ? "freebsd" : getOS(); + llvm::sys::path::append(Path, "lib", OSLibName); + llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" + +Arch + Env + Suffix); + return Path.str(); +} + Tool *ToolChain::SelectTool(const JobAction &JA) const { if (getDriver().ShouldUseClangCompiler(JA)) return getClang(); @@ -306,7 +345,6 @@ return GetProgramPath("ld"); } - types::ID ToolChain::LookupTypeForExtension(const char *Ext) const { return types::lookupTypeForExtension(Ext); } Index: cfe/trunk/lib/Driver/Tools.cpp === --- cfe/trunk/lib/Driver/Tools.cpp +++ cfe/trunk/lib/Driver/Tools.cpp @@ -2402,60 +2402,11 @@ } } -// Until ARM libraries are build separately, we have them all in one library -static StringRef getArchNameForCompilerRTLib(const ToolChain &TC, - const ArgList &Args) { - const llvm::Triple &Triple = TC.getTriple(); - bool IsWindows = Triple.isOSWindows(); - - if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86) -return "i386"; - - if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb) -return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows) - ? "armhf" - : "arm"; - - return TC.getArchName(); -} - -static SmallString<128> getCompilerRTLibDir(const ToolChain &TC) { - // The runtimes are located in the OS-specific resource directory. - SmallString<128> Res(TC.getDriver().ResourceDir); - const llvm::Triple &Triple = TC.getTriple(); - // TC.getOS() yield "freebsd10.0" wh
r249030 - Allow a ToolChain to compute the path of a compiler-rt's component.
Author: vkalintiris Date: Thu Oct 1 11:54:58 2015 New Revision: 249030 URL: http://llvm.org/viewvc/llvm-project?rev=249030&view=rev Log: Allow a ToolChain to compute the path of a compiler-rt's component. Summary: This patch moves getCompilerRT() from the clang::driver::tools namespace to the ToolChain class. This is needed for multilib toolchains that need to place their libraries in Clang's resource directory with a layout that is different from the default one. Reviewers: atanasyan, rsmith Subscribers: tberghammer, danalbert, srhines, cfe-commits Differential Revision: http://reviews.llvm.org/D13339 Modified: cfe/trunk/include/clang/Driver/ToolChain.h cfe/trunk/lib/Driver/SanitizerArgs.cpp cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/lib/Driver/Tools.cpp Modified: cfe/trunk/include/clang/Driver/ToolChain.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=249030&r1=249029&r2=249030&view=diff == --- cfe/trunk/include/clang/Driver/ToolChain.h (original) +++ cfe/trunk/include/clang/Driver/ToolChain.h Thu Oct 1 11:54:58 2015 @@ -250,6 +250,10 @@ public: return ToolChain::RLT_Libgcc; } + virtual std::string getCompilerRT(const llvm::opt::ArgList &Args, +StringRef Component, +bool Shared = false) const; + /// IsUnwindTablesDefault - Does this tool chain use -funwind-tables /// by default. virtual bool IsUnwindTablesDefault() const; Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=249030&r1=249029&r2=249030&view=diff == --- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original) +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Thu Oct 1 11:54:58 2015 @@ -609,13 +609,11 @@ void SanitizerArgs::addArgs(const ToolCh if (TC.getTriple().isOSWindows() && needsUbsanRt()) { // Instruct the code generator to embed linker directives in the object file // that cause the required runtime libraries to be linked. -CmdArgs.push_back( -Args.MakeArgString("--dependent-lib=" + - tools::getCompilerRT(TC, Args, "ubsan_standalone"))); +CmdArgs.push_back(Args.MakeArgString( +"--dependent-lib=" + TC.getCompilerRT(Args, "ubsan_standalone"))); if (types::isCXX(InputType)) CmdArgs.push_back(Args.MakeArgString( - "--dependent-lib=" + - tools::getCompilerRT(TC, Args, "ubsan_standalone_cxx"))); + "--dependent-lib=" + TC.getCompilerRT(Args, "ubsan_standalone_cxx"))); } } Modified: cfe/trunk/lib/Driver/ToolChain.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=249030&r1=249029&r2=249030&view=diff == --- cfe/trunk/lib/Driver/ToolChain.cpp (original) +++ cfe/trunk/lib/Driver/ToolChain.cpp Thu Oct 1 11:54:58 2015 @@ -23,7 +23,9 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/TargetRegistry.h" + using namespace clang::driver; +using namespace clang::driver::tools; using namespace clang; using namespace llvm::opt; @@ -265,6 +267,43 @@ Tool *ToolChain::getTool(Action::ActionC llvm_unreachable("Invalid tool kind."); } +static StringRef getArchNameForCompilerRTLib(const ToolChain &TC, + const ArgList &Args) { + const llvm::Triple &Triple = TC.getTriple(); + bool IsWindows = Triple.isOSWindows(); + + if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86) +return "i386"; + + if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb) +return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows) + ? "armhf" + : "arm"; + + return TC.getArchName(); +} + +std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component, + bool Shared) const { + const llvm::Triple &TT = getTriple(); + const char *Env = + (TT.getEnvironment() == llvm::Triple::Android) ? "-android" : ""; + bool IsITANMSVCWindows = + TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment(); + + StringRef Arch = getArchNameForCompilerRTLib(*this, Args); + const char *Prefix = IsITANMSVCWindows ? "" : "lib"; + const char *Suffix = Shared ? (Triple.isOSWindows() ? ".dll" : ".so") + : (IsITANMSVCWindows ? ".lib" : ".a"); + + SmallString<128> Path(getDriver().ResourceDir); + StringRef OSLibName = Triple.isOSFreeBSD() ? "freebsd" : getOS(); + llvm::sys::path::append(Path, "lib", OSLibName); + llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" + +
Re: [PATCH] D13318: Add a utility function to add target information to a command line
zarko updated this revision to Diff 36266. zarko added a comment. Address comment comments. http://reviews.llvm.org/D13318 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp unittests/Tooling/ToolingTest.cpp Index: unittests/Tooling/ToolingTest.cpp === --- unittests/Tooling/ToolingTest.cpp +++ unittests/Tooling/ToolingTest.cpp @@ -18,6 +18,8 @@ #include "clang/Tooling/Tooling.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Config/llvm-config.h" +#include "llvm/Support/TargetSelect.h" +#include "llvm/Support/TargetRegistry.h" #include "gtest/gtest.h" #include #include @@ -291,6 +293,84 @@ EXPECT_FALSE(Found); } +namespace { +/// Find a target name such that looking for it in TargetRegistry by that name +/// returns the same target. We expect that there is at least one target +/// configured with this property. +std::string getAnyTarget() { + llvm::InitializeAllTargets(); + for (const auto &Target : llvm::TargetRegistry::targets()) { +std::string Error; +if (llvm::TargetRegistry::lookupTarget(Target.getName(), Error) == +&Target) { + return Target.getName(); +} + } + return ""; +} +} + +TEST(addTargetAndModeForProgramName, AddsTargetAndMode) { + std::string Target = getAnyTarget(); + ASSERT_FALSE(Target.empty()); + + std::vector Args = {"clang", "-foo"}; + addTargetAndModeForProgramName(Args, ""); + EXPECT_EQ((std::vector{"clang", "-foo"}), Args); + addTargetAndModeForProgramName(Args, Target + "-g++"); + EXPECT_EQ((std::vector{"clang", "-target", Target, + "--driver-mode=g++", "-foo"}), +Args); +} + +TEST(addTargetAndModeForProgramName, PathIgnored) { + std::string Target = getAnyTarget(); + ASSERT_FALSE(Target.empty()); + + SmallString<32> ToolPath; + llvm::sys::path::append(ToolPath, "foo", "bar", Target + "-g++"); + + std::vector Args = {"clang", "-foo"}; + addTargetAndModeForProgramName(Args, ToolPath); + EXPECT_EQ((std::vector{"clang", "-target", Target, + "--driver-mode=g++", "-foo"}), +Args); +} + +TEST(addTargetAndModeForProgramName, IgnoresExistingTarget) { + std::string Target = getAnyTarget(); + ASSERT_FALSE(Target.empty()); + + std::vector Args = {"clang", "-foo", "-target", "something"}; + addTargetAndModeForProgramName(Args, Target + "-g++"); + EXPECT_EQ((std::vector{"clang", "--driver-mode=g++", "-foo", + "-target", "something"}), +Args); + + std::vector ArgsAlt = {"clang", "-foo", "-target=something"}; + addTargetAndModeForProgramName(ArgsAlt, Target + "-g++"); + EXPECT_EQ((std::vector{"clang", "--driver-mode=g++", "-foo", + "-target=something"}), +ArgsAlt); +} + +TEST(addTargetAndModeForProgramName, IgnoresExistingMode) { + std::string Target = getAnyTarget(); + ASSERT_FALSE(Target.empty()); + + std::vector Args = {"clang", "-foo", "--driver-mode=abc"}; + addTargetAndModeForProgramName(Args, Target + "-g++"); + EXPECT_EQ((std::vector{"clang", "-target", Target, "-foo", + "--driver-mode=abc"}), +Args); + + std::vector ArgsAlt = {"clang", "-foo", "--driver-mode", "abc"}; + addTargetAndModeForProgramName(ArgsAlt, Target + "-g++"); + EXPECT_EQ((std::vector{"clang", "-target", Target, "-foo", + "--driver-mode", "abc"}), +ArgsAlt); +} + #ifndef LLVM_ON_WIN32 TEST(ClangToolTest, BuildASTs) { FixedCompilationDatabase Compilations("/", std::vector()); Index: lib/Tooling/Tooling.cpp === --- lib/Tooling/Tooling.cpp +++ lib/Tooling/Tooling.cpp @@ -17,6 +17,7 @@ #include "clang/Driver/Compilation.h" #include "clang/Driver/Driver.h" #include "clang/Driver/Tool.h" +#include "clang/Driver/ToolChain.h" #include "clang/Frontend/ASTUnit.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendDiagnostic.h" @@ -162,6 +163,31 @@ return AbsolutePath.str(); } +void addTargetAndModeForProgramName(std::vector &CommandLine, +StringRef InvokedAs) { + if (!CommandLine.empty() && !InvokedAs.empty()) { +bool AlreadyHasTarget = false; +bool AlreadyHasMode = false; +// Skip CommandLine[0]. +for (auto Token = ++CommandLine.begin(); Token != CommandLine.end(); + ++Token) { + StringRef TokenRef(*Token); + AlreadyHasTarget |= + (TokenRef == "-target" || TokenRef.startswith("-target=")); + AlreadyHasMode |= (TokenRef == "--driver-mode" || + TokenRef.startswith("--driver-mode=")); +} +auto TargetMode = +clang::driver::ToolChain::getTargetAndModeFromProgramName(InvokedAs); +if (!AlreadyHasMode && !TargetMode.second.empty()) { + CommandLine.insert(
r249031 - Module debugging: Also emit Objective-C interfaces forward declarations
Author: adrian Date: Thu Oct 1 11:57:02 2015 New Revision: 249031 URL: http://llvm.org/viewvc/llvm-project?rev=249031&view=rev Log: Module debugging: Also emit Objective-C interfaces forward declarations in their module scope when building a clang module. Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/test/Modules/Inputs/DebugObjC.h cfe/trunk/test/Modules/ModuleDebugInfo.m Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=249031&r1=249030&r2=249031&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Oct 1 11:57:02 2015 @@ -1663,9 +1663,10 @@ llvm::DIType *CGDebugInfo::CreateType(co // debug type since we won't be able to lay out the entire type. ObjCInterfaceDecl *Def = ID->getDefinition(); if (!Def || !Def->getImplementation()) { +llvm::DIScope *Mod = getParentModuleOrNull(ID); llvm::DIType *FwdDecl = DBuilder.createReplaceableCompositeType( -llvm::dwarf::DW_TAG_structure_type, ID->getName(), TheCU, DefUnit, Line, -RuntimeLang); +llvm::dwarf::DW_TAG_structure_type, ID->getName(), Mod ? Mod : TheCU, +DefUnit, Line, RuntimeLang); ObjCInterfaceCache.push_back(ObjCInterfaceCacheEntry(Ty, FwdDecl, Unit)); return FwdDecl; } Modified: cfe/trunk/test/Modules/Inputs/DebugObjC.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugObjC.h?rev=249031&r1=249030&r2=249031&view=diff == --- cfe/trunk/test/Modules/Inputs/DebugObjC.h (original) +++ cfe/trunk/test/Modules/Inputs/DebugObjC.h Thu Oct 1 11:57:02 2015 @@ -1,3 +1,5 @@ +@class FwdDecl; + @interface ObjCClass { int ivar; } Modified: cfe/trunk/test/Modules/ModuleDebugInfo.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.m?rev=249031&r1=249030&r2=249031&view=diff == --- cfe/trunk/test/Modules/ModuleDebugInfo.m (original) +++ cfe/trunk/test/Modules/ModuleDebugInfo.m Thu Oct 1 11:57:02 2015 @@ -20,6 +20,8 @@ // CHECK: distinct !DICompileUnit(language: DW_LANG_ObjC // CHECK-SAME:isOptimized: false, // CHECK: !DICompositeType(tag: DW_TAG_structure_type, +// CHECK-SAME: name: "FwdDecl", +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, // CHECK-SAME: name: "ObjCClass", // CHECK: !DIObjCProperty(name: "property", // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "ivar" @@ -28,6 +30,9 @@ // CHECK: !DISubprogram(name: "-[ categoryMethod]" // MODULE-CHECK: !DICompositeType(tag: DW_TAG_structure_type, -// MODULE-CHECK-SAME: name: "ObjCClass", +// MODULE-CHECK-SAME: name: "FwdDecl", // MODULE-CHECK-SAME: scope: ![[MODULE:[0-9]+]], // MODULE-CHECK: ![[MODULE]] = !DIModule(scope: null, name: "DebugObjC" +// MODULE-CHECK: !DICompositeType(tag: DW_TAG_structure_type, +// MODULE-CHECK-SAME: name: "ObjCClass", +// MODULE-CHECK-SAME: scope: ![[MODULE]], ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11908: Clang support for -fthinlto.
On Wed, Sep 30, 2015 at 2:33 PM, Duncan P. N. Exon Smith wrote: > +echristo, +espindola, +pcc > >> On 2015-Sep-30, at 12:39, Teresa Johnson wrote: >> >> On Wed, Sep 30, 2015 at 11:11 AM, Duncan P. N. Exon Smith >> wrote: >>> On 2015-Sep-30, at 10:40, Teresa Johnson wrote: On Wed, Sep 30, 2015 at 10:19 AM, Duncan P. N. Exon Smith wrote: > >> On 2015-Sep-23, at 10:28, Teresa Johnson wrote: >> >> Index: include/clang/Driver/Options.td >> === >> --- include/clang/Driver/Options.td >> +++ include/clang/Driver/Options.td >> @@ -686,6 +686,9 @@ >> def flto_EQ : Joined<["-"], "flto=">, >> Group; >> def flto : Flag<["-"], "flto">, Flags<[CC1Option]>, Group; >> def fno_lto : Flag<["-"], "fno-lto">, Group; >> +def fthinlto : Flag<["-"], "fthinlto">, Flags<[CC1Option]>, >> Group; > > I wonder whether this is the right way to add new variants of LTO. > I.e., maybe this should be "-flto=thin"? Do you happen to know how this > would conflict with GCC options? (I see we ignore "-flto="...) I looked at GCC docs and sources. It does have a -flto= variant. From https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html: - If you specify the optional n, the optimization and code generation done at link time is executed in parallel using n parallel jobs by utilizing an installed make program. The environment variable MAKE may be used to override the program used. The default value for n is 1. You can also specify -flto=jobserver to use GNU make's job server mode to determine the number of parallel jobs. This is useful when the Makefile calling GCC is already executing in parallel. You must prepend a ‘+’ to the command recipe in the parent Makefile for this to work. This option likely only works if MAKE is GNU make. I would anticipate that we may want to support something like this for ThinLTO eventually to specify the number of parallel jobs for the ThinLTO backend processes. So it might be confusing to overload -flto=. >>> >>> Hmm. You're right that the GCC usage seems pretty different. >>> >>> I guess you're envisioning -fthinlto=jobserver? >> >> Or -fthinlto=n. >> >>> I wonder if we could >>> do this as -flto=thin,jobserver or something similar? >> >> I am ok with -flto=thin and then later extending to -flto=thin,n etc. >> >> This simplifies the interaction with -fno-lto. >> >> I think a -flto=thin followed by -flto should probably revert to >> normal LTO, WDYT? >> >> Another thing to consider is to add -flto=full or something like that >> which is currently an alias for -flto. So we would have: >> >> -flto= where type could be "full" or "thin" >> -flto means -flto=full >> -fno-lto disables all types of flto >> and last option of the above 3 wins. > > This SGTM. "monolithic" might be more descriptive than "full", or > maybe someone has a better name. > > Eric/Rafael/Peter: any thoughts about driver options? For now I have implemented -flto=full and -flto=thin. I guess "full" is easier to type than "monolithic", but I am fine with switching it to the latter or any better suggestions. I am about to post the new patch. > >>> >>> It's pretty hard to remove driver options, so I think it's important to >>> get the interface right. I anticipate that we'll add more LTO variants >>> in the future, so we should take care that this scales reasonably. >>> >>> (This may be a good discussion for cfe-dev, not sure.) >>> > E.g., as a user I'd expect -fno-lto to turn off whatever LTO was turned > on. And I might expect -flto to override -fthinlto, or vice versa. Good point, I think the last one should win. Looks like right now -fthinlto is effectively always winning. I will fix that. Not sure about -fno-lto behavior if we leave -fthinlto as a separate option. Let me know what you think based on the GCC usage of -flto=. >>> >>> Right, if they're totally separate, then either behaviour for -fno-lto >>> is confusing. > -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13318: Add a utility function to add target information to a command line
zarko marked an inline comment as done. zarko added a comment. Thanks; would someone with commit access land this? echristo: for a more concrete example, imagine a source code indexer that collects up dependencies. This is a Clang tool that gets called by the build system for each translation unit and is provided with the path to the compiler it would use to build it along with the arguments to that compiler. The indexer is unlikely to be on the same path as the compiler, but the indexer's path needs to be preserved because of the way Clang tools search for resources. The only important part of the path to the compiler is the compiler's name, because sometimes the name contains information about the target or driver mode that affects how the indexer should interpret the translation unit. This function applies the logic that Clang's driver uses to deduce that information; it then adds arguments that cause the tooling infrastructure to correctly interpret the code. http://reviews.llvm.org/D13318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13330: Implement __attribute__((unique_instantiation))
loladiro added a comment. Thanks for the quick review. Comment at: include/clang/Basic/Attr.td:1462 @@ +1461,3 @@ +def UniqueInstantiation : InheritableAttr { + let Spellings = [GCC<"unique_instantiation">]; + let Subjects = SubjectList<[CXXRecord]>; aaron.ballman wrote: > This is not a GCC attribute, so it should not be spelled as such. Since this > only applies to C++ code, I would recommend a C++11 spelling (in the clang > namespace). If you think this is something C++03 and earlier code would > really benefit from, then you could also add a GNU-style spelling. No, this is C++11 only. Will change the spelling. Comment at: include/clang/Basic/Attr.td:1464 @@ +1463,3 @@ + let Subjects = SubjectList<[CXXRecord]>; + let Documentation = [Undocumented]; +} aaron.ballman wrote: > Please, no undocumented attributes. Will document. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2443 @@ -2442,1 +2442,3 @@ ":must be between 1 and %2}2">; +def err_unique_instantiation_wrong_decl : Error< + "unique_instantiation attribute on something that is not a explicit template declaration or instantiation.">; aaron.ballman wrote: > Would this make more sense as an option in warn_attribute_wrong_decl_type > instead? (there's an err_ version as well if you wish to keep this an error). > > Also, please ensure that the attribute is quoted in the diagnostic -- it > makes things less confusing for the user. Ok, so should I add an "explicit template instantiations" option to that err? Comment at: lib/AST/ASTContext.cpp:8244 @@ -8242,2 +8243,3 @@ case TSK_ExplicitInstantiationDefinition: -return GVA_StrongODR; +CTSD = dyn_cast(FD->getDeclContext()); +if (!CTSD || !CTSD->hasAttr()) aaron.ballman wrote: > I think this would be easier to read (and not have to hoist a declaration out > of the switch) as: > > ``` > if (const auto *CTSD = dyn_cast<>()) { > if (CTSD->hasAttr<>()) > return GVA_StrongExternal; > } > return GVA_StrongODR; > ``` > Ok. Comment at: lib/Sema/SemaDeclAttr.cpp:4539 @@ +4538,3 @@ +// by an ExplicitInstantiationDeclaration. +if (CTSD->getSpecializationKind() == TSK_ExplicitInstantiationDefinition) { + if (!CTSD->getPreviousDecl()) aaron.ballman wrote: > Why is this required as part of the feature design? It seems restrictive. This was part of Doug's original Spec, so I implemented it: > A unique explicit instantiation definition shall follow an explicit > instantiation declaration of the same entity. [//Note//: this > requirement encourages a programming style that uses unique explicit > instantiation declarations (typically in a header) to suppress > implicit instantiations of a template or its members, so that the > unique explicit instantiation definition of that template or its members > is unique. //- end note//] I think that makes a decent amount of sense, since you really want to avoid the case where some translation units don't see the extern template declaration. Comment at: lib/Sema/SemaDeclAttr.cpp:4546 @@ +4545,3 @@ +// have performed the same check on the previous declaration here. +CXXRecordDecl *Previous = CTSD->getPreviousDecl(); +if (Previous) { aaron.ballman wrote: > Is this something that can be handled by mergeDeclAttribute()? I'm not > certain how that interplays with templates specifically, but usually we do > this sort of logic within a Sema::mergeFooAttr() function. Hmm, I'm not sure, the goal of this is to ensure that all declarations and definitions of this extern template have the attribute set. It's not really `merging` per se. Though I suppose it could be made to fit in that framework. Repository: rL LLVM http://reviews.llvm.org/D13330 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11908: Clang support for -fthinlto.
tejohnson updated this revision to Diff 36267. tejohnson added a comment. Address review comments. Instead of -fthinlto use -flto=thin, and add in -flto=full as an alias to the existing -flto. Add tests to check for proper overriding of -flto variants on the command line, and convert grep tests to FileCheck. http://reviews.llvm.org/D11908 Files: include/clang/Driver/Driver.h include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/Driver/Driver.cpp lib/Driver/SanitizerArgs.cpp lib/Driver/Tools.cpp lib/Frontend/CompilerInvocation.cpp test/Driver/clang_f_opts.c test/Driver/lto.c test/Driver/thinlto.c test/Misc/thinlto.c Index: test/Misc/thinlto.c === --- /dev/null +++ test/Misc/thinlto.c @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -flto=thin -emit-llvm-bc < %s | llvm-bcanalyzer -dump | FileCheck %s +// CHECK: %t +// RUN: FileCheck -check-prefix=CHECK-COMPILE-ACTIONS < %t %s +// +// CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir +// CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc + +// RUN: %clang -ccc-print-phases %s -flto=thin 2> %t +// RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s +// +// CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}thinlto.c", c +// CHECK-COMPILELINK-ACTIONS: 1: preprocessor, {0}, cpp-output +// CHECK-COMPILELINK-ACTIONS: 2: compiler, {1}, ir +// CHECK-COMPILELINK-ACTIONS: 3: backend, {2}, lto-bc +// CHECK-COMPILELINK-ACTIONS: 4: linker, {3}, image + +// -flto=thin should cause link using gold plugin with thinlto option, +// also confirm that it takes precedence over earlier -fno-lto and -flto=full. +// RUN: %clang -### %s -flto=full -fno-lto -flto=thin 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-THIN-ACTION < %t %s +// +// CHECK-LINK-THIN-ACTION: "-plugin" "{{.*}}/LLVMgold.so" +// CHECK-LINK-THIN-ACTION: "-plugin-opt=thinlto" + +// Check that subsequent -flto=full takes precedence +// RUN: %clang -### %s -flto=thin -flto=full 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-FULL-ACTION < %t %s +// +// CHECK-LINK-FULL-ACTION: "-plugin" "{{.*}}/LLVMgold.so" +// CHECK-LINK-FULL-ACTION-NOT: "-plugin-opt=thinlto" + +// Check that subsequent -fno-lto takes precedence +// RUN: %clang -### %s -flto=thin -fno-lto 2> %t +// RUN: FileCheck -check-prefix=CHECK-LINK-NOLTO-ACTION < %t %s +// +// CHECK-LINK-NOLTO-ACTION-NOT: "-plugin" "{{.*}}/LLVMgold.so" +// CHECK-LINK-NOLTO-ACTION-NOT: "-plugin-opt=thinlto" Index: test/Driver/lto.c === --- test/Driver/lto.c +++ test/Driver/lto.c @@ -1,25 +1,51 @@ // -flto causes a switch to llvm-bc object files. -// RUN: %clang -ccc-print-phases -c %s -flto 2> %t.log -// RUN: grep '2: compiler, {1}, ir' %t.log -// RUN: grep '3: backend, {2}, lto-bc' %t.log +// RUN: %clang -ccc-print-phases -c %s -flto 2> %t +// RUN: FileCheck -check-prefix=CHECK-COMPILE-ACTIONS < %t %s +// +// CHECK-COMPILE-ACTIONS: 2: compiler, {1}, ir +// CHECK-COMPILE-ACTIONS: 3: backend, {2}, lto-bc -// RUN: %clang -ccc-print-phases %s -flto 2> %t.log -// RUN: grep '0: input, ".*lto.c", c' %t.log -// RUN: grep '1: preprocessor, {0}, cpp-output' %t.log -// RUN: grep '2: compiler, {1}, ir' %t.log -// RUN: grep '3: backend, {2}, lto-bc' %t.log -// RUN: grep '4: linker, {3}, image' %t.log +// RUN: %clang -ccc-print-phases %s -flto 2> %t +// RUN: FileCheck -check-prefix=CHECK-COMPILELINK-ACTIONS < %t %s +// +// CHECK-COMPILELINK-ACTIONS: 0: input, "{{.*}}lto.c", c +// CHECK-COMPILELINK-ACTIONS: 1: preprocessor, {0}, cpp-output +// CHECK-COMPILELINK-ACTIONS: 2: compiler, {1}, ir +// CHECK-COMPILELINK-ACTIONS: 3: backend, {2}, lto-bc +// CHECK-COMPILELINK-ACTIONS: 4: linker, {3}, image // llvm-bc and llvm-ll outputs need to match regular suffixes // (unfortunately). -// RUN: %clang %s -flto -save-temps -### 2> %t.log -// RUN: grep '"-o" ".*lto\.i" "-x" "c" ".*lto\.c"' %t.log -// RUN: grep '"-o" ".*lto\.bc" .*".*lto\.i"' %t.log -// RUN: grep '"-o" ".*lto\.o" .*".*lto\.bc"' %t.log -// RUN: grep '".*a\.\(out\|exe\)" .*".*lto\.o"' %t.log +// RUN: %clang %s -flto -save-temps -### 2> %t +// RUN: FileCheck -check-prefix=CHECK-COMPILELINK-SUFFIXES < %t %s +// +// CHECK-COMPILELINK-SUFFIXES: "-o" "{{.*}}lto.i" "-x" "c" "{{.*}}lto.c" +// CHECK-COMPILELINK-SUFFIXES: "-o" "{{.*}}lto.bc" {{.*}}"{{.*}}lto.i" +// CHECK-COMPILELINK-SUFFIXES: "-o" "{{.*}}lto.o" {{.*}}"{{.*}}lto.bc" +// CHECK-COMPILELINK-SUFFIXES: "{{.*}}a.{{(out|exe)}}" {{.*}}"{{.*}}lto.o" -// RUN: %clang %s -flto -S -### 2> %t.log -// RUN: grep '"-o" ".*lto\.s" "-x" "c" ".*lto\.c"' %t.log +// RUN: %clang %s -flto -S -### 2> %t +// RUN: FileCheck -check-prefix=CHECK-COMPILE-SUFFIXES < %t %s +// +// CHECK-COMPILE-SUFFIXES: "-o" "{{.*}}lto.s" "-x" "c" "{{.*}}lto.c" // RUN: not %clang %s -emit-llvm 2>&1 | FileCheck --check-prefix=LLVM-LINK %s // LLVM-LINK: -emit-llvm cannot be used when linking + +// -flto should cause link u
Re: [PATCH] D13330: Implement __attribute__((unique_instantiation))
On Thu, Oct 1, 2015 at 1:09 PM, Keno Fischer wrote: > loladiro added a comment. > > Thanks for the quick review. > > > > Comment at: include/clang/Basic/Attr.td:1462 > @@ +1461,3 @@ > +def UniqueInstantiation : InheritableAttr { > + let Spellings = [GCC<"unique_instantiation">]; > + let Subjects = SubjectList<[CXXRecord]>; > > aaron.ballman wrote: >> This is not a GCC attribute, so it should not be spelled as such. Since this >> only applies to C++ code, I would recommend a C++11 spelling (in the clang >> namespace). If you think this is something C++03 and earlier code would >> really benefit from, then you could also add a GNU-style spelling. > No, this is C++11 only. Will change the spelling. > > > Comment at: include/clang/Basic/Attr.td:1464 > @@ +1463,3 @@ > + let Subjects = SubjectList<[CXXRecord]>; > + let Documentation = [Undocumented]; > +} > > aaron.ballman wrote: >> Please, no undocumented attributes. > Will document. > > > Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2443 > @@ -2442,1 +2442,3 @@ >":must be between 1 and %2}2">; > +def err_unique_instantiation_wrong_decl : Error< > + "unique_instantiation attribute on something that is not a explicit > template declaration or instantiation.">; > > aaron.ballman wrote: >> Would this make more sense as an option in warn_attribute_wrong_decl_type >> instead? (there's an err_ version as well if you wish to keep this an error). >> >> Also, please ensure that the attribute is quoted in the diagnostic -- it >> makes things less confusing for the user. > Ok, so should I add an "explicit template instantiations" option to that err? "explicit instantiation definition" seems reasonable if this is limited to only applying to definitions and not declarations. You could also automated the checking for this by updating ClangAttrEmitter.cpp, but I don't think that's a requirement for this patch. > > Comment at: lib/AST/ASTContext.cpp:8244 > @@ -8242,2 +8243,3 @@ >case TSK_ExplicitInstantiationDefinition: > -return GVA_StrongODR; > +CTSD = dyn_cast(FD->getDeclContext()); > +if (!CTSD || !CTSD->hasAttr()) > > aaron.ballman wrote: >> I think this would be easier to read (and not have to hoist a declaration >> out of the switch) as: >> >> ``` >> if (const auto *CTSD = dyn_cast<>()) { >> if (CTSD->hasAttr<>()) >> return GVA_StrongExternal; >> } >> return GVA_StrongODR; >> ``` >> > Ok. > > > Comment at: lib/Sema/SemaDeclAttr.cpp:4539 > @@ +4538,3 @@ > +// by an ExplicitInstantiationDeclaration. > +if (CTSD->getSpecializationKind() == > TSK_ExplicitInstantiationDefinition) { > + if (!CTSD->getPreviousDecl()) > > aaron.ballman wrote: >> Why is this required as part of the feature design? It seems restrictive. > This was part of Doug's original Spec, so I implemented it: > > > >> A unique explicit instantiation definition shall follow an explicit >> instantiation declaration of the same entity. [//Note//: this >> requirement encourages a programming style that uses unique explicit >> instantiation declarations (typically in a header) to suppress >> implicit instantiations of a template or its members, so that the >> unique explicit instantiation definition of that template or its members >> is unique. //- end note//] > > I think that makes a decent amount of sense, since you really want to avoid > the case where some translation units don't see the extern template > declaration. Okay, that makes sense to me as well. Thank you for pointing that out. You may want to include this (at least in part) in the comments. > > Comment at: lib/Sema/SemaDeclAttr.cpp:4546 > @@ +4545,3 @@ > +// have performed the same check on the previous declaration here. > +CXXRecordDecl *Previous = CTSD->getPreviousDecl(); > +if (Previous) { > > aaron.ballman wrote: >> Is this something that can be handled by mergeDeclAttribute()? I'm not >> certain how that interplays with templates specifically, but usually we do >> this sort of logic within a Sema::mergeFooAttr() function. > Hmm, I'm not sure, the goal of this is to ensure that all declarations and > definitions of this extern template have the attribute set. It's not really > `merging` per se. Though I suppose it could be made to fit in that framework. Merging is usually the place where we handle "what do other declarations and definitions of this thing do", which is why it came to mind. If it turns out that it's an inappropriate place for it, it would be good to know why. ~Aaron > > > Repository: > rL LLVM > > http://reviews.llvm.org/D13330 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13203: [Clang] - Massaging code to fix MSVS 2015 win32-release configuration
grimar updated this revision to Diff 36271. grimar added a comment. Added comment per review request. http://reviews.llvm.org/D13203 Files: ASTContext.cpp Index: ASTContext.cpp === --- ASTContext.cpp +++ ASTContext.cpp @@ -364,14 +364,19 @@ } else { RC = getRawCommentForDeclNoCache(I); OriginalDeclForRC = I; - RawCommentAndCacheFlags Raw; + // TODO: these next two lines helps to workaround msvs 2015 + // internal compiler error. This is a replacement + // of delayed assignment RedeclComments[I] = Raw + // which caused that issue. + RedeclComments[I] = RawCommentAndCacheFlags(); + RawCommentAndCacheFlags &Raw = RedeclComments[I]; + // if (RC) { Raw.setRaw(RC); Raw.setKind(RawCommentAndCacheFlags::FromDecl); } else Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl); Raw.setOriginalDecl(I); - RedeclComments[I] = Raw; if (RC) break; } Index: ASTContext.cpp === --- ASTContext.cpp +++ ASTContext.cpp @@ -364,14 +364,19 @@ } else { RC = getRawCommentForDeclNoCache(I); OriginalDeclForRC = I; - RawCommentAndCacheFlags Raw; + // TODO: these next two lines helps to workaround msvs 2015 + // internal compiler error. This is a replacement + // of delayed assignment RedeclComments[I] = Raw + // which caused that issue. + RedeclComments[I] = RawCommentAndCacheFlags(); + RawCommentAndCacheFlags &Raw = RedeclComments[I]; + // if (RC) { Raw.setRaw(RC); Raw.setKind(RawCommentAndCacheFlags::FromDecl); } else Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl); Raw.setOriginalDecl(I); - RedeclComments[I] = Raw; if (RC) break; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D13357: [Concepts] Add diagnostic; specializations of variable and function concepts
nwilson created this revision. nwilson added reviewers: rsmith, hubert.reinterpretcast, aaron.ballman, faisalv, fraggamuffin. nwilson added a subscriber: cfe-commits. http://reviews.llvm.org/D13357 Files: include/clang/AST/Decl.h include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaTemplate.cpp test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp Index: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp === --- /dev/null +++ test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.concept/p7.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s + +template concept bool VCEI { true }; +template concept bool VCEI; // expected-error {{variable concept cannot be explicitly instantiated}} + +template concept bool VCPS { true }; +template concept bool VCPS { true }; // expected-error {{variable concept cannot be partially specialized}} + +template concept bool VCES { true }; +template<> concept bool VCES { true }; // expected-error {{variable concept cannot be explicitly specialized}} + +template concept bool FCEI() { return true; } +template concept bool FCEI(); // expected-error {{function concept cannot be explicitly instantiated}} + +template concept bool FCES() { return true; } +template<> concept bool FCES() { return true; } // expected-error {{function concept cannot be explicitly specialized}} Index: lib/Sema/SemaTemplate.cpp === --- lib/Sema/SemaTemplate.cpp +++ lib/Sema/SemaTemplate.cpp @@ -7588,6 +7588,14 @@ Diag(D.getDeclSpec().getConstexprSpecLoc(), diag::err_explicit_instantiation_constexpr); + // C++ Concepts TS [dcl.spec.concept]p7: A program shall not declare an + // explicit instantiation, [...] of a concept definition. + if (D.getDeclSpec().isConceptSpecified() && R->isFunctionType()) { +Diag(D.getIdentifierLoc(), diag::err_concept_decl_specialized) + << 1 << 0; +return true; + } + // C++0x [temp.explicit]p2: // There are two forms of explicit instantiation: an explicit instantiation // definition and an explicit instantiation declaration. An explicit @@ -7659,6 +7667,14 @@ return true; } + // C++ Concepts TS [dcl.spec.concept]p7: A program shall not declare an + // explicit instantiation, [...] of a concept definition. + if (D.getDeclSpec().isConceptSpecified()) { +Diag(D.getIdentifierLoc(), diag::err_concept_decl_specialized) + << 0 << 0; +return true; + } + // Translate the parser's template argument list into our AST format. TemplateArgumentListInfo TemplateArgs = makeTemplateArgumentListInfo(*this, *D.getName().TemplateId); Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -5898,6 +5898,21 @@ << 0 << 3; NewVD->setInvalidDecl(true); } + + // C++ Concepts TS [dcl.spec.concept]p7: A program shall not declare [...] + // an explicit specialization, or a partial specialization of a concept + // definition + if (IsVariableTemplateSpecialization && !IsPartialSpecialization) { +Diag(NewVD->getLocation(), diag::err_concept_decl_specialized) + << 0 << 1; +NewVD->setInvalidDecl(true); + } + + if (IsVariableTemplateSpecialization && IsPartialSpecialization) { +Diag(NewVD->getLocation(), diag::err_concept_decl_specialized) + << 0 << 2; +NewVD->setInvalidDecl(true); + } } } @@ -7544,6 +7559,9 @@ } if (isConcept) { + // This is a function concept + NewFD->setConcept(true); + // C++ Concepts TS [dcl.spec.concept]p1: The concept specifier shall be // applied only to the definition of a function template [...] if (!D.isFunctionDefinition()) { @@ -7595,6 +7613,14 @@ << 1 << 3; NewFD->setInvalidDecl(true); } + + // C++ Concepts TS [dcl.spec.concept]p7: A program shall not declare [...] + // an explicit specialization, [...] of a concept definition. + if (isFunctionTemplateSpecialization) { +Diag(NewFD->getLocation(), diag::err_concept_decl_specialized) + << 1 << 1; +NewFD->setInvalidDecl(); + } } // If __module_private__ was specified, mark the function accordingly. @@ -7858,8 +7884,8 @@ TemplateArgs); HasExplicitTemplateArgs = true; - - if (NewFD->isInvalidDecl()) { + + if (NewFD->isInvalidDecl() && !NewFD->isConcept()) { HasExplicitTemplateArgs = false; } else if (FunctionTemplate) { // Function template with explicit template arguments. Index: include/clang/Basic/DiagnosticSemaKinds.td ==
Re: r248982 - Decorating virtual functions load with invariant.load
I added !invariant.load to virtual functions load, so when optimizer see case like this %vtable = load ... %1 = load (...) %vtable, !invariant.load !0 call %1(...) %2 = load (...) %vtable, !invariant.load !0 call %2(...) can merge the 2 virtual functions load into one like this: %vtable = load ... %1 = load (...) %vtable, !invariant.load !0 call %1(...) call %1(...) This change requires -fstrict-vtable-pointers because without !invariant.group optimizer will not merge vtable load into one load, and because of it !invariant.load will be useless. On Wed, Sep 30, 2015 at 9:28 PM, Chandler Carruth wrote: > On Wed, Sep 30, 2015 at 8:52 PM Piotr Padlewski via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: prazek >> Date: Wed Sep 30 22:50:41 2015 >> New Revision: 248982 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=248982&view=rev >> Log: >> Decorating virtual functions load with invariant.load >> > > This change description really doesn't tell me anything about what this > change does. Could you try to write more detailed and explanatory change > logs? > > Imagine a reader that has little or no context reading the change log to > gain the context necessary to read the actual patch and understand what is > going on with it. > > >> >> http://reviews.llvm.org/D13279 >> >> Modified: >> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp >> cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp >> >> Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=248982&r1=248981&r2=248982&view=diff >> >> == >> --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original) >> +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed Sep 30 22:50:41 2015 >> @@ -1609,7 +1609,16 @@ llvm::Value *ItaniumCXXABI::getVirtualFu >>uint64_t VTableIndex = >> CGM.getItaniumVTableContext().getMethodVTableIndex(GD); >>llvm::Value *VFuncPtr = >>CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfn"); >> - return CGF.Builder.CreateAlignedLoad(VFuncPtr, CGF.getPointerAlign()); >> + auto *Inst = CGF.Builder.CreateAlignedLoad(VFuncPtr, >> CGF.getPointerAlign()); >> + >> + // It's safe to add "invariant.load" without -fstrict-vtable-pointers, >> but it >> + // would not help in devirtualization. >> + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && >> + CGM.getCodeGenOpts().StrictVTablePointers) >> +Inst->setMetadata(llvm::LLVMContext::MD_invariant_load, >> + llvm::MDNode::get(CGM.getLLVMContext(), >> +llvm::ArrayRef> *>())); >> + return Inst; >> } >> >> llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall( >> >> Modified: cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp?rev=248982&r1=248981&r2=248982&view=diff >> >> == >> --- cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp (original) >> +++ cfe/trunk/test/CodeGenCXX/virtual-function-calls.cpp Wed Sep 30 >> 22:50:41 2015 >> @@ -1,4 +1,5 @@ >> // RUN: %clang_cc1 %s -triple %itanium_abi_triple -std=c++11 -emit-llvm >> -o - | FileCheck %s >> +// RUN: %clang_cc1 %s -triple %itanium_abi_triple -std=c++11 -emit-llvm >> -o - -fstrict-vtable-pointers -O1 | FileCheck >> --check-prefix=CHECK-INVARIANT %s >> >> // PR5021 >> namespace PR5021 { >> @@ -42,10 +43,14 @@ namespace VirtualNoreturn { >> [[noreturn]] virtual void f(); >>}; >> >> - // CHECK: @_ZN15VirtualNoreturn1f >> + // CHECK-LABEL: @_ZN15VirtualNoreturn1f >> + // CHECK-INVARIANT-LABEL: define void @_ZN15VirtualNoreturn1f >>void f(A *p) { >> p->f(); >> // CHECK: call {{.*}}void %{{[^#]*$}} >> // CHECK-NOT: unreachable >> +// CHECK-INVARIANT: load {{.*}} align 8, !invariant.load >> ![[EMPTY_NODE:[0-9]]] >>} >> } >> + >> +// CHECK-INVARIANT: ![[EMPTY_NODE]] = !{} >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r249048 - [CMake] Don't include the test directories if CLANG_INCLUDE_TESTS is Off
Author: cbieneman Date: Thu Oct 1 13:16:56 2015 New Revision: 249048 URL: http://llvm.org/viewvc/llvm-project?rev=249048&view=rev Log: [CMake] Don't include the test directories if CLANG_INCLUDE_TESTS is Off This matches Clang's behavior. Modified: clang-tools-extra/trunk/CMakeLists.txt Modified: clang-tools-extra/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/CMakeLists.txt?rev=249048&r1=249047&r2=249048&view=diff == --- clang-tools-extra/trunk/CMakeLists.txt (original) +++ clang-tools-extra/trunk/CMakeLists.txt Thu Oct 1 13:16:56 2015 @@ -12,7 +12,7 @@ add_subdirectory(tool-template) # Add the common testsuite after all the tools. # TODO: Support tests with more granularity when features are off? -if(CLANG_ENABLE_STATIC_ANALYZER) +if(CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_TESTS) add_subdirectory(test) add_subdirectory(unittests) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D9888: [OPENMP] Driver support for OpenMP offloading
sfantao updated this revision to Diff 36263. sfantao added a comment. This diff refactors the original patch and is rebased on top of the latests offloading changes inserted for CUDA. Here I don't touch the CUDA support. I tried, however, to have the implementation modular enough so that it could eventually be combined with the CUDA implementation. In my view OpenMP offloading is more general in the sense that it does not refer to a given tool chain, instead it uses existing toolchains to generate code for offloading devices. So, I believe that a tool chain (which I did not include in this patch) targeting NVPTX will be able to handle both CUDA and OpenMP offloading models. Chris, Art, I understand you have worked out the latest CUDA changes so any feedback from you is greatly appreciated! Here are few more details about this diff: Add tool to bundle and unbundle corresponding host and device files into a single one. One of the goals of OpenMP offloading is to enable users to offload with little effort, by annotating the code with a few pragmas. I'd also like to save users the trouble of changing their existent applications' build system. So having the compiler always return a single file instead of one for the host and each target even if the user is doing separate compilation is desirable. This diff includes a tool named clang-offload-bundled (happy to change the name or even include it in the driver if someone thinks it is the best direction to go) that is used on all input files that are not source files to unbundle them, and on top level jobs that are not linking jobs to bundle the results obtained for host and each target. The format of the bundled files is currently very simple: text formats are concatenated with comments that have a magic string and target identifying triple in between, and binary formats have a header that contains the triple and the offset and size of the code for host and each target. This tool still has to be improved in the future to deal with archive files so that each individual file in the archive is properly dealt with. We see that archives are very commonly used in current application to combine separate compilation results. So I'm convinced users would enjoy this feature. The building of the driver actions is unchanged. I don't create device specific actions. Instead only the bundling/unbundling are inserted as first or last action if the file type requires that. Add offloading kind to `ToolChain` Offloading does not require a new toolchain to be created. Existent toolchains are used and the offloading kind is used to drive specific behavior in each toolchain so that valid device code is generated. This is a major difference from what is currently done for CUDA. But I guess the CUDA implementation easily fits this design and the Nvidia GPU toolchain could be reused for both CUDA and OpenMP offloading. Use Job results cache to easily use host results in device actions and vice-versa. An array of the results for each job is kept so that the device job can use the result previously generated for the host and used it as input or vice-versa. In OpenMP the device declarations have be communicated from the host frontend to the device frontend. So this is used to conveniently pass that information. Unlike CUDA, OpenMP doesn't have already outline functions with "device" attributes that the frontend can rely on to make the decision on what to be emitted or not. The result cache can also be updated to keep the required information for the CUDA implementation to decide host/device binaries combining (injection is the term used in the code). I don't have a concrete proposal for that however, given that is not clear to me what are the plans for CUDA to support separate compilation, I understand that the CUDA binary is inserted directly in host IR (Art, can you shed some light on this?). Use compiler generated linker script to do the device/host code combining and correctly support separate compilation. Currently the OpenMP support in the toolchains is only implemented for Generic GCC targets and a linker script is used to embed the resulting device images into the host binary ELF sections. Also, the linker script defines the symbols that are emitted during code generation so that the address of the images can be easily retrieved. Minor refactoring of the existing code to enable reusing. I've outlined some of the exiting code into static function so that it could be reused by the new offloading related hooks. Any comments/remarks are very welcome! Thanks! Samuel http://reviews.llvm.org/D9888 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Driver/Action.h include/clang/Driver/CC1Options.td include/clang/Driver/Driver.h include/clang/Driver/Options.td include/clang/Driver/ToolChain.h include/clang/Driver/Types.h lib/Driver/Action.cpp lib/Drive
r249053 - Teach -Wtautological-overlap-compare about enums
Author: gbiv Date: Thu Oct 1 13:47:52 2015 New Revision: 249053 URL: http://llvm.org/viewvc/llvm-project?rev=249053&view=rev Log: Teach -Wtautological-overlap-compare about enums Prior to this patch, -Wtautological-overlap-compare would only warn us if there was a sketchy logical comparison between variables and IntegerLiterals. This patch makes -Wtautological-overlap-compare aware of EnumConstantDecls, so it can apply the same logic to them. Modified: cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/test/Sema/warn-overlap.c Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=249053&r1=249052&r2=249053&view=diff == --- cfe/trunk/lib/Analysis/CFG.cpp (original) +++ cfe/trunk/lib/Analysis/CFG.cpp Thu Oct 1 13:47:52 2015 @@ -39,6 +39,78 @@ static SourceLocation GetEndLoc(Decl *D) return D->getLocation(); } +/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral +/// or EnumConstantDecl from the given Expr. If it fails, returns nullptr. +const Expr *tryTransformToIntOrEnumConstant(const Expr *E) { + E = E->IgnoreParens(); + if (isa(E)) +return E; + if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) +return isa(DR->getDecl()) ? DR : nullptr; + return nullptr; +} + +/// Tries to interpret a binary operator into `Decl Op Expr` form, if Expr is +/// an integer literal or an enum constant. +/// +/// If this fails, at least one of the returned DeclRefExpr or Expr will be +/// null. +static std::tuple +tryNormalizeBinaryOperator(const BinaryOperator *B) { + BinaryOperatorKind Op = B->getOpcode(); + + const Expr *MaybeDecl = B->getLHS(); + const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS()); + // Expr looked like `0 == Foo` instead of `Foo == 0` + if (Constant == nullptr) { +// Flip the operator +if (Op == BO_GT) + Op = BO_LT; +else if (Op == BO_GE) + Op = BO_LE; +else if (Op == BO_LT) + Op = BO_GT; +else if (Op == BO_LE) + Op = BO_GE; + +MaybeDecl = B->getRHS(); +Constant = tryTransformToIntOrEnumConstant(B->getLHS()); + } + + auto *D = dyn_cast(MaybeDecl->IgnoreParenImpCasts()); + return std::make_tuple(D, Op, Constant); +} + +/// For an expression `x == Foo && x == Bar`, this determines whether the +/// `Foo` and `Bar` are either of the same enumeration type, or both integer +/// literals. +/// +/// It's an error to pass this arguments that are not either IntegerLiterals +/// or DeclRefExprs (that have decls of type EnumConstantDecl) +static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) { + // User intent isn't clear if they're mixing int literals with enum + // constants. + if (isa(E1) != isa(E2)) +return false; + + // Integer literal comparisons, regardless of literal type, are acceptable. + if (isa(E1)) +return true; + + // IntegerLiterals are handled above and only EnumConstantDecls are expected + // beyond this point + assert(isa(E1) && isa(E2)); + auto *Decl1 = cast(E1)->getDecl(); + auto *Decl2 = cast(E2)->getDecl(); + + assert(isa(Decl1) && isa(Decl2)); + const DeclContext *DC1 = Decl1->getDeclContext(); + const DeclContext *DC2 = Decl2->getDeclContext(); + + assert(isa(DC1) && isa(DC2)); + return DC1 == DC2; +} + class CFGBuilder; /// The CFG builder uses a recursive algorithm to build the CFG. When @@ -694,56 +766,35 @@ private: if (!LHS->isComparisonOp() || !RHS->isComparisonOp()) return TryResult(); -BinaryOperatorKind BO1 = LHS->getOpcode(); -const DeclRefExpr *Decl1 = -dyn_cast(LHS->getLHS()->IgnoreParenImpCasts()); -const IntegerLiteral *Literal1 = -dyn_cast(LHS->getRHS()->IgnoreParens()); -if (!Decl1 && !Literal1) { - if (BO1 == BO_GT) -BO1 = BO_LT; - else if (BO1 == BO_GE) -BO1 = BO_LE; - else if (BO1 == BO_LT) -BO1 = BO_GT; - else if (BO1 == BO_LE) -BO1 = BO_GE; - Decl1 = dyn_cast(LHS->getRHS()->IgnoreParenImpCasts()); - Literal1 = dyn_cast(LHS->getLHS()->IgnoreParens()); -} +const DeclRefExpr *Decl1; +const Expr *Expr1; +BinaryOperatorKind BO1; +std::tie(Decl1, BO1, Expr1) = tryNormalizeBinaryOperator(LHS); -if (!Decl1 || !Literal1) +if (!Decl1 || !Expr1) return TryResult(); -BinaryOperatorKind BO2 = RHS->getOpcode(); -const DeclRefExpr *Decl2 = -dyn_cast(RHS->getLHS()->IgnoreParenImpCasts()); -const IntegerLiteral *Literal2 = -dyn_cast(RHS->getRHS()->IgnoreParens()); -if (!Decl2 && !Literal2) { - if (BO2 == BO_GT) -BO2 = BO_LT; - else if (BO2 == BO_GE) -BO2 = BO_LE; - else if (BO2 == BO_LT) -BO2 = BO_GT; - else if (BO2 == BO_LE) -BO2 = BO_GE; - Decl2 = dyn_cast(RHS->getRHS()->IgnoreParenImpCasts()); - Literal2 = dyn_cast(RHS->getLHS()->IgnoreParens()); -
Re: [PATCH] D13157: Teach -Wtautological-overlap-compare about enums
george.burgess.iv closed this revision. george.burgess.iv marked 4 inline comments as done. george.burgess.iv added a comment. Changed code to address all feedback + committed as r249053. Thanks for the reviews! Comment at: lib/Analysis/CFG.cpp:49 @@ +48,3 @@ +tryNormalizeBinaryOperator(const BinaryOperator *B) { + auto TryTransformToIntOrEnumConstant = [](const Expr *E) -> const Expr * { +E = E->IgnoreParens(); rtrieu wrote: > Why is this a lambda instead of a helper function? Because it's small + super specific to `tryNormalizeBinaryOperator`, and making it a lambda lets me scope it it only to where it's useful. It's now a helper function. :) Comment at: lib/Analysis/CFG.cpp:99-104 @@ +98,8 @@ + // Currently we're only given EnumConstantDecls or IntegerLiterals + auto *C1 = cast(cast(A)->getDecl()); + auto *C2 = cast(cast(B)->getDecl()); + + const TagDecl *E1 = TagDecl::castFromDeclContext(C1->getDeclContext()); + const TagDecl *E2 = TagDecl::castFromDeclContext(C2->getDeclContext()); + return E1 == E2; +} rtrieu wrote: > There's a few extra casts in here, plus some blind conversions between types. > Add your assumptions for the types in asserts. Also, DeclContext should use > cast<> to get to Decl types. I recommend the following: > > ``` > assert(isa(E1) && isa(E2)); > auto *Decl1 = cast(E1)->getDecl(); > auto *Decl2 = cast(E2)->getDecl(); > > assert(isa(Decl1) && isa(Decl2)); > const DeclContext *DC1 = Decl1->getDeclContext(); > const DeclContext *DC2 = Decl2->getDeclContext(); > > assert(isa(DC1) && isa(DC2)); > return DC1 == DC2; > > ``` I was under the impression that the `cast(Bar)` asserts `isa(Bar)` for me, so I thought that asserts like those would just be redundant. Changed to your version anyway :) http://reviews.llvm.org/D13157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13157: Teach -Wtautological-overlap-compare about enums
On Thu, Oct 1, 2015 at 2:50 PM, George Burgess IV wrote: > george.burgess.iv closed this revision. > george.burgess.iv marked 4 inline comments as done. > george.burgess.iv added a comment. > > Changed code to address all feedback + committed as r249053. Thanks for the > reviews! > > > > Comment at: lib/Analysis/CFG.cpp:49 > @@ +48,3 @@ > +tryNormalizeBinaryOperator(const BinaryOperator *B) { > + auto TryTransformToIntOrEnumConstant = [](const Expr *E) -> const Expr * { > +E = E->IgnoreParens(); > > rtrieu wrote: >> Why is this a lambda instead of a helper function? > Because it's small + super specific to `tryNormalizeBinaryOperator`, and > making it a lambda lets me scope it it only to where it's useful. It's now a > helper function. :) > > > Comment at: lib/Analysis/CFG.cpp:99-104 > @@ +98,8 @@ > + // Currently we're only given EnumConstantDecls or IntegerLiterals > + auto *C1 = cast(cast(A)->getDecl()); > + auto *C2 = cast(cast(B)->getDecl()); > + > + const TagDecl *E1 = TagDecl::castFromDeclContext(C1->getDeclContext()); > + const TagDecl *E2 = TagDecl::castFromDeclContext(C2->getDeclContext()); > + return E1 == E2; > +} > > rtrieu wrote: >> There's a few extra casts in here, plus some blind conversions between >> types. Add your assumptions for the types in asserts. Also, DeclContext >> should use cast<> to get to Decl types. I recommend the following: >> >> ``` >> assert(isa(E1) && isa(E2)); >> auto *Decl1 = cast(E1)->getDecl(); >> auto *Decl2 = cast(E2)->getDecl(); >> >> assert(isa(Decl1) && isa(Decl2)); >> const DeclContext *DC1 = Decl1->getDeclContext(); >> const DeclContext *DC2 = Decl2->getDeclContext(); >> >> assert(isa(DC1) && isa(DC2)); >> return DC1 == DC2; >> >> ``` > I was under the impression that the `cast(Bar)` asserts `isa(Bar)` > for me, so I thought that asserts like those would just be redundant. Changed > to your version anyway :) cast<> already does assert, so those new asserts are redundant and should be removed. Sorry, I didn't catch that suggestion earlier. I would recommend: auto *Decl1 = cast(E1)->getDecl(); auto *Decl2 = cast(E2)->getDecl(); assert(isa(Decl1) && isa(Decl2)); return Decl1->getDeclContext() == Decl2->getDeclContext(); I don't think there's a point to asserting that an EnumConstantDecl has a declaration context that is an EnumDecl -- if that weren't true, we'd have broken long before reaching this code. ~Aaron > > > http://reviews.llvm.org/D13157 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13157: Teach -Wtautological-overlap-compare about enums
rtrieu added a comment. Next time, add > Differential Revision: to your commit and Phabricator will close the diff automatically. http://llvm.org/docs/Phabricator.html Comment at: lib/Analysis/CFG.cpp:99-104 @@ +98,8 @@ + // Currently we're only given EnumConstantDecls or IntegerLiterals + auto *C1 = cast(cast(A)->getDecl()); + auto *C2 = cast(cast(B)->getDecl()); + + const TagDecl *E1 = TagDecl::castFromDeclContext(C1->getDeclContext()); + const TagDecl *E2 = TagDecl::castFromDeclContext(C2->getDeclContext()); + return E1 == E2; +} george.burgess.iv wrote: > rtrieu wrote: > > There's a few extra casts in here, plus some blind conversions between > > types. Add your assumptions for the types in asserts. Also, DeclContext > > should use cast<> to get to Decl types. I recommend the following: > > > > ``` > > assert(isa(E1) && isa(E2)); > > auto *Decl1 = cast(E1)->getDecl(); > > auto *Decl2 = cast(E2)->getDecl(); > > > > assert(isa(Decl1) && isa(Decl2)); > > const DeclContext *DC1 = Decl1->getDeclContext(); > > const DeclContext *DC2 = Decl2->getDeclContext(); > > > > assert(isa(DC1) && isa(DC2)); > > return DC1 == DC2; > > > > ``` > I was under the impression that the `cast(Bar)` asserts `isa(Bar)` > for me, so I thought that asserts like those would just be redundant. Changed > to your version anyway :) You are correct, 'cast(Bar)' does assert 'isa(Bar)'. However, when Bar is not Foo, using the assert here means the crash will produce a backtrace will point straight to this function instead of an assert that points deep into the casting functions. http://reviews.llvm.org/D13157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13157: Teach -Wtautological-overlap-compare about enums
On Thu, Oct 1, 2015 at 3:01 PM, Richard Trieu wrote: > rtrieu added a comment. > > Next time, add > >> Differential Revision: > > > to your commit and Phabricator will close the diff automatically. > > http://llvm.org/docs/Phabricator.html > > > > Comment at: lib/Analysis/CFG.cpp:99-104 > @@ +98,8 @@ > + // Currently we're only given EnumConstantDecls or IntegerLiterals > + auto *C1 = cast(cast(A)->getDecl()); > + auto *C2 = cast(cast(B)->getDecl()); > + > + const TagDecl *E1 = TagDecl::castFromDeclContext(C1->getDeclContext()); > + const TagDecl *E2 = TagDecl::castFromDeclContext(C2->getDeclContext()); > + return E1 == E2; > +} > > george.burgess.iv wrote: >> rtrieu wrote: >> > There's a few extra casts in here, plus some blind conversions between >> > types. Add your assumptions for the types in asserts. Also, DeclContext >> > should use cast<> to get to Decl types. I recommend the following: >> > >> > ``` >> > assert(isa(E1) && isa(E2)); >> > auto *Decl1 = cast(E1)->getDecl(); >> > auto *Decl2 = cast(E2)->getDecl(); >> > >> > assert(isa(Decl1) && isa(Decl2)); >> > const DeclContext *DC1 = Decl1->getDeclContext(); >> > const DeclContext *DC2 = Decl2->getDeclContext(); >> > >> > assert(isa(DC1) && isa(DC2)); >> > return DC1 == DC2; >> > >> > ``` >> I was under the impression that the `cast(Bar)` asserts `isa(Bar)` >> for me, so I thought that asserts like those would just be redundant. >> Changed to your version anyway :) > You are correct, 'cast(Bar)' does assert 'isa(Bar)'. However, when > Bar is not Foo, using the assert here means the crash will produce a > backtrace will point straight to this function instead of an assert that > points deep into the casting functions. Doubling the expense for assert builds so that we get a slightly better stack trace in the event our assumptions are wrong doesn't seem like a good tradeoff. It means everyone running an assert build pays the price twice to save a few moments of scanning the backtrace in a situation that's (hopefully) highly unlikely to occur in practice. ~Aaron > > > http://reviews.llvm.org/D13157 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r249060 - Simplify Sema::DeduceFunctionTypeFromReturnExpr and eliminae a redundant check.
Author: dgregor Date: Thu Oct 1 14:52:44 2015 New Revision: 249060 URL: http://llvm.org/viewvc/llvm-project?rev=249060&view=rev Log: Simplify Sema::DeduceFunctionTypeFromReturnExpr and eliminae a redundant check. NFC Modified: cfe/trunk/lib/Sema/SemaStmt.cpp Modified: cfe/trunk/lib/Sema/SemaStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=249060&r1=249059&r2=249060&view=diff == --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) +++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Oct 1 14:52:44 2015 @@ -2987,14 +2987,9 @@ bool Sema::DeduceFunctionTypeFromReturnE // statement with a non-type-dependent operand. assert(AT->isDeduced() && "should have deduced to dependent type"); return false; - } else if (RetExpr) { -// If the deduction is for a return statement and the initializer is -// a braced-init-list, the program is ill-formed. -if (isa(RetExpr)) { - Diag(RetExpr->getExprLoc(), diag::err_auto_fn_return_init_list); - return true; -} + } + if (RetExpr) { // Otherwise, [...] deduce a value for U using the rules of template // argument deduction. DeduceAutoResult DAR = DeduceAutoType(OrigResultType, RetExpr, Deduced); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13157: Teach -Wtautological-overlap-compare about enums
> Next time, add Differential Revision: to your commit and Phabricator will close the diff automatically. Ooh, shiny. Thanks for letting me know! > Doubling the expense for assert builds so that we get a slightly better stack trace in the event our assumptions are wrong doesn't seem like a good tradeoff I'd think that the optimizer would be able to eliminate the redundant checks for Release+Asserts builds, and that this code wouldn't get hit often enough for it to make a meaningful impact on the execution time of a Debug build of clang. That said, I'm happy to potentially make things a bit faster if that's what we choose to do. :) Richard, do you feel strongly one way or the other? If not, I'll go ahead and take them out. George On Thu, Oct 1, 2015 at 12:03 PM, Aaron Ballman wrote: > On Thu, Oct 1, 2015 at 3:01 PM, Richard Trieu wrote: > > rtrieu added a comment. > > > > Next time, add > > > >> Differential Revision: > > > > > > to your commit and Phabricator will close the diff automatically. > > > > http://llvm.org/docs/Phabricator.html > > > > > > > > Comment at: lib/Analysis/CFG.cpp:99-104 > > @@ +98,8 @@ > > + // Currently we're only given EnumConstantDecls or IntegerLiterals > > + auto *C1 = cast(cast(A)->getDecl()); > > + auto *C2 = cast(cast(B)->getDecl()); > > + > > + const TagDecl *E1 = > TagDecl::castFromDeclContext(C1->getDeclContext()); > > + const TagDecl *E2 = > TagDecl::castFromDeclContext(C2->getDeclContext()); > > + return E1 == E2; > > +} > > > > george.burgess.iv wrote: > >> rtrieu wrote: > >> > There's a few extra casts in here, plus some blind conversions > between types. Add your assumptions for the types in asserts. Also, > DeclContext should use cast<> to get to Decl types. I recommend the > following: > >> > > >> > ``` > >> > assert(isa(E1) && isa(E2)); > >> > auto *Decl1 = cast(E1)->getDecl(); > >> > auto *Decl2 = cast(E2)->getDecl(); > >> > > >> > assert(isa(Decl1) && > isa(Decl2)); > >> > const DeclContext *DC1 = Decl1->getDeclContext(); > >> > const DeclContext *DC2 = Decl2->getDeclContext(); > >> > > >> > assert(isa(DC1) && isa(DC2)); > >> > return DC1 == DC2; > >> > > >> > ``` > >> I was under the impression that the `cast(Bar)` asserts > `isa(Bar)` for me, so I thought that asserts like those would just be > redundant. Changed to your version anyway :) > > You are correct, 'cast(Bar)' does assert 'isa(Bar)'. However, > when Bar is not Foo, using the assert here means the crash will produce a > backtrace will point straight to this function instead of an assert that > points deep into the casting functions. > > Doubling the expense for assert builds so that we get a slightly > better stack trace in the event our assumptions are wrong doesn't seem > like a good tradeoff. It means everyone running an assert build pays > the price twice to save a few moments of scanning the backtrace in a > situation that's (hopefully) highly unlikely to occur in practice. > > ~Aaron > > > > > > > http://reviews.llvm.org/D13157 > > > > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12993: [analyzer] Add TK_EntireMemSpace invalidation trait.
This revision was automatically updated to reflect the committed changes. Closed by commit rL249063: [analyzer] Add TK_EntireMemSpace invalidation trait. (authored by dcoughlin). Changed prior to commit: http://reviews.llvm.org/D12993?vs=35170&id=36288#toc Repository: rL LLVM http://reviews.llvm.org/D12993 Files: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h === --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -1335,7 +1335,10 @@ /// Suppress pointer-escaping of a region. TK_SuppressEscape = 0x2, // Do not invalidate super region. -TK_DoNotInvalidateSuperRegion = 0x4 +TK_DoNotInvalidateSuperRegion = 0x4, +/// When applied to a MemSpaceRegion, indicates the entire memory space +/// should be invalidated. +TK_EntireMemSpace = 0x8 // Do not forget to extend StorageTypeForKinds if number of traits exceed // the number of bits StorageTypeForKinds can store. Index: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -650,35 +650,25 @@ RegionBindingsRef B; -private: - GlobalsFilterKind GlobalsFilter; protected: const ClusterBindings *getCluster(const MemRegion *R) { return B.lookup(R); } - /// Returns true if the memory space of the given region is one of the global - /// regions specially included at the start of analysis. - bool isInitiallyIncludedGlobalRegion(const MemRegion *R) { -switch (GlobalsFilter) { -case GFK_None: - return false; -case GFK_SystemOnly: - return isa(R->getMemorySpace()); -case GFK_All: - return isa(R->getMemorySpace()); -} - -llvm_unreachable("unknown globals filter"); + /// Returns true if all clusters in the given memspace should be initially + /// included in the cluster analysis. Subclasses may provide their + /// own implementation. + bool includeEntireMemorySpace(const MemRegion *Base) { +return false; } public: ClusterAnalysis(RegionStoreManager &rm, ProgramStateManager &StateMgr, - RegionBindingsRef b, GlobalsFilterKind GFK) + RegionBindingsRef b ) : RM(rm), Ctx(StateMgr.getContext()), svalBuilder(StateMgr.getSValBuilder()), - B(b), GlobalsFilter(GFK) {} + B(b) {} RegionBindingsRef getRegionBindings() const { return B; } @@ -696,8 +686,9 @@ assert(!Cluster.isEmpty() && "Empty clusters should be removed"); static_cast(this)->VisitAddedToCluster(Base, Cluster); - // If this is an interesting global region, add it the work list up front. - if (isInitiallyIncludedGlobalRegion(Base)) + // If the base's memspace should be entirely invalidated, add the cluster + // to the workspace up front. + if (static_cast(this)->includeEntireMemorySpace(Base)) AddToWorkList(WorkListElement(Base), &Cluster); } } @@ -940,6 +931,7 @@ InvalidatedSymbols &IS; RegionAndSymbolInvalidationTraits &ITraits; StoreManager::InvalidatedRegions *Regions; + GlobalsFilterKind GlobalsFilter; public: invalidateRegionsWorker(RegionStoreManager &rm, ProgramStateManager &stateMgr, @@ -950,15 +942,24 @@ RegionAndSymbolInvalidationTraits &ITraitsIn, StoreManager::InvalidatedRegions *r, GlobalsFilterKind GFK) -: ClusterAnalysis(rm, stateMgr, b, GFK), - Ex(ex), Count(count), LCtx(lctx), IS(is), ITraits(ITraitsIn), Regions(r){} + : ClusterAnalysis(rm, stateMgr, b), + Ex(ex), Count(count), LCtx(lctx), IS(is), ITraits(ITraitsIn), Regions(r), + GlobalsFilter(GFK) {} void VisitCluster(const MemRegion *baseR, const ClusterBindings *C); void VisitBinding(SVal V); using ClusterAnalysis::AddToWorkList; bool AddToWorkList(const MemRegion *R); + + /// Returns true if all clusters in the memory space for \p Base should be + /// be invalidated. + bool includeEntireMemorySpace(const MemRegion *Base); + + /// Returns true if the memory space of the given region is one of the global + /// regions specially included at the start of invalidation. + bool isInitiallyIncludedGlobalRegion(const MemRegion *R); }; } @@ -1159,6 +1160,29 @@ B = B.addBinding(baseR, BindingKey::Direct, V); } +bool invalidateRegionsWorker::isInitiallyIncludedGlobalRegion( +const MemRegion *R) { + switch (GlobalsFilter) { + case GFK_None: +return false; + case GFK_SystemOnly: +return isa(R->getMemorySpace()); + case GFK_All: +re
r249063 - [analyzer] Add TK_EntireMemSpace invalidation trait.
Author: dcoughlin Date: Thu Oct 1 15:09:11 2015 New Revision: 249063 URL: http://llvm.org/viewvc/llvm-project?rev=249063&view=rev Log: [analyzer] Add TK_EntireMemSpace invalidation trait. This commit supports Sean Eveson's work on loop widening. It is NFC for now. It adds a new TK_EntireMemSpace invalidation trait that, when applied to a MemSpaceRegion, indicates that the entire memory space should be invalidated. Clients can add this trait before invalidating. For example: RegionAndSymbolInvalidationTraits ITraits; ITraits.setTrait(MRMgr.getStackLocalsRegion(STC), RegionAndSymbolInvalidationTraits::TK_EntireMemSpace); This commit updates the existing logic invalidating global memspace regions for calls to additionally handle arbitrary memspaces. When generating initial clusters during cluster analysis we now add a cluster to the worklist if the memspace for its base is marked with TK_EntireMemSpace. This also moves the logic for invalidating globals from ClusterAnalysis to invalidateRegionsWorker so that it is not shared with removeDeadBindingsWorker. There are no explicit tests with this patch -- but when applied to Sean's patch for loop widening in http://reviews.llvm.org/D12358 and after updating his code to set the trait, the failing tests in that patch now pass. Differential Revision: http://reviews.llvm.org/D12993 Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=249063&r1=249062&r2=249063&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Thu Oct 1 15:09:11 2015 @@ -1335,7 +1335,10 @@ public: /// Suppress pointer-escaping of a region. TK_SuppressEscape = 0x2, // Do not invalidate super region. -TK_DoNotInvalidateSuperRegion = 0x4 +TK_DoNotInvalidateSuperRegion = 0x4, +/// When applied to a MemSpaceRegion, indicates the entire memory space +/// should be invalidated. +TK_EntireMemSpace = 0x8 // Do not forget to extend StorageTypeForKinds if number of traits exceed // the number of bits StorageTypeForKinds can store. Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=249063&r1=249062&r2=249063&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Oct 1 15:09:11 2015 @@ -650,35 +650,25 @@ protected: RegionBindingsRef B; -private: - GlobalsFilterKind GlobalsFilter; protected: const ClusterBindings *getCluster(const MemRegion *R) { return B.lookup(R); } - /// Returns true if the memory space of the given region is one of the global - /// regions specially included at the start of analysis. - bool isInitiallyIncludedGlobalRegion(const MemRegion *R) { -switch (GlobalsFilter) { -case GFK_None: - return false; -case GFK_SystemOnly: - return isa(R->getMemorySpace()); -case GFK_All: - return isa(R->getMemorySpace()); -} - -llvm_unreachable("unknown globals filter"); + /// Returns true if all clusters in the given memspace should be initially + /// included in the cluster analysis. Subclasses may provide their + /// own implementation. + bool includeEntireMemorySpace(const MemRegion *Base) { +return false; } public: ClusterAnalysis(RegionStoreManager &rm, ProgramStateManager &StateMgr, - RegionBindingsRef b, GlobalsFilterKind GFK) + RegionBindingsRef b ) : RM(rm), Ctx(StateMgr.getContext()), svalBuilder(StateMgr.getSValBuilder()), - B(b), GlobalsFilter(GFK) {} + B(b) {} RegionBindingsRef getRegionBindings() const { return B; } @@ -696,8 +686,9 @@ public: assert(!Cluster.isEmpty() && "Empty clusters should be removed"); static_cast(this)->VisitAddedToCluster(Base, Cluster); - // If this is an interesting global region, add it the work list up front. - if (isInitiallyIncludedGlobalRegion(Base)) + // If the base's memspace should be entirely invalidated, add the cluster + // to the workspace up front. + if (static_cast(this)->includeEntireMemorySpace(Base)) AddToWorkList(WorkListElement(Base), &Cluster); } } @@ -940,6 +931,7 @@ class invalidateRegionsWorker : public C InvalidatedSymbols &IS; RegionAndSymbolInvalidationTraits &ITraits; StoreManager::InvalidatedRegions *Regions; + GlobalsFilterK
r249065 - Perform Objective-C lifetime adjustments before comparing deduced lambda result types.
Author: dgregor Date: Thu Oct 1 15:20:47 2015 New Revision: 249065 URL: http://llvm.org/viewvc/llvm-project?rev=249065&view=rev Log: Perform Objective-C lifetime adjustments before comparing deduced lambda result types. Objective-C ARC lifetime qualifiers are dropped when canonicalizing function types. Perform the same adjustment before comparing the deduced result types of lambdas. Fixes rdar://problem/22344904. Added: cfe/trunk/test/SemaObjCXX/cxx1y-lambda.mm Modified: cfe/trunk/include/clang/AST/ASTContext.h cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/lib/Sema/SemaStmt.cpp Modified: cfe/trunk/include/clang/AST/ASTContext.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=249065&r1=249064&r2=249065&view=diff == --- cfe/trunk/include/clang/AST/ASTContext.h (original) +++ cfe/trunk/include/clang/AST/ASTContext.h Thu Oct 1 15:20:47 2015 @@ -968,6 +968,9 @@ public: const FunctionType *adjustFunctionType(const FunctionType *Fn, FunctionType::ExtInfo EInfo); + /// Adjust the given function result type. + CanQualType getCanonicalFunctionResultType(QualType ResultType) const; + /// \brief Change the result type of a function type once it is deduced. void adjustDeducedFunctionResultType(FunctionDecl *FD, QualType ResultType); Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=249065&r1=249064&r2=249065&view=diff == --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Oct 1 15:20:47 2015 @@ -2990,6 +2990,21 @@ static bool isCanonicalResultType(QualTy T.getObjCLifetime() == Qualifiers::OCL_ExplicitNone); } +CanQualType +ASTContext::getCanonicalFunctionResultType(QualType ResultType) const { + CanQualType CanResultType = getCanonicalType(ResultType); + + // Canonical result types do not have ARC lifetime qualifiers. + if (CanResultType.getQualifiers().hasObjCLifetime()) { +Qualifiers Qs = CanResultType.getQualifiers(); +Qs.removeObjCLifetime(); +return CanQualType::CreateUnsafe( + getQualifiedType(CanResultType.getUnqualifiedType(), Qs)); + } + + return CanResultType; +} + QualType ASTContext::getFunctionType(QualType ResultTy, ArrayRef ArgArray, const FunctionProtoType::ExtProtoInfo &EPI) const { @@ -3027,14 +3042,8 @@ ASTContext::getFunctionType(QualType Res CanonicalEPI.HasTrailingReturn = false; CanonicalEPI.ExceptionSpec = FunctionProtoType::ExceptionSpecInfo(); -// Result types do not have ARC lifetime qualifiers. -QualType CanResultTy = getCanonicalType(ResultTy); -if (ResultTy.getQualifiers().hasObjCLifetime()) { - Qualifiers Qs = CanResultTy.getQualifiers(); - Qs.removeObjCLifetime(); - CanResultTy = getQualifiedType(CanResultTy.getUnqualifiedType(), Qs); -} - +// Adjust the canonical function result type. +CanQualType CanResultTy = getCanonicalFunctionResultType(ResultTy); Canonical = getFunctionType(CanResultTy, CanonicalArgs, CanonicalEPI); // Get the new insert position for the node we care about. Modified: cfe/trunk/lib/Sema/SemaLambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=249065&r1=249064&r2=249065&view=diff == --- cfe/trunk/lib/Sema/SemaLambda.cpp (original) +++ cfe/trunk/lib/Sema/SemaLambda.cpp Thu Oct 1 15:20:47 2015 @@ -685,7 +685,8 @@ void Sema::deduceClosureReturnType(Captu QualType ReturnType = (RetE ? RetE->getType() : Context.VoidTy).getUnqualifiedType(); -if (Context.hasSameType(ReturnType, CSI.ReturnType)) +if (Context.getCanonicalFunctionResultType(ReturnType) == + Context.getCanonicalFunctionResultType(CSI.ReturnType)) continue; // FIXME: This is a poor diagnostic for ReturnStmts without expressions. Modified: cfe/trunk/lib/Sema/SemaStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=249065&r1=249064&r2=249065&view=diff == --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) +++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Oct 1 15:20:47 2015 @@ -3028,8 +3028,11 @@ bool Sema::DeduceFunctionTypeFromReturnE // the program is ill-formed. if (AT->isDeduced() && !FD->isInvalidDecl()) { AutoType *NewAT = Deduced->getContainedAutoType(); -if (!FD->isDependentContext() && -!Context.hasSameType(AT->getDeducedType(), NewAT->getDeducedType())) { +CanQualType OldDeducedType = Context.getCanonicalFunctionResultType( + AT->getDeducedTy
Re: [PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast
mgehre updated this revision to Diff 36291. mgehre added a comment. Renamed the check to cppcoreguidelines-pro-type-reinterpret-cast http://reviews.llvm.org/D13313 Files: clang-tidy/CMakeLists.txt clang-tidy/Makefile clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/CppCoreGuidelinesModule.cpp clang-tidy/cppcoreguidelines/Makefile clang-tidy/cppcoreguidelines/ProTypeReinterpretCast.cpp clang-tidy/cppcoreguidelines/ProTypeReinterpretCast.h clang-tidy/tool/CMakeLists.txt clang-tidy/tool/ClangTidyMain.cpp clang-tidy/tool/Makefile docs/clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-pro-type-reinterpret-cast.cpp Index: test/clang-tidy/cppcoreguidelines-pro-type-reinterpret-cast.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-type-reinterpret-cast.cpp @@ -0,0 +1,6 @@ +// RUN: %python %S/check_clang_tidy.py %s cppcoreguidelines-pro-type-reinterpret-cast %t + +int i = 0; +void* j; +void f() { j = reinterpret_cast(i); } +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast (C++ Core Guidelines, rule Type.1) [cppcoreguidelines-pro-type-reinterpret-cast] Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -2,6 +2,7 @@ = .. toctree:: + cppcoreguidelines-pro-type-reinterpret-cast google-build-explicit-make-pair google-build-namespaces google-build-using-namespace Index: docs/clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast.rst === --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast.rst @@ -0,0 +1,9 @@ +cppcoreguidelines-pro-type-reinterpret-cast +=== + +This check flags all uses of reinterpret_cast in C++ code. + +Use of these casts can violate type safety and cause the program to access a variable that is actually of type X to be accessed as if it were of an unrelated type Z. + +This rule is part of the "Type safety" profile of the C++ Core Guidelines, see +https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type1-dont-use-reinterpret_cast. Index: clang-tidy/tool/Makefile === --- clang-tidy/tool/Makefile +++ clang-tidy/tool/Makefile @@ -16,7 +16,9 @@ include $(CLANG_LEVEL)/../../Makefile.config LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader support mc option -USEDLIBS = clangTidy.a clangTidyLLVMModule.a clangTidyGoogleModule.a \ +USEDLIBS = clangTidy.a clangTidyLLVMModule.a \ + clangTidyCppCoreGuidelinesModule.a \ + clangTidyGoogleModule.a \ clangTidyMiscModule.a clangTidyModernizeModule.a clangTidyReadability.a \ clangTidyUtils.a clangStaticAnalyzerFrontend.a \ clangStaticAnalyzerCheckers.a clangStaticAnalyzerCore.a \ Index: clang-tidy/tool/ClangTidyMain.cpp === --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -352,6 +352,11 @@ static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination = LLVMModuleAnchorSource; +// This anchor is used to force the linker to link the CppCoreGuidelinesModule. +extern volatile int CppCoreGuidelinesModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination = +CppCoreGuidelinesModuleAnchorSource; + // This anchor is used to force the linker to link the GoogleModule. extern volatile int GoogleModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED GoogleModuleAnchorDestination = Index: clang-tidy/tool/CMakeLists.txt === --- clang-tidy/tool/CMakeLists.txt +++ clang-tidy/tool/CMakeLists.txt @@ -10,6 +10,7 @@ clangASTMatchers clangBasic clangTidy + clangTidyCppCoreGuidelinesModule clangTidyGoogleModule clangTidyLLVMModule clangTidyMiscModule Index: clang-tidy/cppcoreguidelines/ProTypeReinterpretCast.h === --- /dev/null +++ clang-tidy/cppcoreguidelines/ProTypeReinterpretCast.h @@ -0,0 +1,33 @@ +//===--- ProTypeReinterpretCast.h - clang-tidy--*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_REINTERPRETCAST_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_REINTERPRETCAST_H + +#include "../ClangTid