[libcxx] r248987 - Suppress array initialization warnings in std::experimental::apply tests

2015-10-01 Thread Eric Fiselier via cfe-commits
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

2015-10-01 Thread Eric Fiselier via cfe-commits
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

2015-10-01 Thread Gábor Horváth via cfe-commits
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.

2015-10-01 Thread Eric Fiselier via cfe-commits
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

2015-10-01 Thread Eric Fiselier via cfe-commits
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.

2015-10-01 Thread Sean Eveson via cfe-commits
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

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Eric Fiselier via cfe-commits
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

2015-10-01 Thread Manuel Klimek via cfe-commits
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

2015-10-01 Thread Eric Fiselier via cfe-commits
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.

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Manuel Klimek via cfe-commits
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

2015-10-01 Thread Renato Golin via cfe-commits
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.

2015-10-01 Thread Eric Fiselier via cfe-commits
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

2015-10-01 Thread Eric Fiselier via cfe-commits
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.

2015-10-01 Thread Angel Garcia via cfe-commits
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.

2015-10-01 Thread Angel Garcia Gomez via cfe-commits
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.

2015-10-01 Thread Angel Garcia via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Beren Minor via cfe-commits
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

2015-10-01 Thread Beren Minor via cfe-commits
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

2015-10-01 Thread Beren Minor via cfe-commits
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

2015-10-01 Thread Beren Minor via cfe-commits
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.

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Daniel Jasper via cfe-commits
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

2015-10-01 Thread Beren Minor via cfe-commits
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

2015-10-01 Thread Daniel Jasper via cfe-commits
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

2015-10-01 Thread Daniel Jasper via cfe-commits
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.

2015-10-01 Thread Alexey Bataev via cfe-commits
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

2015-10-01 Thread Eric Fiselier via cfe-commits
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.

2015-10-01 Thread Yaron Keren via cfe-commits
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.

2015-10-01 Thread Vasileios Kalintiris via cfe-commits
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.

2015-10-01 Thread Vasileios Kalintiris via cfe-commits
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

2015-10-01 Thread Aaron Ballman via cfe-commits
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.

2015-10-01 Thread Angel Garcia via cfe-commits
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.

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Angel Garcia via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Renato Golin via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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

2015-10-01 Thread Renato Golin via cfe-commits
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.

2015-10-01 Thread Angel Garcia Gomez via cfe-commits
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

2015-10-01 Thread Alexander Kornienko via cfe-commits
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.

2015-10-01 Thread Simon Atanasyan via cfe-commits
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.

2015-10-01 Thread Simon Atanasyan via cfe-commits
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

2015-10-01 Thread Olivier Goffart via cfe-commits
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

2015-10-01 Thread Hal Finkel via cfe-commits
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

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Angel Garcia via cfe-commits
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

2015-10-01 Thread Milian Wolff via cfe-commits
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

2015-10-01 Thread Chad Rosier via cfe-commits
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.

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Angel Garcia Gomez via cfe-commits
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

2015-10-01 Thread Yaron Keren via cfe-commits
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

2015-10-01 Thread Manuel Klimek via cfe-commits
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.

2015-10-01 Thread Benjamin Kramer via cfe-commits
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))

2015-10-01 Thread Aaron Ballman via cfe-commits
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

2015-10-01 Thread Aaron Ballman via cfe-commits
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

2015-10-01 Thread Eric Christopher via cfe-commits
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.

2015-10-01 Thread Reid Kleckner via cfe-commits
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

2015-10-01 Thread Neil Hickey via cfe-commits
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.

2015-10-01 Thread Scott Egerton via cfe-commits
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.

2015-10-01 Thread Vasileios Kalintiris via cfe-commits
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.

2015-10-01 Thread Vasileios Kalintiris via cfe-commits
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

2015-10-01 Thread David Majnemer via cfe-commits
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

2015-10-01 Thread David Majnemer via cfe-commits
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

2015-10-01 Thread Strahinja Petrovic via cfe-commits
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

2015-10-01 Thread Aaron Ballman via cfe-commits
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.

2015-10-01 Thread Vasileios Kalintiris via cfe-commits
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.

2015-10-01 Thread Vasileios Kalintiris via cfe-commits
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

2015-10-01 Thread Luke Zarko via cfe-commits
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

2015-10-01 Thread Adrian Prantl via cfe-commits
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.

2015-10-01 Thread Teresa Johnson via cfe-commits
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

2015-10-01 Thread Luke Zarko via cfe-commits
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))

2015-10-01 Thread Keno Fischer via cfe-commits
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.

2015-10-01 Thread Teresa Johnson via cfe-commits
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))

2015-10-01 Thread Aaron Ballman via cfe-commits
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

2015-10-01 Thread George Rimar via cfe-commits
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

2015-10-01 Thread Nathan Wilson via cfe-commits
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

2015-10-01 Thread Piotr Padlewski via cfe-commits
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

2015-10-01 Thread Chris Bieneman via cfe-commits
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

2015-10-01 Thread Samuel Antao via cfe-commits
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

2015-10-01 Thread George Burgess IV via cfe-commits
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

2015-10-01 Thread George Burgess IV via cfe-commits
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

2015-10-01 Thread Aaron Ballman via cfe-commits
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

2015-10-01 Thread Richard Trieu via cfe-commits
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

2015-10-01 Thread Aaron Ballman via cfe-commits
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.

2015-10-01 Thread Douglas Gregor via cfe-commits
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

2015-10-01 Thread George Burgess IV via cfe-commits
> 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.

2015-10-01 Thread Devin Coughlin via cfe-commits
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.

2015-10-01 Thread Devin Coughlin via cfe-commits
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.

2015-10-01 Thread Douglas Gregor via cfe-commits
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

2015-10-01 Thread Matthias Gehre via cfe-commits
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

  1   2   >