Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

2016-09-07 Thread Anna Zaks via cfe-commits
zaks.anna added a comment.

> -fsanitize=* as a driver argument *when linking* is an explicit request to 
> link against the sanitizer runtimes.


Sanitizer users pass this option to the clang driver to get the runtime 
checking. Not all of them understand the implications and immediately realize 
that extra libraries will be linked in (which might be incompatible with other 
options they pass) or at least they are not thinking about it when using the 
feature. So I do not view this as an *explicit request* to link against the 
extra libraries. (I might be missing the point you are trying to convey by 
highlighting *when linking*. When this option is passed, we both compile and 
link differently. I view this as an option to turn on the sanitizers feature as 
a whole.)

We saw several occurrences of people passing this option to clang when building 
libsystem components and not realizing why there is a problem at link time. 
This is not a great experience, but happy linking and running into problems at 
run time would be worse.


https://reviews.llvm.org/D24048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

2016-09-07 Thread Akira Hatanaka via cfe-commits
I looked at r278501 and realized it’s doing more than just fixing the test case 
in PR28288. For example, when the following code was compiled,

vi8 = vi8 << vuc8; // vuc8: <8 x unsigned char>, vi8:  <8 x int>
vc8 = vc8 << vs4; // vc8: <8 x char>,  vs4: <4 x short>

prior to r278501, clang would reject the first statement but accept the second, 
while after r278501, it would accept the first and reject the second. gcc 6.2 
rejects both.

Was this change discussed thoroughly and is this really the behavior we want?

> On Sep 7, 2016, at 12:53 AM, Vladimir Yakovlev  wrote:
> 
> I did another fix (attached). It fixes the error and chang-check has not any 
> new errors. Please take a look.
> 
> Where can I see your fix?
> 
> ​
> 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread David Majnemer via cfe-commits
majnemer added inline comments.


Comment at: lib/AST/ExprConstant.cpp:7024-7050
@@ -7023,1 +7023,29 @@
 
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotl(Shift % BitWidth), E);
+  }
+
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+APSInt Val, Shift;
+if (!EvaluateInteger(E->getArg(0), Val, Info) ||
+!EvaluateInteger(E->getArg(1), Shift, Info))
+  return false;
+
+APSInt BitWidth(llvm::APInt(32, Val.getBitWidth()));
+return Success(Val.rotr(Shift % BitWidth), E);
+  }
+

Any reason why we need this?

Given:
  #include 

  constexpr int x = _rotl8(1, 2);

MSVC 2015 reports:
  error C2131: expression did not evaluate to a constant


https://reviews.llvm.org/D24311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

2016-09-07 Thread Justin Bogner via cfe-commits
Richard Smith  writes:
> My 2c: `-nodefaultlibs` means "don't link against any libraries I
> didn't explicitly tell you to". `-fsanitize=*` as a driver argument
> *when linking* is an explicit request to link against the sanitizer
> runtimes. So that should win. If you don't want to link against the
> sanitizer runtimes, link without the `-fsanitize=` flag, or append
> `-fno-sanitize=all` to the link line.

+1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski updated this revision to Diff 70646.
agutowski marked an inline comment as done.
agutowski added a comment.

Add evaluating values for constant arguments


https://reviews.llvm.org/D24311

Files:
  include/clang/Basic/Builtins.def
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics-rotations.c
  test/Sema/constant-builtins-2.c

Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -447,61 +447,6 @@
   return (unsigned __int64)__in1 * (unsigned __int64)__in2;
 }
 /**\
-|* Bit Twiddling
-\**/
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotl8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value << _Shift) | (_Value >> (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotr8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value >> _Shift) | (_Value << (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotl16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value << _Shift) | (_Value >> (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotr16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value >> _Shift) | (_Value << (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotl(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotr(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotl(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotr(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotl64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (64 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotr64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (64 - _Shift)) : _Value;
-}
-/**\
 |* Bit Counting and Testing
 \**/
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -696,6 +696,58 @@
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *LeftShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *RightShifted = Builder.CreateLShr(Val, Shift);
+Value *LeftShifted = Builder.CreateShl(Val, LeftShift);
+Value *Rotated = Builder.CreateOr(LeftShifted, RightShifted);
+
+Value *ShiftIsZero = Builder.CreateICmpEQ(Shift, ArgZero);
+Value *Result = Builder.CreateSelect(ShiftIsZero, Val, Rotated);
+return RValue::get(Result);
+  }
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = 

[PATCH] D24330: Add some MS aliases for existing intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski created this revision.
agutowski added reviewers: rnk, thakis, compnerd, majnemer.
agutowski added a subscriber: cfe-commits.

https://reviews.llvm.org/D24330

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/BuiltinsX86.def
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/emmintrin.h
  lib/Headers/ia32intrin.h
  lib/Headers/intrin.h
  lib/Headers/xmmintrin.h
  test/CodeGen/builtins-x86.c
  test/Sema/constant-builtins-2.c
  test/Sema/constant-builtins.c

Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -60,12 +60,6 @@
   return __builtin_ia32_rdpmc(__A);
 }
 
-/* __rdtsc */
-static __inline__ unsigned long long __attribute__((__always_inline__, __nodebug__))
-__rdtsc(void) {
-  return __builtin_ia32_rdtsc();
-}
-
 /* __rdtscp */
 static __inline__ unsigned long long __attribute__((__always_inline__, __nodebug__))
 __rdtscp(unsigned int *__A) {
Index: lib/Headers/emmintrin.h
===
--- lib/Headers/emmintrin.h
+++ lib/Headers/emmintrin.h
@@ -2447,52 +2447,6 @@
 }
 #endif
 
-/// \brief The cache line containing __p is flushed and invalidated from all
-///caches in the coherency domain.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the \c CLFLUSH instruction.
-///
-/// \param __p
-///A pointer to the memory location used to identify the cache line to be
-///flushed.
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_clflush(void const *__p)
-{
-  __builtin_ia32_clflush(__p);
-}
-
-/// \brief Forces strong memory ordering (serialization) between load
-///instructions preceding this instruction and load instructions following
-///this instruction, ensuring the system completes all previous loads before
-///executing subsequent loads.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the \c LFENCE instruction.
-///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_lfence(void)
-{
-  __builtin_ia32_lfence();
-}
-
-/// \brief Forces strong memory ordering (serialization) between load and store
-///instructions preceding this instruction and load and store instructions
-///following this instruction, ensuring that the system completes all
-///previous memory accesses before executing subsequent memory accesses.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the \c MFENCE instruction.
-///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_mfence(void)
-{
-  __builtin_ia32_mfence();
-}
-
 /// \brief Converts 16-bit signed integers from both 128-bit integer vector
 ///operands into 8-bit signed integers, and packs the results into the
 ///destination. Positive values greater than 0x7F are saturated to 0x7F.
@@ -3206,19 +3160,6 @@
   return (__m128d)__a;
 }
 
-/// \brief Indicates that a spin loop is being executed for the purposes of
-///optimizing power consumption during the loop.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the \c PAUSE instruction.
-///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_pause(void)
-{
-  __builtin_ia32_pause();
-}
-
 #undef __DEFAULT_FN_ATTRS
 
 #define _MM_SHUFFLE2(x, y) (((x) << 1) | (y))
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -518,14 +518,6 @@
   *_Index = 31 - __builtin_clzl(_Mask);
   return 1;
 }
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-__popcnt16(unsigned short _Value) {
-  return __builtin_popcount((int)_Value);
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-__popcnt(unsigned int _Value) {
-  return __builtin_popcount(_Value);
-}
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _bittest(long const *_BitBase, long _BitPos) {
   return (*_BitBase >> _BitPos) & 1;
@@ -568,11 +560,6 @@
   *_Index = 63 - __builtin_clzll(_Mask);
   return 1;
 }
-static __inline__
-unsigned __int64 __DEFAULT_FN_ATTRS
-__popcnt64(unsigned __int64 _Value) {
-  return __builtin_popcountll(_Value);
-}
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _bittest64(__int64 const *_BitBase, __int64 _BitPos) {
   return (*_BitBase >> _BitPos) & 1;
Index: lib/Headers/xmmintrin.h
===
--- lib/Headers/xmmintrin.h
+++ lib/Headers/xmmintrin.h
@@ -2085,21 +2085,6 @@
   __builtin_nontemporal_store((__v4sf)__a, (__v4sf*)__p);
 }
 
-/// \brief Forces strong memory ordering (serialization) between store
-///instructions preceding this instruction and store instructions following
-///this instruction, ensuring the system completes all previous stores
-///before executing subsequent stores.
-///
-/// \headerfile 
-///
-/// This intrinsic corresponds to the \c SFENCE instruction.
-///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_sfence(void)
-{
-  __builtin_ia32_sfence();
-}
-
 /// \brief 

Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Richard Smith via cfe-commits
On 7 Sep 2016 6:59 pm, "Peter Collingbourne"  wrote:

On Wed, Sep 7, 2016 at 6:44 PM, Richard Smith  wrote:

> On 7 Sep 2016 6:23 pm, "Peter Collingbourne"  wrote:
>
> pcc marked 4 inline comments as done.
>
> 
> Comment at: lib/CodeGen/CGVTables.cpp:588
> @@ +587,3 @@
> +if (auto *F = dyn_cast(Cache))
> +  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
> +Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);
> 
> rsmith wrote:
> > Do you have any idea why we're doing this? It looks wrong to me. These
> ABI entry points are exposed and could certainly have their addresses taken
> and used in this translation unit.
> I introduced this in D18071. Although a translation unit can take the
> address of one of these functions, that would involve declaring a function
> with a reserved name, so I believe we'd be allowed to impose restrictions
> such as `unnamed_addr` on the address.
>
>
> These are declared by , and thus presumably intended to be
> useable by programs, so I'm not convinced that reasoning applies.
>

cxxabi.h isn't part of the standard, is it? So whatever it does shouldn't
have an effect on the standard.


It's part of the itanium c++ abi standard.

Do we gain anything from this?
>

Not at present (the relative ABI is the only user I know of and that isn't
fully implemented yet on trunk), although something like this may be
required if we ever fully implement the relative ABI.

Thanks,
-- 
-- 
Peter
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread David Majnemer via cfe-commits
majnemer added a subscriber: majnemer.
majnemer added a comment.

In https://reviews.llvm.org/D24311#536545, @agutowski wrote:

> In https://reviews.llvm.org/D24311#536333, @rnk wrote:
>
> > You should locally verify that this generates the correct assembly when 
> > optimizations are enabled, and if it doesn't, try to make the input look 
> > more like llvm/test/CodeGen/X86/rotate.ll
>
>
> Yeah, I checked that it's optimized to ROL/ROR instruction - now, after 
> looking closer, I can see that it's optimized for all functions except for 
> the ones operating on 16-bit integers. Could that be a calculated decision, 
> or should I try to make it generate ROL/ROR whenever it can?
>
> Edit: I tried to make the code as similar to the one from rotate.ll as 
> possible, and it's still not optimized to ROL/ROR for 16-bit integers, so I 
> guess it should stay that way.


I wouldn't worry about this too much...  It is up to the backend to do the 
right thing.


https://reviews.llvm.org/D24311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Peter Collingbourne via cfe-commits
On Wed, Sep 7, 2016 at 6:44 PM, Richard Smith  wrote:

> On 7 Sep 2016 6:23 pm, "Peter Collingbourne"  wrote:
>
> pcc marked 4 inline comments as done.
>
> 
> Comment at: lib/CodeGen/CGVTables.cpp:588
> @@ +587,3 @@
> +if (auto *F = dyn_cast(Cache))
> +  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
> +Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);
> 
> rsmith wrote:
> > Do you have any idea why we're doing this? It looks wrong to me. These
> ABI entry points are exposed and could certainly have their addresses taken
> and used in this translation unit.
> I introduced this in D18071. Although a translation unit can take the
> address of one of these functions, that would involve declaring a function
> with a reserved name, so I believe we'd be allowed to impose restrictions
> such as `unnamed_addr` on the address.
>
>
> These are declared by , and thus presumably intended to be
> useable by programs, so I'm not convinced that reasoning applies.
>

cxxabi.h isn't part of the standard, is it? So whatever it does shouldn't
have an effect on the standard.


> Do we gain anything from this?
>

Not at present (the relative ABI is the only user I know of and that isn't
fully implemented yet on trunk), although something like this may be
required if we ever fully implement the relative ABI.

Thanks,
-- 
-- 
Peter
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280899 - Add explicit casts to size_t to try to appease MSVC.

2016-09-07 Thread Peter Collingbourne via cfe-commits
Author: pcc
Date: Wed Sep  7 20:45:28 2016
New Revision: 280899

URL: http://llvm.org/viewvc/llvm-project?rev=280899=rev
Log:
Add explicit casts to size_t to try to appease MSVC.

Modified:
cfe/trunk/include/clang/AST/VTableBuilder.h

Modified: cfe/trunk/include/clang/AST/VTableBuilder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/VTableBuilder.h?rev=280899=280898=280899=diff
==
--- cfe/trunk/include/clang/AST/VTableBuilder.h (original)
+++ cfe/trunk/include/clang/AST/VTableBuilder.h Wed Sep  7 20:45:28 2016
@@ -243,11 +243,11 @@ public:
   ~VTableLayout();
 
   ArrayRef vtable_components() const {
-return {VTableComponents.get(), NumVTableComponents};
+return {VTableComponents.get(), size_t(NumVTableComponents)};
   }
 
   ArrayRef vtable_thunks() const {
-return {VTableThunks.get(), NumVTableThunks};
+return {VTableThunks.get(), size_t(NumVTableThunks)};
   }
 
   uint64_t getAddressPoint(BaseSubobject Base) const {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Richard Smith via cfe-commits
On 7 Sep 2016 6:23 pm, "Peter Collingbourne"  wrote:

pcc marked 4 inline comments as done.


Comment at: lib/CodeGen/CGVTables.cpp:588
@@ +587,3 @@
+if (auto *F = dyn_cast(Cache))
+  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);

rsmith wrote:
> Do you have any idea why we're doing this? It looks wrong to me. These
ABI entry points are exposed and could certainly have their addresses taken
and used in this translation unit.
I introduced this in D18071. Although a translation unit can take the
address of one of these functions, that would involve declaring a function
with a reserved name, so I believe we'd be allowed to impose restrictions
such as `unnamed_addr` on the address.


These are declared by , and thus presumably intended to be
useable by programs, so I'm not convinced that reasoning applies. Do we
gain anything from this?


Repository:
  rL LLVM

https://reviews.llvm.org/D22642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Peter Collingbourne via cfe-commits
pcc marked 4 inline comments as done.


Comment at: lib/CodeGen/CGVTables.cpp:588
@@ +587,3 @@
+if (auto *F = dyn_cast(Cache))
+  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);

rsmith wrote:
> Do you have any idea why we're doing this? It looks wrong to me. These ABI 
> entry points are exposed and could certainly have their addresses taken and 
> used in this translation unit.
I introduced this in D18071. Although a translation unit can take the address 
of one of these functions, that would involve declaring a function with a 
reserved name, so I believe we'd be allowed to impose restrictions such as 
`unnamed_addr` on the address.


Repository:
  rL LLVM

https://reviews.llvm.org/D22642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Peter Collingbourne via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280897: CodeGen: Clean up implementation of vtable 
initializer builder. NFC. (authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D22642?vs=70622=70632#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22642

Files:
  cfe/trunk/include/clang/AST/VTableBuilder.h
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/CodeGen/CGVTables.cpp
  cfe/trunk/lib/CodeGen/CGVTables.h
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

Index: cfe/trunk/include/clang/AST/VTableBuilder.h
===
--- cfe/trunk/include/clang/AST/VTableBuilder.h
+++ cfe/trunk/include/clang/AST/VTableBuilder.h
@@ -218,12 +218,6 @@
 class VTableLayout {
 public:
   typedef std::pair VTableThunkTy;
-
-  typedef const VTableComponent *vtable_component_iterator;
-  typedef const VTableThunkTy *vtable_thunk_iterator;
-  typedef llvm::iterator_range
-  vtable_component_range;
-
   typedef llvm::DenseMap AddressPointsMapTy;
 
 private:
@@ -248,31 +242,12 @@
bool IsMicrosoftABI);
   ~VTableLayout();
 
-  uint64_t getNumVTableComponents() const {
-return NumVTableComponents;
-  }
-
-  vtable_component_range vtable_components() const {
-return vtable_component_range(vtable_component_begin(),
-  vtable_component_end());
-  }
-
-  vtable_component_iterator vtable_component_begin() const {
-return VTableComponents.get();
-  }
-
-  vtable_component_iterator vtable_component_end() const {
-return VTableComponents.get() + NumVTableComponents;
-  }
-
-  uint64_t getNumVTableThunks() const { return NumVTableThunks; }
-
-  vtable_thunk_iterator vtable_thunk_begin() const {
-return VTableThunks.get();
+  ArrayRef vtable_components() const {
+return {VTableComponents.get(), NumVTableComponents};
   }
 
-  vtable_thunk_iterator vtable_thunk_end() const {
-return VTableThunks.get() + NumVTableThunks;
+  ArrayRef vtable_thunks() const {
+return {VTableThunks.get(), NumVTableThunks};
   }
 
   uint64_t getAddressPoint(BaseSubobject Base) const {
Index: cfe/trunk/lib/CodeGen/CGVTables.h
===
--- cfe/trunk/lib/CodeGen/CGVTables.h
+++ cfe/trunk/lib/CodeGen/CGVTables.h
@@ -49,22 +49,29 @@
   /// indices.
   SecondaryVirtualPointerIndicesMapTy SecondaryVirtualPointerIndices;
 
+  /// Cache for the pure virtual member call function.
+  llvm::Constant *PureVirtualFn = nullptr;
+
+  /// Cache for the deleted virtual member call function.
+  llvm::Constant *DeletedVirtualFn = nullptr;
+
   /// emitThunk - Emit a single thunk.
   void emitThunk(GlobalDecl GD, const ThunkInfo , bool ForVTable);
 
   /// maybeEmitThunkForVTable - Emit the given thunk for the vtable if needed by
   /// the ABI.
   void maybeEmitThunkForVTable(GlobalDecl GD, const ThunkInfo );
 
+  llvm::Constant *CreateVTableComponent(unsigned Idx,
+const VTableLayout ,
+llvm::Constant *RTTI,
+unsigned );
+
 public:
-  /// CreateVTableInitializer - Create a vtable initializer for the given record
-  /// decl.
-  /// \param Components - The vtable components; this is really an array of
-  /// VTableComponents.
-  llvm::Constant *CreateVTableInitializer(
-  const CXXRecordDecl *RD, const VTableComponent *Components,
-  unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-  unsigned NumVTableThunks, llvm::Constant *RTTI);
+  /// CreateVTableInitializer - Create a vtable initializer with the given
+  /// layout.
+  llvm::Constant *CreateVTableInitializer(const VTableLayout ,
+  llvm::Constant *RTTI);
 
   CodeGenVTables(CodeGenModule );
 
Index: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
===
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1462,9 +1462,7 @@
   CGM.GetAddrOfRTTIDescriptor(CGM.getContext().getTagDeclType(RD));
 
   // Create and set the initializer.
-  llvm::Constant *Init = CGVT.CreateVTableInitializer(
-  RD, VTLayout.vtable_component_begin(), VTLayout.getNumVTableComponents(),
-  VTLayout.vtable_thunk_begin(), VTLayout.getNumVTableThunks(), RTTI);
+  llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
   VTable->setInitializer(Init);
 
   // Set the correct linkage.
@@ -1575,7 +1573,7 @@
 
   ItaniumVTableContext  = CGM.getItaniumVTableContext();
   llvm::ArrayType *ArrayType = llvm::ArrayType::get(
-  CGM.Int8PtrTy, VTContext.getVTableLayout(RD).getNumVTableComponents());
+  CGM.Int8PtrTy, VTContext.getVTableLayout(RD).vtable_components().size());
 
   VTable = 

r280897 - CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Peter Collingbourne via cfe-commits
Author: pcc
Date: Wed Sep  7 20:14:39 2016
New Revision: 280897

URL: http://llvm.org/viewvc/llvm-project?rev=280897=rev
Log:
CodeGen: Clean up implementation of vtable initializer builder. NFC.

- Simplify signature of CreateVTableInitializer function.
- Move vtable component builder to a separate function.
- Remove unnecessary accessors from VTableLayout class.

This is in preparation for a future change that will alter the type of the
vtable initializer.

Differential Revision: https://reviews.llvm.org/D22642

Modified:
cfe/trunk/include/clang/AST/VTableBuilder.h
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CGVTables.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

Modified: cfe/trunk/include/clang/AST/VTableBuilder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/VTableBuilder.h?rev=280897=280896=280897=diff
==
--- cfe/trunk/include/clang/AST/VTableBuilder.h (original)
+++ cfe/trunk/include/clang/AST/VTableBuilder.h Wed Sep  7 20:14:39 2016
@@ -218,12 +218,6 @@ private:
 class VTableLayout {
 public:
   typedef std::pair VTableThunkTy;
-
-  typedef const VTableComponent *vtable_component_iterator;
-  typedef const VTableThunkTy *vtable_thunk_iterator;
-  typedef llvm::iterator_range
-  vtable_component_range;
-
   typedef llvm::DenseMap AddressPointsMapTy;
 
 private:
@@ -248,31 +242,12 @@ public:
bool IsMicrosoftABI);
   ~VTableLayout();
 
-  uint64_t getNumVTableComponents() const {
-return NumVTableComponents;
-  }
-
-  vtable_component_range vtable_components() const {
-return vtable_component_range(vtable_component_begin(),
-  vtable_component_end());
-  }
-
-  vtable_component_iterator vtable_component_begin() const {
-return VTableComponents.get();
-  }
-
-  vtable_component_iterator vtable_component_end() const {
-return VTableComponents.get() + NumVTableComponents;
-  }
-
-  uint64_t getNumVTableThunks() const { return NumVTableThunks; }
-
-  vtable_thunk_iterator vtable_thunk_begin() const {
-return VTableThunks.get();
+  ArrayRef vtable_components() const {
+return {VTableComponents.get(), NumVTableComponents};
   }
 
-  vtable_thunk_iterator vtable_thunk_end() const {
-return VTableThunks.get() + NumVTableThunks;
+  ArrayRef vtable_thunks() const {
+return {VTableThunks.get(), NumVTableThunks};
   }
 
   uint64_t getAddressPoint(BaseSubobject Base) const {

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=280897=280896=280897=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Sep  7 20:14:39 2016
@@ -1576,7 +1576,7 @@ void CGDebugInfo::CollectVTableInfo(cons
 const VTableLayout  =
 CGM.getMicrosoftVTableContext().getVFTableLayout(RD, 
CharUnits::Zero());
 unsigned VSlotCount =
-VFTLayout.getNumVTableComponents() - CGM.getLangOpts().RTTIData;
+VFTLayout.vtable_components().size() - CGM.getLangOpts().RTTIData;
 unsigned VTableWidth = PtrWidth * VSlotCount;
 
 // Create a very wide void* type and insert it directly in the element 
list.

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=280897=280896=280897=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Sep  7 20:14:39 2016
@@ -517,139 +517,121 @@ void CodeGenVTables::EmitThunks(GlobalDe
 emitThunk(GD, Thunk, /*ForVTable=*/false);
 }
 
-llvm::Constant *CodeGenVTables::CreateVTableInitializer(
-const CXXRecordDecl *RD, const VTableComponent *Components,
-unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-unsigned NumVTableThunks, llvm::Constant *RTTI) {
-  SmallVector Inits;
-
-  llvm::Type *Int8PtrTy = CGM.Int8PtrTy;
-  
-  llvm::Type *PtrDiffTy = 
-CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
-
-  unsigned NextVTableThunkIndex = 0;
-
-  llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
-
-  for (unsigned I = 0; I != NumComponents; ++I) {
-VTableComponent Component = Components[I];
-
-llvm::Constant *Init = nullptr;
+llvm::Constant *CodeGenVTables::CreateVTableComponent(
+unsigned Idx, const VTableLayout , llvm::Constant *RTTI,
+unsigned ) {
+  VTableComponent Component = VTLayout.vtable_components()[Idx];
+
+  auto OffsetConstant = [&](CharUnits Offset) {
+return llvm::ConstantExpr::getIntToPtr(
+llvm::ConstantInt::get(CGM.PtrDiffTy, 

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

2016-09-07 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: lib/Sema/SemaChecking.cpp:4089-4090
@@ +4088,4 @@
+if (BinOp->isAdditiveOp()) {
+  bool LIsInt = BinOp->getLHS()->EvaluateAsInt(LResult, S.Context);
+  bool RIsInt = BinOp->getRHS()->EvaluateAsInt(RResult, S.Context);
+

meikeb wrote:
> rsmith wrote:
> > What happens if one of these expressions is value-dependent? The evaluator 
> > can crash or assert if given a value-dependent expression. If we don't 
> > defer these checks in dependent contexts, you'll need to handle that 
> > possibility somehow.
> > 
> > Example:
> > 
> >   template void f(const char *p) {
> > printf("blah blah %s" + N, p);
> >   }
> I think I don't understand what you are trying to tell me. Especially the 
> example you provided does just fine and behaves as I expected. As far as I 
> followed EvaluateAsInt it does not assert but returns false if we don't get a 
> constexpr here. We warn under -Wformat-nonliteral for value-dependent string 
> literals.
> 
> Could you explain this more or provide an example that triggers an assert or 
> explain what behavior is wrong regarding the provided example? Thanks! 
We should not warn for that example, since (for instance) calling `f<0>` is 
fine (we should warn for `f<11>`, though, since it has no format specifiers).

While `EvaluateAsInt` happens to not assert for that particular value-dependent 
input, it does assert for some other value-dependent cases. It's not easy for 
me to find you such a case, because Clang is currently careful to never call 
this function on a value-dependent expression, but perhaps this will trigger an 
assert:

  struct S { constexpr S(int n) : n(n) {} int n; };
  template void f(const char *p) {
printf("blah blah %s" + S(N).n, p);
  }


https://reviews.llvm.org/D23820



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

2016-09-07 Thread Dean Michael Berris via cfe-commits
dberris closed this revision.
dberris added a comment.

This has been landed as https://reviews.llvm.org/rL280889.


https://reviews.llvm.org/D23932



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280889 - [XRay] ARM 32-bit no-Thumb support in Clang

2016-09-07 Thread Dean Michael Berris via cfe-commits
Author: dberris
Date: Wed Sep  7 19:23:28 2016
New Revision: 280889

URL: http://llvm.org/viewvc/llvm-project?rev=280889=rev
Log:
[XRay] ARM 32-bit no-Thumb support in Clang

Just a test for now, adapted from x86_64 tests of XRay.
This is one of 3 commits to different repositories of XRay ARM port. The
other 2 are:

1. https://reviews.llvm.org/D23931 (LLVM)
2. https://reviews.llvm.org/D23933 (compiler-rt)

Differential Review: https://reviews.llvm.org/D23932

Added:
cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp

Added: cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp?rev=280889=auto
==
--- cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp (added)
+++ cfe/trunk/test/CodeGen/xray-attributes-supported-arm.cpp Wed Sep  7 
19:23:28 2016
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu | FileCheck %s
+
+// Make sure that the LLVM attribute for XRay-annotated functions do show up.
+[[clang::xray_always_instrument]] void foo() {
+// CHECK: define void @_Z3foov() #0
+};
+
+[[clang::xray_never_instrument]] void bar() {
+// CHECK: define void @_Z3barv() #1
+};
+
+// CHECK: #0 = {{.*}}"function-instrument"="xray-always"
+// CHECK: #1 = {{.*}}"function-instrument"="xray-never"


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

2016-09-07 Thread Dean Michael Berris via cfe-commits
dberris added a comment.

Landing this one now.


https://reviews.llvm.org/D23932



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22057: Prevent devirtualization of calls to un-instantiated functions.

2016-09-07 Thread Sunil Srivastava via cfe-commits
Sunil_Srivastava added inline comments.


Comment at: test/CodeGen/no-devirt.cpp:16
@@ +15,3 @@
+   if (a > 6) return data[a] ;  // Should not devirtualize
+   if (a > 4) return data.func1(a); // Should devirtualize
+   return data.func2(a);// Should devirtualize

Quuxplusone wrote:
> This is a really dumb question from the peanut gallery, but, could you 
> explain why these two cases (lines 15 and 16) should differ?  It really seems 
> like both calls should be able to be devirtualized, because the compiler 
> statically knows what they should call.
> 
> I think you mention the difference between lines 15 and 16 here:
> 
> > except, for some reason, when it is an operator and used with an operator 
> > syntax
> 
> but you don't explain *why* the difference is desirable. Shouldn't we just 
> fix that difference, then?
> 
> Is your first fix (
> 
> > The first fix will be in the front end to force the instantiation on 
> > virtual calls that are potentially devirtualizable.
> 
> ) basically "fix the difference between lines 15 and 16", or is it talking 
> about something else entirely?
AFAICS, The two cases (line 15 and 16) should not differ.

The first fix will "fix the difference between line 15 and 16". I have the 
change for that ready, but once we do that, there will be no way (known to me) 
of testing the second "fix". Hence the second "fix" is being proposed for 
commit before the first.


https://reviews.llvm.org/D22057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

2016-09-07 Thread Eric Fiselier via cfe-commits
+1

On Wed, Sep 7, 2016 at 6:04 PM, Richard Smith  wrote:

> rsmith added a subscriber: rsmith.
> rsmith added a comment.
>
> My 2c: `-nodefaultlibs` means "don't link against any libraries I didn't
> explicitly tell you to". `-fsanitize=*` as a driver argument *when linking*
> is an explicit request to link against the sanitizer runtimes. So that
> should win. If you don't want to link against the sanitizer runtimes, link
> without the `-fsanitize=` flag, or append `-fno-sanitize=all` to the link
> line.
>
>
> https://reviews.llvm.org/D24048
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

2016-09-07 Thread Richard Smith via cfe-commits
rsmith added a subscriber: rsmith.
rsmith added a comment.

My 2c: `-nodefaultlibs` means "don't link against any libraries I didn't 
explicitly tell you to". `-fsanitize=*` as a driver argument *when linking* is 
an explicit request to link against the sanitizer runtimes. So that should win. 
If you don't want to link against the sanitizer runtimes, link without the 
`-fsanitize=` flag, or append `-fno-sanitize=all` to the link line.


https://reviews.llvm.org/D24048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Richard Smith via cfe-commits
rsmith accepted this revision.
This revision is now accepted and ready to land.


Comment at: include/clang/AST/VTableBuilder.h:222-225
@@ -221,6 +221,6 @@
 
   typedef const VTableComponent *vtable_component_iterator;
   typedef const VTableThunkTy *vtable_thunk_iterator;
   typedef llvm::iterator_range
   vtable_component_range;
 

Can you remove these? It looks like they should be unused now.


Comment at: lib/CodeGen/CGVTables.cpp:527-528
@@ +526,4 @@
+  auto OffsetConstant = [&](CharUnits Offset) {
+llvm::Type *PtrDiffTy =
+CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
+return llvm::ConstantExpr::getIntToPtr(

This is just `CGM.PtrDiffTy`.


Comment at: lib/CodeGen/CGVTables.cpp:588
@@ +587,3 @@
+if (auto *F = dyn_cast(Cache))
+  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);

Do you have any idea why we're doing this? It looks wrong to me. These ABI 
entry points are exposed and could certainly have their addresses taken and 
used in this translation unit.


Comment at: lib/CodeGen/CGVTables.cpp:598-615
@@ -638,5 +597,20 @@
+  CGM.getCXXABI().GetPureVirtualCallName());
+} else if (cast(GD.getDecl())->isDeleted()) {
+  return SpecialVirtualFn(DeletedVirtualFn,
+  CGM.getCXXABI().GetDeletedVirtualCallName());
+} else {
+  // Check if we should use a thunk.
+  if (NextVTableThunkIndex < VTLayout.vtable_thunks().size() &&
+  VTLayout.vtable_thunks()[NextVTableThunkIndex].first == Idx) {
+const ThunkInfo  =
+VTLayout.vtable_thunks()[NextVTableThunkIndex].second;
+
+maybeEmitThunkForVTable(GD, Thunk);
+NextVTableThunkIndex++;
+return CGM.GetAddrOfThunk(GD, Thunk);
+  } else {
+llvm::Type *Ty = CGM.getTypes().GetFunctionTypeForVTable(GD);
 
-Init = llvm::ConstantExpr::getBitCast(Init, Int8PtrTy);
+return CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
   }
 }

Please remove the `else`-after-`return` throughout here. That'll make it much 
more obvious to a reader of the code that fallthrough to the next `case` is not 
possible.


Comment at: lib/CodeGen/CGVTables.h:52
@@ -51,1 +51,3 @@
 
+  llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
+

I'd prefer these to be two separate declarations, with doc comments.


https://reviews.llvm.org/D22642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24048: [Driver] [Darwin] Add sanitizer libraries even if -nodefaultlibs is passed

2016-09-07 Thread Chris Bieneman via cfe-commits
beanz updated this revision to Diff 70624.
beanz added a comment.

- Added new driver flag -flink-sanitizer-runtimes which forces linking 
sanitizer runtimes.

Additional cleanup will be required to support the GNUTools and FreeBSD drivers 
as well as making the Darwin behavior more consistent. I will update the patch 
with additional changes if this approach looks acceptable.


https://reviews.llvm.org/D24048

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  test/Driver/nodefaultlib.c
  test/Driver/nostdlib.c

Index: test/Driver/nostdlib.c
===
--- test/Driver/nostdlib.c
+++ test/Driver/nostdlib.c
@@ -29,3 +29,9 @@
 // CHECK-LINUX-NOSTDLIB: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
 // CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i686.a"
 // CHECK-MSVC-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
+
+// RUN: %clang -target i686-apple-darwin -nostdlib -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB1 %s
+// DARWIN_SAN_LIB1-NOT: libclang_rt.asan_osx_dynamic.dylib
+
+// RUN: %clang -target i686-apple-darwin -nostdlib -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB2 %s
+// DARWIN_SAN_LIB2: libclang_rt.asan_osx_dynamic.dylib
Index: test/Driver/nodefaultlib.c
===
--- test/Driver/nodefaultlib.c
+++ test/Driver/nodefaultlib.c
@@ -8,3 +8,9 @@
 // RUN: %clang -target i686-pc-linux-gnu -stdlib=libc++ -nodefaultlibs -lstdc++ -### %s 2>&1 | FileCheck -check-prefix=TEST2 %s
 // TEST2-NOT: "-lc++"
 // TEST2: "-lstdc++"
+
+// RUN: %clang -target i686-apple-darwin -nodefaultlibs -fsanitize=address -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB1 %s
+// DARWIN_SAN_LIB1-NOT: libclang_rt.asan_osx_dynamic.dylib
+
+// RUN: %clang -target i686-apple-darwin -nodefaultlibs -fsanitize=address -flink-sanitizer-runtimes -### %s 2>&1 | FileCheck -check-prefix=DARWIN_SAN_LIB2 %s
+// DARWIN_SAN_LIB2: libclang_rt.asan_osx_dynamic.dylib
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8061,6 +8061,8 @@
 
 // Let the tool chain choose which runtime library to link.
 getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs);
+  } else if (Args.hasArg(options::OPT_flink_sanitizer_runtimes)) {
+getMachOToolChain().AddLinkSanitizerLibArgs(Args, CmdArgs);
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
Index: lib/Driver/ToolChains.h
===
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -282,6 +282,10 @@
   virtual void AddLinkRuntimeLibArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const;
 
+  virtual void
+  AddLinkSanitizerLibArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const {}
+
   virtual void addStartObjectFileArgs(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const {
   }
@@ -571,6 +575,10 @@
 
   RuntimeLibType GetRuntimeLibType(const llvm::opt::ArgList ) const override;
 
+  void
+  AddLinkSanitizerLibArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const override;
+
   void AddLinkRuntimeLibArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
 
Index: lib/Driver/ToolChains.cpp
===
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -425,22 +425,7 @@
 return;
   }
 
-  const SanitizerArgs  = getSanitizerArgs();
-  if (Sanitize.needsAsanRt())
-AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
-  if (Sanitize.needsUbsanRt())
-AddLinkSanitizerLibArgs(Args, CmdArgs, "ubsan");
-  if (Sanitize.needsTsanRt())
-AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
-  if (Sanitize.needsStatsRt()) {
-StringRef OS = isTargetMacOS() ? "osx" : "iossim";
-AddLinkRuntimeLib(Args, CmdArgs,
-  (Twine("libclang_rt.stats_client_") + OS + ".a").str(),
-  /*AlwaysLink=*/true);
-AddLinkSanitizerLibArgs(Args, CmdArgs, "stats");
-  }
-  if (Sanitize.needsEsanRt())
-AddLinkSanitizerLibArgs(Args, CmdArgs, "esan");
+  AddLinkSanitizerLibArgs(Args, CmdArgs);
 
   // Otherwise link libSystem, then the dynamic runtime library, and finally any
   // target specific static runtime library.
@@ -495,6 +480,26 @@
   }
 }
 
+void DarwinClang::AddLinkSanitizerLibArgs(
+const llvm::opt::ArgList , llvm::opt::ArgStringList ) const {
+  const SanitizerArgs  = getSanitizerArgs();
+  if (Sanitize.needsAsanRt())
+

Re: [PATCH] D22642: CodeGen: Clean up implementation of vtable initializer builder. NFC.

2016-09-07 Thread Peter Collingbourne via cfe-commits
pcc updated this revision to Diff 70622.
pcc added a comment.

- Refresh


https://reviews.llvm.org/D22642

Files:
  include/clang/AST/VTableBuilder.h
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CGVTables.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp

Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1575,10 +1575,7 @@
[](const VTableComponent ) { return VTC.isRTTIKind(); }))
   RTTI = getMSCompleteObjectLocator(RD, Info);
 
-llvm::Constant *Init = CGVT.CreateVTableInitializer(
-RD, VTLayout.vtable_component_begin(),
-VTLayout.getNumVTableComponents(), VTLayout.vtable_thunk_begin(),
-VTLayout.getNumVTableThunks(), RTTI);
+llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
 
 VTable->setInitializer(Init);
 
@@ -1701,7 +1698,8 @@
 
   uint64_t NumVTableSlots =
   VTContext.getVFTableLayout(RD, VFPtr->FullOffsetInMDC)
-  .getNumVTableComponents();
+  .vtable_components()
+  .size();
   llvm::GlobalValue::LinkageTypes VTableLinkage =
   VTableAliasIsRequred ? llvm::GlobalValue::PrivateLinkage : VFTableLinkage;
 
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1462,9 +1462,7 @@
   CGM.GetAddrOfRTTIDescriptor(CGM.getContext().getTagDeclType(RD));
 
   // Create and set the initializer.
-  llvm::Constant *Init = CGVT.CreateVTableInitializer(
-  RD, VTLayout.vtable_component_begin(), VTLayout.getNumVTableComponents(),
-  VTLayout.vtable_thunk_begin(), VTLayout.getNumVTableThunks(), RTTI);
+  llvm::Constant *Init = CGVT.CreateVTableInitializer(VTLayout, RTTI);
   VTable->setInitializer(Init);
 
   // Set the correct linkage.
@@ -1575,7 +1573,7 @@
 
   ItaniumVTableContext  = CGM.getItaniumVTableContext();
   llvm::ArrayType *ArrayType = llvm::ArrayType::get(
-  CGM.Int8PtrTy, VTContext.getVTableLayout(RD).getNumVTableComponents());
+  CGM.Int8PtrTy, VTContext.getVTableLayout(RD).vtable_components().size());
 
   VTable = CGM.CreateOrReplaceCXXRuntimeVariable(
   Name, ArrayType, llvm::GlobalValue::ExternalLinkage);
Index: lib/CodeGen/CGVTables.h
===
--- lib/CodeGen/CGVTables.h
+++ lib/CodeGen/CGVTables.h
@@ -49,22 +49,27 @@
   /// indices.
   SecondaryVirtualPointerIndicesMapTy SecondaryVirtualPointerIndices;
 
+  llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
+
   /// emitThunk - Emit a single thunk.
   void emitThunk(GlobalDecl GD, const ThunkInfo , bool ForVTable);
 
   /// maybeEmitThunkForVTable - Emit the given thunk for the vtable if needed by
   /// the ABI.
   void maybeEmitThunkForVTable(GlobalDecl GD, const ThunkInfo );
 
+  llvm::Constant *CreateVTableComponent(unsigned Idx,
+const VTableLayout ,
+llvm::Constant *RTTI,
+unsigned );
+
 public:
   /// CreateVTableInitializer - Create a vtable initializer for the given record
   /// decl.
   /// \param Components - The vtable components; this is really an array of
   /// VTableComponents.
-  llvm::Constant *CreateVTableInitializer(
-  const CXXRecordDecl *RD, const VTableComponent *Components,
-  unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-  unsigned NumVTableThunks, llvm::Constant *RTTI);
+  llvm::Constant *CreateVTableInitializer(const VTableLayout ,
+  llvm::Constant *RTTI);
 
   CodeGenVTables(CodeGenModule );
 
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -517,139 +517,124 @@
 emitThunk(GD, Thunk, /*ForVTable=*/false);
 }
 
-llvm::Constant *CodeGenVTables::CreateVTableInitializer(
-const CXXRecordDecl *RD, const VTableComponent *Components,
-unsigned NumComponents, const VTableLayout::VTableThunkTy *VTableThunks,
-unsigned NumVTableThunks, llvm::Constant *RTTI) {
-  SmallVector Inits;
-
+llvm::Constant *CodeGenVTables::CreateVTableComponent(
+unsigned Idx, const VTableLayout , llvm::Constant *RTTI,
+unsigned ) {
+  VTableComponent Component = VTLayout.vtable_components()[Idx];
   llvm::Type *Int8PtrTy = CGM.Int8PtrTy;
-  
-  llvm::Type *PtrDiffTy = 
-CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
 
-  unsigned NextVTableThunkIndex = 0;
+  auto OffsetConstant = [&](CharUnits Offset) {
+llvm::Type *PtrDiffTy =
+CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
+return llvm::ConstantExpr::getIntToPtr(
+

Re: r280728 - Modules: Fix an assertion in DeclContext::buildLookup.

2016-09-07 Thread Richard Smith via cfe-commits
On Wed, Sep 7, 2016 at 12:45 PM, Manman Ren  wrote:

> On Tue, Sep 6, 2016 at 6:54 PM, Richard Smith 
> wrote:
>
>> On Tue, Sep 6, 2016 at 11:16 AM, Manman Ren via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: mren
>>> Date: Tue Sep  6 13:16:54 2016
>>> New Revision: 280728
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=280728=rev
>>> Log:
>>> Modules: Fix an assertion in DeclContext::buildLookup.
>>>
>>> When calling getMostRecentDecl, we can pull in more definitions from
>>> a module. We call getPrimaryContext afterwards to make sure that
>>> we buildLookup on a primary context.
>>>
>>> rdar://27926200
>>>
>>> Added:
>>> cfe/trunk/test/Modules/Inputs/lookup-assert/
>>> cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h
>>> cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h
>>> cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h
>>> cfe/trunk/test/Modules/Inputs/lookup-assert/module.map
>>> cfe/trunk/test/Modules/lookup-assert.m
>>> Modified:
>>> cfe/trunk/lib/AST/DeclBase.cpp
>>>
>>> Modified: cfe/trunk/lib/AST/DeclBase.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBa
>>> se.cpp?rev=280728=280727=280728=diff
>>> 
>>> ==
>>> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
>>> +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Sep  6 13:16:54 2016
>>> @@ -1411,10 +1411,6 @@ DeclContext::lookup(DeclarationName Name
>>>assert(DeclKind != Decl::LinkageSpec &&
>>>   "Should not perform lookups into linkage specs!");
>>>
>>> -  const DeclContext *PrimaryContext = getPrimaryContext();
>>> -  if (PrimaryContext != this)
>>> -return PrimaryContext->lookup(Name);
>>> -
>>>// If we have an external source, ensure that any later
>>> redeclarations of this
>>>// context have been loaded, since they may add names to the result
>>> of this
>>>// lookup (or add external visible storage).
>>> @@ -1422,6 +1418,12 @@ DeclContext::lookup(DeclarationName Name
>>>if (Source)
>>>  (void)cast(this)->getMostRecentDecl();
>>>
>>> +  // getMostRecentDecl can change the result of getPrimaryContext. Call
>>> +  // getPrimaryContext afterwards.
>>> +  const DeclContext *PrimaryContext = getPrimaryContext();
>>> +  if (PrimaryContext != this)
>>> +return PrimaryContext->lookup(Name);
>>>
>>
>> This seems like a bug in getPrimaryContext() -- if there is a
>> not-yet-loaded definition of the DeclContext, getPrimaryContext() should
>> return that instead of returning a non-defining declaration. Why is
>> ObjCInterfaceDecl::hasDefinition (indirectly called by
>> getPrimaryContext) not loading the definition in this case?
>>
>
> In the testing case, we have a definition of the ObjC interface from
> textually including a header file, but the definition is also in a module.
> getPrimaryContext for ObjCInterface currently does not  pull from the
> external source.
>

As far as I can see, it does. For ObjCInterface, getPrimaryContext calls
ObjCInterface::getDefinition:

  ObjCInterfaceDecl *getDefinition() {
return hasDefinition()? Data.getPointer()->Definition : nullptr;
  }

hasDefinition() pulls from the external source to find a definition, if it
believes the definition is out of date:

  bool hasDefinition() const {
// If the name of this class is out-of-date, bring it up-to-date, which
// might bring in a definition.
// Note: a null value indicates that we don't have a definition and that
// modules are enabled.
if (!Data.getOpaqueValue())
  getMostRecentDecl();

return Data.getPointer();
  }

So, getPrimaryContext() should always pull from the external source when
building with modules, unless we already have a primary context. In either
case, the context returned by getPrimaryContext() should never change -- we
should do sufficient deserialization for it to return the right answer.


> Since getPrimaryContext  does not guarantee to pull from the external
> source, I thought that is why we call getMostRecentDecl in
> DeclContext::lookup.
>

That's present to solve a different problem, where we can merge together
multiple definitions of the same entity, and where later definitions can
provide additional lookup results. This happens in C++ due to lazy
declarations of implicit special member functions, and should never result
in the primary context changing.


> Are you suggesting to pull from external source in getPrimaryContext?
>
> Cheers,
> Manman
>
>
>
>>
>>> +
>>>if (hasExternalVisibleStorage()) {
>>>  assert(Source && "external visible storage but no external
>>> source?");
>>>
>>>
>>> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>>> nputs/lookup-assert/Base.h?rev=280728=auto
>>> 
>>> ==
>>> --- 

Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski marked 2 inline comments as done.
agutowski added a comment.

In https://reviews.llvm.org/D24311#536333, @rnk wrote:

> You should locally verify that this generates the correct assembly when 
> optimizations are enabled, and if it doesn't, try to make the input look more 
> like llvm/test/CodeGen/X86/rotate.ll


Yeah, I checked that it's optimized to ROL/ROR instruction - now, after looking 
closer, I can see that it's optimized for all functions except for the ones 
operating on 16-bit integers. Could that be a calculated decision, or should I 
try to make it generate ROL/ROR whenever it can?



Comment at: lib/CodeGen/CGBuiltin.cpp:740
@@ +739,3 @@
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *RightShift = Builder.CreateSub(ArgTypeSize, Shift);

Ah, I am sure I saw that bug, but apparently I forgot about it, as on my 
machine it returned correct values. Thanks!


https://reviews.llvm.org/D24311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski updated this revision to Diff 70614.
agutowski added a comment.

Fix undefined value problem when rotating by zero bits
Change tests to work without -Oz flag
Fix types of arguments in tests


https://reviews.llvm.org/D24311

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics-rotations.c

Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -447,61 +447,6 @@
   return (unsigned __int64)__in1 * (unsigned __int64)__in2;
 }
 /**\
-|* Bit Twiddling
-\**/
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotl8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value << _Shift) | (_Value >> (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotr8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value >> _Shift) | (_Value << (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotl16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value << _Shift) | (_Value >> (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotr16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value >> _Shift) | (_Value << (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotl(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotr(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotl(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotr(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotl64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (64 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotr64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (64 - _Shift)) : _Value;
-}
-/**\
 |* Bit Counting and Testing
 \**/
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -696,6 +696,58 @@
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *LeftShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *RightShifted = Builder.CreateLShr(Val, Shift);
+Value *LeftShifted = Builder.CreateShl(Val, LeftShift);
+Value *Rotated = Builder.CreateOr(LeftShifted, RightShifted);
+
+Value *ShiftIsZero = Builder.CreateICmpEQ(Shift, ArgZero);
+Value *Result = Builder.CreateSelect(ShiftIsZero, Val, Rotated);
+return RValue::get(Result);
+  }
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+Value *ArgZero = llvm::Constant::getNullValue(ArgType);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value 

r280878 - clang-format: [JavaScript] Change default AllowShortFunctionsOnASingleLine

2016-09-07 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Wed Sep  7 18:01:13 2016
New Revision: 280878

URL: http://llvm.org/viewvc/llvm-project?rev=280878=rev
Log:
clang-format: [JavaScript] Change default AllowShortFunctionsOnASingleLine
for Google style to "empty".

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=280878=280877=280878=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Sep  7 18:01:13 2016
@@ -610,7 +610,7 @@ FormatStyle getGoogleStyle(FormatStyle::
   } else if (Language == FormatStyle::LK_JavaScript) {
 GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
 GoogleStyle.AlignOperands = false;
-GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
 GoogleStyle.CommentPragmas = "@(export|requirecss|return|see|visibility) ";

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=280878=280877=280878=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Wed Sep  7 18:01:13 2016
@@ -161,7 +161,11 @@ TEST_F(FormatTestJS, ES6DestructuringAss
 }
 
 TEST_F(FormatTestJS, ContainerLiterals) {
-  verifyFormat("var x = {y: function(a) { return a; }};");
+  verifyFormat("var x = {\n"
+   "  y: function(a) {\n"
+   "return a;\n"
+   "  }\n"
+   "};");
   verifyFormat("return {\n"
"  link: function() {\n"
"f();  //\n"
@@ -212,7 +216,11 @@ TEST_F(FormatTestJS, ContainerLiterals)
   verifyFormat("x = foo && {a: 123};");
 
   // Arrow functions in object literals.
-  verifyFormat("var x = {y: (a) => { return a; }};");
+  verifyFormat("var x = {\n"
+   "  y: (a) => {\n"
+   "return a;\n"
+   "  }\n"
+   "};");
   verifyFormat("var x = {y: (a) => a};");
 
   // Computed keys.
@@ -326,11 +334,15 @@ TEST_F(FormatTestJS, FormatsNamespaces)
 
 TEST_F(FormatTestJS, FormatsFreestandingFunctions) {
   verifyFormat("function outer1(a, b) {\n"
-   "  function inner1(a, b) { return a; }\n"
+   "  function inner1(a, b) {\n"
+   "return a;\n"
+   "  }\n"
"  inner1(a, b);\n"
"}\n"
"function outer2(a, b) {\n"
-   "  function inner2(a, b) { return a; }\n"
+   "  function inner2(a, b) {\n"
+   "return a;\n"
+   "  }\n"
"  inner2(a, b);\n"
"}");
   verifyFormat("function f() {}");
@@ -350,7 +362,9 @@ TEST_F(FormatTestJS, GeneratorFunctions)
"  yield 1;\n"
"}\n");
   verifyFormat("class X {\n"
-   "  * generatorMethod() { yield x; }\n"
+   "  * generatorMethod() {\n"
+   "yield x;\n"
+   "  }\n"
"}");
 }
 
@@ -366,7 +380,9 @@ TEST_F(FormatTestJS, AsyncFunctions) {
"  return fetch(x);\n"
"}");
   verifyFormat("class X {\n"
-   "  async asyncMethod() { return fetch(1); }\n"
+   "  async asyncMethod() {\n"
+   "return fetch(1);\n"
+   "  }\n"
"}");
   verifyFormat("function initialize() {\n"
"  // Comment.\n"
@@ -423,8 +439,10 @@ TEST_F(FormatTestJS, ColumnLayoutForArra
 }
 
 TEST_F(FormatTestJS, FunctionLiterals) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   verifyFormat("doFoo(function() {});");
-  verifyFormat("doFoo(function() { return 1; });");
+  verifyFormat("doFoo(function() { return 1; });", Style);
   verifyFormat("var func = function() {\n"
"  return 1;\n"
"};");
@@ -438,7 +456,8 @@ TEST_F(FormatTestJS, FunctionLiterals) {
"getAttribute: function(key) { return this[key]; },\n"
"style: {direction: ''}\n"
"  }\n"
-   "};");
+   "};",
+   Style);
   verifyFormat("abc = xyz ? function() {\n"
"  return 1;\n"
"} : function() {\n"
@@ -476,13 +495,6 @@ TEST_F(FormatTestJS, FunctionLiterals) {
"  // code\n"
"});");
 
-  verifyFormat("f({a: function() { return 1; }});",
-   

Re: [PATCH] D24040: codechecker tool core

2016-09-07 Thread Mehdi AMINI via cfe-commits
mehdi_amini added a comment.



In https://reviews.llvm.org/D24040#532719, @o.gyorgy wrote:

> In https://reviews.llvm.org/D24040#530546, @mehdi_amini wrote:
>
> > > This looks like a fairly large tool. Should it get its own "subproject 
> > > level" directory in the SVN instead of being nested within clang?
> >
> >
> > I'd add that the clang-tools-extra are more closely tied to clang than what 
> > this seems to be. Is there a strong rev-lock with clang that I missed?
>
>
> I've put it here because it is similar to scan-build/scan-build-py and on OSX 
> it uses the intercept-build from scan-build-py. Right now there is an 
> alternative logger on linux (not in this patch) but we plan to move to use 
> only the intercept-build.
>  We process the output generated by clang and clang-tidy and we use the 
> command line options for these tools which could change with newer revisions, 
> in that case this tool needs to be updated too.


By strong rev-lock, I meant "Using the internal C++ API" of clang itself.
The scan-build.py thing seems to me like a tiny script compared to CodeChecker.


https://reviews.llvm.org/D24040



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24322: clang-format: [JavaScript] Do requoting in a separate pass

2016-09-07 Thread Daniel Jasper via cfe-commits
djasper closed this revision.
djasper added a comment.

Submitted as r280874.


https://reviews.llvm.org/D24322



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280874 - clang-format: [JavaScript] Do requoting in a separate pass

2016-09-07 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Wed Sep  7 17:48:53 2016
New Revision: 280874

URL: http://llvm.org/viewvc/llvm-project?rev=280874=rev
Log:
clang-format: [JavaScript] Do requoting in a separate pass

The attempt to fix requoting behavior in r280487 after changes to
tooling::Replacements are incomplete. We essentially need to add to
replacements at the same position, one to insert a line break and one to
change the quoting and that's incompatible with the new
tooling::Replacement API, which does not allow for order-dependent
Replacements. To make the order clear, Replacements::merge() has to be
used, but that requires the merged Replacement to actually refer to the
changed text, which is hard to reproduce for the requoting.

This change fixes the behavior by moving the requoting to a completely
separate pass. The added benefit is that no weird ColumnWidth
calculations are necessary anymore and this should just work even if we
implement string literal splitting in the future.

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=280874=280873=280874=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Sep  7 17:48:53 2016
@@ -792,47 +792,25 @@ std::string configurationAsText(const Fo
 
 namespace {
 
-class Formatter : public TokenAnalyzer {
+class JavaScriptRequoter : public TokenAnalyzer {
 public:
-  Formatter(const Environment , const FormatStyle ,
-bool *IncompleteFormat)
-  : TokenAnalyzer(Env, Style), IncompleteFormat(IncompleteFormat) {}
+  JavaScriptRequoter(const Environment , const FormatStyle )
+  : TokenAnalyzer(Env, Style) {}
 
   tooling::Replacements
   analyze(TokenAnnotator ,
   SmallVectorImpl ,
   FormatTokenLexer ) override {
-tooling::Replacements RequoteReplaces;
-deriveLocalStyle(AnnotatedLines);
 AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
   AnnotatedLines.end());
-
-if (Style.Language == FormatStyle::LK_JavaScript &&
-Style.JavaScriptQuotes != FormatStyle::JSQS_Leave)
-  requoteJSStringLiteral(AnnotatedLines, RequoteReplaces);
-
-for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
-  Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
-}
-
-Annotator.setCommentLineLevels(AnnotatedLines);
-
-WhitespaceManager Whitespaces(
-Env.getSourceManager(), Style,
-inputUsesCRLF(Env.getSourceManager().getBufferData(Env.getFileID(;
-ContinuationIndenter Indenter(Style, Tokens.getKeywords(),
-  Env.getSourceManager(), Whitespaces, 
Encoding,
-  BinPackInconclusiveFunctions);
-UnwrappedLineFormatter(, , Style, 
Tokens.getKeywords(),
-   IncompleteFormat)
-.format(AnnotatedLines);
-return RequoteReplaces.merge(Whitespaces.generateReplacements());
+tooling::Replacements Result;
+requoteJSStringLiteral(AnnotatedLines, Result);
+return Result;
   }
 
 private:
-  // If the last token is a double/single-quoted string literal, generates a
-  // replacement with a single/double quoted string literal, re-escaping the
-  // contents in the process.
+  // Replaces double/single-quoted string literal as appropriate, re-escaping
+  // the contents in the process.
   void requoteJSStringLiteral(SmallVectorImpl ,
   tooling::Replacements ) {
 for (AnnotatedLine *Line : Lines) {
@@ -844,8 +822,7 @@ private:
 StringRef Input = FormatTok->TokenText;
 if (FormatTok->Finalized || !FormatTok->isStringLiteral() ||
 // NB: testing for not starting with a double quote to avoid
-// breaking
-// `template strings`.
+// breaking `template strings`.
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Single &&
  !Input.startswith("\"")) ||
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Double &&
@@ -870,7 +847,6 @@ private:
 IsSingle ? "'" : "\"");
 
 // Escape internal quotes.
-size_t ColumnWidth = FormatTok->TokenText.size();
 bool Escaped = false;
 for (size_t i = 1; i < Input.size() - 1; i++) {
   switch (Input[i]) {
@@ -880,7 +856,6 @@ private:
  (!IsSingle && Input[i + 1] == '\''))) {
   // Remove this \, it's escaping a " or ' that no longer needs
   // escaping
-  ColumnWidth--;
   Replace(Start.getLocWithOffset(i), 1, "");
   continue;
 }
@@ -891,7 +866,6 @@ private:
 if (!Escaped && IsSingle == (Input[i] == '\'')) {
   // Escape the quote.
   

Re: [PATCH] D24322: clang-format: [JavaScript] Do requoting in a separate pass

2016-09-07 Thread Daniel Jasper via cfe-commits
djasper added inline comments.


Comment at: lib/Format/Format.cpp:1707
@@ +1706,3 @@
+  if (NewCode) {
+auto NewEnv = Environment::CreateVirtualEnvironment(
+*NewCode, FileName,

mprobst wrote:
> I guess we don't worry about the performance consequences here?
I don't think so. Lexing is very fast and we do the same thing to sort 
includes. We only create a new virtual environment if there actually are quotes 
to change which should be rare and also creating this environment should be 
fast.

All in all, I'd ignore performance here until this actually shows up in a 
profile (or we trace a slow formatting back to it).


https://reviews.llvm.org/D24322



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24322: clang-format: [JavaScript] Do requoting in a separate pass

2016-09-07 Thread Martin Probst via cfe-commits
mprobst added inline comments.


Comment at: lib/Format/Format.cpp:1707
@@ +1706,3 @@
+  if (NewCode) {
+auto NewEnv = Environment::CreateVirtualEnvironment(
+*NewCode, FileName,

I guess we don't worry about the performance consequences here?


https://reviews.llvm.org/D24322



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D24322: clang-format: [JavaScript] Do requoting in a separate pass

2016-09-07 Thread Daniel Jasper via cfe-commits
djasper created this revision.
djasper added a reviewer: mprobst.
djasper added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

The attempt to fix requoting behavior in r280487 after changes to 
tooling::Replacements are incomplete. We essentially need to add to 
replacements at the same position, one to insert a line break and one to change 
the quoting and that's incompatible with the new tooling::Replacement API, 
which does not allow for order-dependent Replacements. To make the order clear, 
Replacements::merge() has to be used, but that requires the merged Replacement 
to actually refer to the changed text, which is hard to reproduce for the 
requoting.

This change fixes the behavior by moving the requoting to a completely separate 
pass. The added benefit is that no weird ColumnWidth calculations are necessary 
anymore and this should just work even if we implement string literal splitting 
in the future.

https://reviews.llvm.org/D24322

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -1342,6 +1342,14 @@
"'double quoted string that needs wrapping');",
"let x = someVeryLongFunctionThatGoesOnAndOn("
"\"double quoted string that needs wrapping\");");
+
+  verifyFormat("let x =\n"
+   "'foo\\'oo';\n"
+   "let x =\n"
+   "'foo\\'oo';",
+   "let x=\"foo'oo\";\n"
+   "let x=\"foo'oo\";",
+   getGoogleJSStyleWithColumns(15));
 }
 
 TEST_F(FormatTestJS, RequoteStringsDouble) {
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -792,47 +792,25 @@
 
 namespace {
 
-class Formatter : public TokenAnalyzer {
+class JavaScriptRequoter : public TokenAnalyzer {
 public:
-  Formatter(const Environment , const FormatStyle ,
-bool *IncompleteFormat)
-  : TokenAnalyzer(Env, Style), IncompleteFormat(IncompleteFormat) {}
+  JavaScriptRequoter(const Environment , const FormatStyle )
+  : TokenAnalyzer(Env, Style) {}
 
   tooling::Replacements
   analyze(TokenAnnotator ,
   SmallVectorImpl ,
   FormatTokenLexer ) override {
-tooling::Replacements RequoteReplaces;
-deriveLocalStyle(AnnotatedLines);
 AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
   AnnotatedLines.end());
-
-if (Style.Language == FormatStyle::LK_JavaScript &&
-Style.JavaScriptQuotes != FormatStyle::JSQS_Leave)
-  requoteJSStringLiteral(AnnotatedLines, RequoteReplaces);
-
-for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
-  Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
-}
-
-Annotator.setCommentLineLevels(AnnotatedLines);
-
-WhitespaceManager Whitespaces(
-Env.getSourceManager(), Style,
-inputUsesCRLF(Env.getSourceManager().getBufferData(Env.getFileID(;
-ContinuationIndenter Indenter(Style, Tokens.getKeywords(),
-  Env.getSourceManager(), Whitespaces, Encoding,
-  BinPackInconclusiveFunctions);
-UnwrappedLineFormatter(, , Style, Tokens.getKeywords(),
-   IncompleteFormat)
-.format(AnnotatedLines);
-return RequoteReplaces.merge(Whitespaces.generateReplacements());
+tooling::Replacements Result;
+requoteJSStringLiteral(AnnotatedLines, Result);
+return Result;
   }
 
 private:
-  // If the last token is a double/single-quoted string literal, generates a
-  // replacement with a single/double quoted string literal, re-escaping the
-  // contents in the process.
+  // Replaces double/single-quoted string literal as appropriate, re-escaping
+  // the contents in the process.
   void requoteJSStringLiteral(SmallVectorImpl ,
   tooling::Replacements ) {
 for (AnnotatedLine *Line : Lines) {
@@ -844,8 +822,7 @@
 StringRef Input = FormatTok->TokenText;
 if (FormatTok->Finalized || !FormatTok->isStringLiteral() ||
 // NB: testing for not starting with a double quote to avoid
-// breaking
-// `template strings`.
+// breaking `template strings`.
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Single &&
  !Input.startswith("\"")) ||
 (Style.JavaScriptQuotes == FormatStyle::JSQS_Double &&
@@ -870,7 +847,6 @@
 IsSingle ? "'" : "\"");
 
 // Escape internal quotes.
-size_t ColumnWidth = FormatTok->TokenText.size();
 bool Escaped = false;
 for (size_t i = 1; i < Input.size() - 1; i++) {
   switch (Input[i]) {
@@ -880,7 +856,6 @@

[libclc] r280871 - Avoid ambiguity in calling atom_add functions.

2016-09-07 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Wed Sep  7 17:11:02 2016
New Revision: 280871

URL: http://llvm.org/viewvc/llvm-project?rev=280871=rev
Log:
Avoid ambiguity in calling atom_add functions.

clang (since r280553) allows pointer casts in function overloads,
so we need to disambiguate the second argument.

clang might be smarter about overloads in the future
see https://reviews.llvm.org/D24113, but let's be safe in libclc anyway.

Modified:
libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_dec.cl
libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_inc.cl
libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_dec.cl
libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_inc.cl

Modified: libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_dec.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_dec.cl?rev=280871=280870=280871=diff
==
--- libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_dec.cl 
(original)
+++ libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_dec.cl Wed 
Sep  7 17:11:02 2016
@@ -2,7 +2,7 @@
 
 #define IMPL(TYPE) \
 _CLC_OVERLOAD _CLC_DEF TYPE atom_dec(global TYPE *p) { \
-  return atom_sub(p, 1); \
+  return atom_sub(p, (TYPE)1); \
 }
 
 IMPL(int)

Modified: libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_inc.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_inc.cl?rev=280871=280870=280871=diff
==
--- libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_inc.cl 
(original)
+++ libclc/trunk/generic/lib/cl_khr_global_int32_base_atomics/atom_inc.cl Wed 
Sep  7 17:11:02 2016
@@ -2,7 +2,7 @@
 
 #define IMPL(TYPE) \
 _CLC_OVERLOAD _CLC_DEF TYPE atom_inc(global TYPE *p) { \
-  return atom_add(p, 1); \
+  return atom_add(p, (TYPE)1); \
 }
 
 IMPL(int)

Modified: libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_dec.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_dec.cl?rev=280871=280870=280871=diff
==
--- libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_dec.cl 
(original)
+++ libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_dec.cl Wed 
Sep  7 17:11:02 2016
@@ -2,7 +2,7 @@
 
 #define IMPL(TYPE) \
 _CLC_OVERLOAD _CLC_DEF TYPE atom_dec(local TYPE *p) { \
-  return atom_sub(p, 1); \
+  return atom_sub(p, (TYPE)1); \
 }
 
 IMPL(int)

Modified: libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_inc.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_inc.cl?rev=280871=280870=280871=diff
==
--- libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_inc.cl 
(original)
+++ libclc/trunk/generic/lib/cl_khr_local_int32_base_atomics/atom_inc.cl Wed 
Sep  7 17:11:02 2016
@@ -2,7 +2,7 @@
 
 #define IMPL(TYPE) \
 _CLC_OVERLOAD _CLC_DEF TYPE atom_inc(local TYPE *p) { \
-  return atom_add(p, 1); \
+  return atom_add(p, (TYPE)1); \
 }
 
 IMPL(int)


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit

2016-09-07 Thread Luis Héctor Chávez via cfe-commits
lhchavez created this revision.
lhchavez added a reviewer: djasper.
lhchavez added subscribers: srhines, cfe-commits.
lhchavez set the repository for this revision to rL LLVM.
lhchavez added a project: clang-c.

When building pre-upload hooks using git-clang-format, it is useful to limit 
the scope to a single commit (instead of from a commit against the working 
tree) to allow for less false positives in dependent commits.

This change adds a flag (--single-commit) to git-clang-format, which uses a 
different strategy to diff (using git-diff-tree instead of git-diff-index) and 
create the temporary working tree (using the git hashes of the objects present 
in the specified commit instead of the ones found in the current working 
directory).

Repository:
  rL LLVM

https://reviews.llvm.org/D24319

Files:
  cfe/trunk/tools/clang-format/git-clang-format

Index: cfe/trunk/tools/clang-format/git-clang-format
===
--- cfe/trunk/tools/clang-format/git-clang-format
+++ cfe/trunk/tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
 usage = 'git clang-format [OPTIONS] [] [--] [...]'
 
 desc = '''
-Run clang-format on all lines that differ between the working directory
-and , which defaults to HEAD.  Changes are only applied to the working
-directory.
+Run clang-format on all lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit.  Changes are only applied to the working directory.
 
 The following git-config settings set the default of the corresponding option:
   clangFormat.binary
@@ -90,6 +90,9 @@
   p.add_argument('--commit',
  default=config.get('clangformat.commit', 'HEAD'),
  help='default commit to use if none is specified'),
+  p.add_argument('--single-commit', action='store_true',
+ help=('run clang-format on a single commit instead of against '
+   'the working tree')),
   p.add_argument('--diff', action='store_true',
  help='print a diff instead of applying the changes')
   p.add_argument('--extensions',
@@ -121,7 +124,8 @@
   del opts.quiet
 
   commit, files = interpret_args(opts.args, dash_dash, opts.commit)
-  changed_lines = compute_diff_and_extract_lines(commit, files)
+  changed_lines = compute_diff_and_extract_lines(commit, files,
+ opts.single_commit)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -141,7 +145,10 @@
   # The computed diff outputs absolute paths, so we must cd before accessing
   # those files.
   cd_to_toplevel()
-  old_tree = create_tree_from_workdir(changed_lines)
+  if opts.single_commit:
+old_tree = create_tree_from_commit(opts.commit, changed_lines)
+  else:
+old_tree = create_tree_from_workdir(changed_lines)
   new_tree = run_clang_format_and_save_to_tree(changed_lines,
binary=opts.binary,
style=opts.style)
@@ -242,9 +249,9 @@
   return stdout.strip()
 
 
-def compute_diff_and_extract_lines(commit, files):
+def compute_diff_and_extract_lines(commit, files, single_commit):
   """Calls compute_diff() followed by extract_lines()."""
-  diff_process = compute_diff(commit, files)
+  diff_process = compute_diff(commit, files, single_commit)
   changed_lines = extract_lines(diff_process.stdout)
   diff_process.stdout.close()
   diff_process.wait()
@@ -254,13 +261,16 @@
   return changed_lines
 
 
-def compute_diff(commit, files):
+def compute_diff(commit, files, single_commit):
   """Return a subprocess object producing the diff from `commit`.
 
   The return value's `stdin` file object will produce a patch with the
   differences between the working directory and `commit`, filtered on `files`
   (if non-empty).  Zero context lines are used in the patch."""
-  cmd = ['git', 'diff-index', '-p', '-U0', commit, '--']
+  git_tool = 'diff-index'
+  if single_commit:
+git_tool = 'diff-tree'
+  cmd = ['git', git_tool, '-p', '-U0', commit, '--']
   cmd.extend(files)
   p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
   p.stdin.close()
@@ -310,6 +320,29 @@
   os.chdir(toplevel)
 
 
+def create_tree_from_commit(commit, filenames):
+  """Create a new git tree with the given files from `commit`.
+
+  Returns the object ID (SHA-1) of the created tree."""
+  def index_info_generator(lines, filenames):
+for line in lines:
+  match = re.match(r'^(\d+) blob ([0-9a-f]+)\t(.*)', line)
+  if not match:
+continue
+  mode, blob_id, filename = match.groups()
+  if filename not in filenames:
+continue
+  yield '%s %s\t%s' % (mode, blob_id, filename)
+  cmd = ['git', 'ls-tree', '-r', commit]
+  p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+  stdout = p.communicate()[0]
+ 

Re: [PATCH] D24115: [Clang] Fix some Clang-tidy modernize-use-using and Include What You Use warnings; other minor fixes

2016-09-07 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280870: Fix some Clang-tidy modernize-use-using and Include 
What You Use warnings… (authored by eugenezelenko).

Changed prior to commit:
  https://reviews.llvm.org/D24115?vs=69937=70606#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24115

Files:
  cfe/trunk/lib/Lex/Lexer.cpp
  cfe/trunk/lib/Lex/LiteralSupport.cpp
  cfe/trunk/lib/Lex/PPDirectives.cpp
  cfe/trunk/lib/Lex/PPExpressions.cpp
  cfe/trunk/lib/Lex/PPMacroExpansion.cpp
  cfe/trunk/lib/Lex/Pragma.cpp
  cfe/trunk/lib/Lex/Preprocessor.cpp
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp

Index: cfe/trunk/lib/Lex/LiteralSupport.cpp
===
--- cfe/trunk/lib/Lex/LiteralSupport.cpp
+++ cfe/trunk/lib/Lex/LiteralSupport.cpp
@@ -14,13 +14,25 @@
 
 #include "clang/Lex/LiteralSupport.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/LexDiagnostic.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Token.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
+#include 
+#include 
+#include  
+#include 
+#include 
 
 using namespace clang;
 
@@ -135,7 +147,7 @@
   if (Diags)
 Diag(Diags, Features, Loc, ThisTokBegin, EscapeBegin, ThisTokBuf,
  diag::err_hex_escape_no_digits) << "x";
-  HadError = 1;
+  HadError = true;
   break;
 }
 
@@ -453,7 +465,6 @@
   ResultBuf += bytesToWrite;
 }
 
-
 ///   integer-constant: [C99 6.4.4.1]
 /// decimal-constant integer-suffix
 /// octal-constant integer-suffix
@@ -986,7 +997,6 @@
   return Result.convertFromString(Str, APFloat::rmNearestTiesToEven);
 }
 
-
 /// \verbatim
 ///   user-defined-character-literal: [C++11 lex.ext]
 /// character-literal ud-suffix
Index: cfe/trunk/lib/Lex/Preprocessor.cpp
===
--- cfe/trunk/lib/Lex/Preprocessor.cpp
+++ cfe/trunk/lib/Lex/Preprocessor.cpp
@@ -43,15 +43,24 @@
 #include "clang/Lex/PreprocessingRecord.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/ScratchBuffer.h"
-#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Capacity.h"
-#include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+
 using namespace clang;
 
 LLVM_INSTANTIATE_REGISTRY(PragmaHandlerRegistry)
@@ -74,11 +83,12 @@
   IncrementalProcessing(false), TUKind(TUKind), CodeComplete(nullptr),
   CodeCompletionFile(nullptr), CodeCompletionOffset(0),
   LastTokenWasAt(false), ModuleImportExpectsIdentifier(false),
-  CodeCompletionReached(0), CodeCompletionII(0), MainFileDir(nullptr),
-  SkipMainFilePreamble(0, true), CurPPLexer(nullptr), CurDirLookup(nullptr),
-  CurLexerKind(CLK_Lexer), CurSubmodule(nullptr), Callbacks(nullptr),
-  CurSubmoduleState(), MacroArgCache(nullptr),
-  Record(nullptr), MIChainHead(nullptr), DeserialMIChainHead(nullptr) {
+  CodeCompletionReached(false), CodeCompletionII(nullptr),
+  MainFileDir(nullptr), SkipMainFilePreamble(0, true), CurPPLexer(nullptr),
+  CurDirLookup(nullptr), CurLexerKind(CLK_Lexer), CurSubmodule(nullptr),
+  Callbacks(nullptr), CurSubmoduleState(),
+  MacroArgCache(nullptr), Record(nullptr), MIChainHead(nullptr),
+  DeserialMIChainHead(nullptr) {
   OwnsHeaderSearch = OwnsHeaders;
   
   CounterValue = 0; // __COUNTER__ starts at 0.
@@ -490,7 +500,6 @@
 // Preprocessor Initialization Methods
 //===--===//
 
-
 /// EnterMainSourceFile - Enter the specified FileID as the main source file,
 /// which implicitly adds the builtin defines etc.
 void Preprocessor::EnterMainSourceFile() {
@@ -758,7 +767,6 @@
   LastTokenWasAt = Result.is(tok::at);
 }
 
-
 /// \brief Lex a token following the 'import' contextual keyword.
 ///
 void Preprocessor::LexAfterModuleImport(Token ) {
Index: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
===
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp
@@ -13,23 +13,48 @@
 

r280870 - Fix some Clang-tidy modernize-use-using and Include What You Use warnings; other minor fixes.

2016-09-07 Thread Eugene Zelenko via cfe-commits
Author: eugenezelenko
Date: Wed Sep  7 16:53:17 2016
New Revision: 280870

URL: http://llvm.org/viewvc/llvm-project?rev=280870=rev
Log:
Fix some Clang-tidy modernize-use-using and Include What You Use warnings; 
other minor fixes.

Differential revision: https://reviews.llvm.org/D24115

Modified:
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/lib/Lex/LiteralSupport.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/PPExpressions.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/lib/Lex/Pragma.cpp
cfe/trunk/lib/Lex/Preprocessor.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/lib/Lex/Lexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=280870=280869=280870=diff
==
--- cfe/trunk/lib/Lex/Lexer.cpp (original)
+++ cfe/trunk/lib/Lex/Lexer.cpp Wed Sep  7 16:53:17 2016
@@ -14,18 +14,27 @@
 #include "clang/Lex/Lexer.h"
 #include "UnicodeCharSets.h"
 #include "clang/Basic/CharInfo.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/Preprocessor.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/UnicodeCharRanges.h"
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+#include 
+#include 
+
 using namespace clang;
 
 
//===--===//
@@ -45,7 +54,6 @@ tok::ObjCKeywordKind Token::getObjCKeywo
   return specId ? specId->getObjCKeywordID() : tok::objc_not_keyword;
 }
 
-
 
//===--===//
 // Lexer Class Implementation
 
//===--===//
@@ -196,7 +204,6 @@ Lexer *Lexer::Create_PragmaLexer(SourceL
   return L;
 }
 
-
 /// Stringify - Convert the specified string into a C string, with surrounding
 /// ""'s, and with escaped \ and " characters.
 std::string Lexer::Stringify(StringRef Str, bool Charify) {
@@ -398,7 +405,6 @@ unsigned Lexer::getSpelling(const Token
   return getSpellingSlow(Tok, TokStart, LangOpts, const_cast(Buffer));
 }
 
-
 /// MeasureTokenLength - Relex the token at the specified location and return
 /// its length in bytes in the input file.  If the token needs cleaning (e.g.
 /// includes a trigraph or an escaped newline) then this count includes bytes
@@ -526,13 +532,15 @@ SourceLocation Lexer::GetBeginningOfToke
 }
 
 namespace {
+
   enum PreambleDirectiveKind {
 PDK_Skipped,
 PDK_StartIf,
 PDK_EndIf,
 PDK_Unknown
   };
-}
+
+} // end anonymous namespace
 
 std::pair Lexer::ComputePreamble(StringRef Buffer,
  const LangOptions ,
@@ -694,7 +702,6 @@ std::pair Lexer::Compute
: TheTok.isAtStartOfLine());
 }
 
-
 /// AdvanceToTokenCharacter - Given a location that specifies the start of a
 /// token, return a new location that specifies a character within the token.
 SourceLocation Lexer::AdvanceToTokenCharacter(SourceLocation TokStart,
@@ -961,7 +968,7 @@ StringRef Lexer::getImmediateMacroName(S
   assert(Loc.isMacroID() && "Only reasonble to call this on macros");
 
   // Find the location of the immediate macro expansion.
-  while (1) {
+  while (true) {
 FileID FID = SM.getFileID(Loc);
 const SrcMgr::SLocEntry *E = (FID);
 const SrcMgr::ExpansionInfo  = E->getExpansion();
@@ -1031,7 +1038,6 @@ bool Lexer::isIdentifierBodyChar(char c,
   return isIdentifierBody(c, LangOpts.DollarIdents);
 }
 
-
 
//===--===//
 // Diagnostics forwarding code.
 
//===--===//
@@ -1157,7 +1163,7 @@ unsigned Lexer::getEscapedNewLineSize(co
 /// them), skip over them and return the first non-escaped-newline found,
 /// otherwise return P.
 const char *Lexer::SkipEscapedNewLines(const char *P) {
-  while (1) {
+  while (true) {
 const char *AfterEscape;
 if (*P == '\\') {
   AfterEscape = P+1;
@@ -1310,7 +1316,6 @@ Slash:
   return *Ptr;
 }
 
-
 /// getCharAndSizeSlowNoWarn - Handle the slow/uncommon case of the
 /// getCharAndSizeNoWarn method.  Here we know that we can accumulate into 
Size,
 /// and that we have already incremented Ptr by Size bytes.
@@ -1548,7 +1553,7 @@ FinishIdentifier:
   // Otherwise, $,\,? in identifier found.  Enter slower path.
 
   C = getCharAndSize(CurPtr, Size);
-  while (1) {
+  while (true) {
  

Re: [PATCH] D24314: [libc++] Clean up MSVC support

2016-09-07 Thread Shoaib Meenai via cfe-commits
smeenai marked 2 inline comments as done.
smeenai added a comment.

https://reviews.llvm.org/D24314



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24314: [libc++] Clean up MSVC support

2016-09-07 Thread Shoaib Meenai via cfe-commits
smeenai updated this revision to Diff 70598.
smeenai added a comment.

Simpliying conditional per EricWF's suggestion


https://reviews.llvm.org/D24314

Files:
  include/support/win32/support.h

Index: include/support/win32/support.h
===
--- include/support/win32/support.h
+++ include/support/win32/support.h
@@ -22,7 +22,7 @@
 #include 
 #endif
 #if defined(_LIBCPP_MSVCRT)
-#include 
+#include 
 #endif
 #define swprintf _snwprintf
 #define vswprintf _vsnwprintf
@@ -44,26 +44,8 @@
 }
 #endif // __MINGW32__
 
-#if defined(_LIBCPP_MSVCRT)
+#if defined(_VC_CRT_MAJOR_VERSION) && _VC_CRT_MAJOR_VERSION < 14
 #define snprintf _snprintf
-#define atoll _atoi64
-#define strtoll _strtoi64
-#define strtoull _strtoui64
-#define wcstoll _wcstoi64
-#define wcstoull _wcstoui64
-_LIBCPP_ALWAYS_INLINE float strtof(const char *nptr, char **endptr)
-{
-  return _Stof(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE double strtod(const char *nptr, char **endptr)
-{
-  return _Stod(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE long double strtold(const char *nptr, char **endptr)
-{
-  return _Stold(nptr, endptr, 0);
-}
-
 #define _Exit _exit
 #endif
 


Index: include/support/win32/support.h
===
--- include/support/win32/support.h
+++ include/support/win32/support.h
@@ -22,7 +22,7 @@
 #include 
 #endif
 #if defined(_LIBCPP_MSVCRT)
-#include 
+#include 
 #endif
 #define swprintf _snwprintf
 #define vswprintf _vsnwprintf
@@ -44,26 +44,8 @@
 }
 #endif // __MINGW32__
 
-#if defined(_LIBCPP_MSVCRT)
+#if defined(_VC_CRT_MAJOR_VERSION) && _VC_CRT_MAJOR_VERSION < 14
 #define snprintf _snprintf
-#define atoll _atoi64
-#define strtoll _strtoi64
-#define strtoull _strtoui64
-#define wcstoll _wcstoi64
-#define wcstoull _wcstoui64
-_LIBCPP_ALWAYS_INLINE float strtof(const char *nptr, char **endptr)
-{
-  return _Stof(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE double strtod(const char *nptr, char **endptr)
-{
-  return _Stod(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE long double strtold(const char *nptr, char **endptr)
-{
-  return _Stold(nptr, endptr, 0);
-}
-
 #define _Exit _exit
 #endif
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24314: [libc++] Clean up MSVC support

2016-09-07 Thread Shoaib Meenai via cfe-commits
smeenai added inline comments.


Comment at: include/support/win32/support.h:48
@@ -47,2 +47,3 @@
 #if defined(_LIBCPP_MSVCRT)
+#if _VC_CRT_MAJOR_VERSION < 14
 #define snprintf _snprintf

EricWF wrote:
> Maybe fold these to `#if`'s into `#if defined(_VC_CRT_MAJOR_VERSION) && 
> _VC_CRT_MAJOR_VERSION < 14`.
Good idea.


https://reviews.llvm.org/D24314



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D24314: [libc++] Clean up MSVC support

2016-09-07 Thread Shoaib Meenai via cfe-commits
smeenai created this revision.
smeenai added reviewers: compnerd, EricWF, mclow.lists.
smeenai added a subscriber: cfe-commits.

Visual Studio 2013 (CRT version 12) added support for many C99 long long
and long double functions. Visual Studio 2015 (CRT version 14) increased
C99 and C11 compliance further. Since we don't support Visual Studio
versions older than 2013, we can considerably clean up the support
header.

https://reviews.llvm.org/D24314

Files:
  include/support/win32/support.h

Index: include/support/win32/support.h
===
--- include/support/win32/support.h
+++ include/support/win32/support.h
@@ -22,7 +22,7 @@
 #include 
 #endif
 #if defined(_LIBCPP_MSVCRT)
-#include 
+#include 
 #endif
 #define swprintf _snwprintf
 #define vswprintf _vsnwprintf
@@ -45,27 +45,11 @@
 #endif // __MINGW32__
 
 #if defined(_LIBCPP_MSVCRT)
+#if _VC_CRT_MAJOR_VERSION < 14
 #define snprintf _snprintf
-#define atoll _atoi64
-#define strtoll _strtoi64
-#define strtoull _strtoui64
-#define wcstoll _wcstoi64
-#define wcstoull _wcstoui64
-_LIBCPP_ALWAYS_INLINE float strtof(const char *nptr, char **endptr)
-{
-  return _Stof(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE double strtod(const char *nptr, char **endptr)
-{
-  return _Stod(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE long double strtold(const char *nptr, char **endptr)
-{
-  return _Stold(nptr, endptr, 0);
-}
-
 #define _Exit _exit
 #endif
+#endif
 
 #if defined(_LIBCPP_MSVC)
 


Index: include/support/win32/support.h
===
--- include/support/win32/support.h
+++ include/support/win32/support.h
@@ -22,7 +22,7 @@
 #include 
 #endif
 #if defined(_LIBCPP_MSVCRT)
-#include 
+#include 
 #endif
 #define swprintf _snwprintf
 #define vswprintf _vsnwprintf
@@ -45,27 +45,11 @@
 #endif // __MINGW32__
 
 #if defined(_LIBCPP_MSVCRT)
+#if _VC_CRT_MAJOR_VERSION < 14
 #define snprintf _snprintf
-#define atoll _atoi64
-#define strtoll _strtoi64
-#define strtoull _strtoui64
-#define wcstoll _wcstoi64
-#define wcstoull _wcstoui64
-_LIBCPP_ALWAYS_INLINE float strtof(const char *nptr, char **endptr)
-{
-  return _Stof(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE double strtod(const char *nptr, char **endptr)
-{
-  return _Stod(nptr, endptr, 0);
-}
-_LIBCPP_ALWAYS_INLINE long double strtold(const char *nptr, char **endptr)
-{
-  return _Stold(nptr, endptr, 0);
-}
-
 #define _Exit _exit
 #endif
+#endif
 
 #if defined(_LIBCPP_MSVC)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

2016-09-07 Thread Gábor Horváth via cfe-commits
xazax.hun added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1003
@@ +1002,3 @@
+//
+ProgramStateRef MallocChecker::addExtentSize(CheckerContext ,
+ const CXXNewExpr *NE,

zaks.anna wrote:
> I am not sure this code belongs to the malloc checker since it only supports 
> the array bounds checker. Is there a reason it's not part of that checker?
I think it is part of the malloc checker because it already does something very 
very similar to malloc, see the MallocMemAux function. So in fact, for the 
array bounds checker to work properly, the malloc checker should be turned on.


https://reviews.llvm.org/D24307



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

2016-09-07 Thread Eric Fiselier via cfe-commits
EricWF added inline comments.


Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+while (auto head = dtors) {
+  dtors = head->next;
+  head->dtor(head->obj);

tavianator wrote:
> EricWF wrote:
> > There is a bug here. If `head->next == nullptr` and if 
> > `head->dtor(head->obj))` creates a TL variable in the destructor then that 
> > destructor will not be invoked.
> > 
> > Here's an updated test case which catches the bug: 
> > https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
> I can't reproduce that failure here, your exact test case passes (even with 
> `#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test commented 
> out).
> 
> Tracing the implementation logic, it seems correct.  If `head->next == 
> nullptr` then this line does `dtors = nullptr`.  Then if 
> `head->dtor(head->obj)` registers a new `thread_local`, 
> `__cxa_thread_atexit()` does `head = malloc(...); ... dtors = head;`.  Then 
> the next iteration of the loop `while (auto head = dtors) {` picks up that 
> new node.
> 
> Have I missed something?
I can't reproduce this morning either, I must have been doing something funny. 
I'll look at this with a fresh head tomorrow. If I can't find anything this 
will be good to go. Thanks for working on this. 


https://reviews.llvm.org/D21803



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Reid Kleckner via cfe-commits
rnk added a comment.

You should locally verify that this generates the correct assembly when 
optimizations are enabled, and if it doesn't, try to make the input look more 
like llvm/test/CodeGen/X86/rotate.ll



Comment at: test/CodeGen/ms-intrinsics-rotations.c:2
@@ +1,3 @@
+// RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=17.00 \
+// RUN: -triple i686--windows -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG

Can we run without -Oz? The test should still pass.


https://reviews.llvm.org/D24311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: lib/CodeGen/CGBuiltin.cpp:740
@@ +739,3 @@
+Value *LeftShifted = Builder.CreateShl(Val, Shift);
+Value *RightShifted = Builder.CreateLShr(Val, RightShift);
+Value *Shifted = Builder.CreateOr(LeftShifted, RightShifted);

I don't think this is correct when rotating by zero. That will cause us to 
shift right by ArgTypeSize, which will result in an undefined value: 
http://llvm.org/docs/LangRef.html#lshr-instruction

I think we need to generate a select or conditional branch as in the original 
code.


Comment at: test/CodeGen/ms-intrinsics-rotations.c:138
@@ +137,3 @@
+
+// CHECK-64BIT-LONG: i64 @test_lrotr
+// CHECK-64BIT-LONG:   [[SHIFT:%[0-9]+]] = and i64 %shift, 63

Nice, testing both LP64 and LLP64. :)


https://reviews.llvm.org/D24311



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

2016-09-07 Thread Anna Zaks via cfe-commits
zaks.anna added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1003
@@ +1002,3 @@
+//
+ProgramStateRef MallocChecker::addExtentSize(CheckerContext ,
+ const CXXNewExpr *NE,

I am not sure this code belongs to the malloc checker since it only supports 
the array bounds checker. Is there a reason it's not part of that checker?


https://reviews.llvm.org/D24307



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24243: [clang-move] A prototype tool for moving class definition to new file.

2016-09-07 Thread Eric Liu via cfe-commits
ioeric added inline comments.


Comment at: clang-move/ClangMove.cpp:103
@@ +102,3 @@
+ const clang::SourceManager *SM) {
+  // Gets the ending ";".
+  auto EndLoc = clang::Lexer::getLocForEndOfToken(D->getLocEnd(), 0, *SM,

hokein wrote:
> ioeric wrote:
> > Consider the code below to also include trailing comment.
> > clang::SourceLocation after_semi = clang::Lexer::findLocationAfterToken(
> > D->getLocEnd, clang::tok::semi, *SM,
> > result.Context->getLangOpts(),
> > /*SkipTrailingWhitespaceAndNewLine=*/true);
> > 
> But this code could not handle cases where the declaration definition has no 
> semi at the end, such as "void f() {}"
Please see the comment for `findLocationAfterToken`.



> If the token is not found or the location is inside a macro, the returned 
> source location will be invalid.





Comment at: clang-move/ClangMove.cpp:129
@@ +128,3 @@
+  for (const auto  : Decls) {
+std::vector DeclNamespaces = GetNamespaces(MovedDecl.Decl);
+auto CurrentIt = CurrentNamespaces.begin();

hokein wrote:
> ioeric wrote:
> > Is it possible to restrict all `MovedDecl.Decl` to NamedDecl so that we can 
> > use `getQualifiedNameAsString` instead of having a second implementation of 
> > retrieving namespaces. 
> > 
> > Also how is anonymous namespace handled here?
> > 
> > 
> Yeah, `getQualifiedNameAsString` looks like a simpler way, but it doesn't 
> suitable for our usage here.
> 
> The `getQualifiedNameAsString` includes the name of the class.  For instance, 
> a class method decl like `void A::f() {}` defined in global namespace, it 
> retruns `A::f` which is not intended. 
> 
> It handles anonymous namespace by wrapping like `namespace { ... } // 
> namespace`, see the test.
> 
I think it should be relatively easy to split the qualified name returned by 
`getQualifiedNameAsString `.

> It handles anonymous namespace by wrapping like namespace { ... } // 
> namespace, see the test.
I mean...wouldn't `getNamespaces` return something like "a::(anonymous)" for 
anonymous namespace? How do you handle this?



Comment at: clang-move/ClangMove.h:39
@@ +38,3 @@
+  struct MoveDefinitionSpec {
+std::string Name;
+std::string OldHeader;

hokein wrote:
> ioeric wrote:
> > Should the `Name` be fully qualified?
> It's not required. The name can be fully/partially qualified, e.g., 
> `::a::b::X`, `a::b::X`, `b::X`, `X`, which has the same behavior with 
> `hasName` ast matcher.
> 
> It the given name is partially qualified, it will match all the class whose 
> name ends with the given name.
> It the given name is partially qualified, it will match all the class whose 
> name ends with the given name.
In this case, all matched classes will be moved right? Might want to mention 
this in the comment.


Comment at: clang-move/ClangMove.h:65
@@ +64,3 @@
+
+  const MoveDefinitionSpec 
+  // The Key is file path, value is the replacements being applied to the file.

hokein wrote:
> ioeric wrote:
> > Is there any reason why you made this a reference?
> To avoid the cost of making a copy of the structure.
If the copy is not that expensive, I'd just make a copy so that I don't need to 
worry about the life-cycle of `Spec`.


https://reviews.llvm.org/D24243



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280852 - Move CHECK right before the function it describes.

2016-09-07 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Wed Sep  7 15:15:03 2016
New Revision: 280852

URL: http://llvm.org/viewvc/llvm-project?rev=280852=rev
Log:
Move CHECK right before the function it describes.

Modified:
cfe/trunk/test/CodeGen/overloadable.c

Modified: cfe/trunk/test/CodeGen/overloadable.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/overloadable.c?rev=280852=280851=280852=diff
==
--- cfe/trunk/test/CodeGen/overloadable.c (original)
+++ cfe/trunk/test/CodeGen/overloadable.c Wed Sep  7 15:15:03 2016
@@ -75,11 +75,11 @@ void bar() {
   ovl_bar(ucharbuf);
 }
 
-// CHECK-LABEL: define void @baz
 void ovl_baz(int *, int) __attribute__((overloadable));
 void ovl_baz(unsigned int *, unsigned int) __attribute__((overloadable));
 void ovl_baz2(int, int *) __attribute__((overloadable));
 void ovl_baz2(unsigned int, unsigned int *) __attribute__((overloadable));
+// CHECK-LABEL: define void @baz
 void baz() {
   unsigned int j;
   // Initial rules for incompatible pointer conversions made this overload


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24113: Allow implicit conversions between incompatible pointer types in overload resolution in C.

2016-09-07 Thread George Burgess IV via cfe-commits
Fix committed as r280847 -- thanks for the reports!

> It seems to me that we could handle this by ranking viable-but-ill-formed
conversion sequences before non-viable ones in
clang::isBetterOverloadCandidate

That's what I was thinking, too. :)

> Clang reports an ambiguity on line 3

TIL. Woohoo for extensions! r280847 should fix this, as well.

On Wed, Sep 7, 2016 at 11:08 AM, Richard Smith 
wrote:

> On Tue, Sep 6, 2016 at 10:55 PM, George Burgess IV <
> george.burgess...@gmail.com> wrote:
>
>> george.burgess.iv added a comment.
>>
>> > Although I think that users will expect atomic_add(unsigned int *, 5)
>> to work.without error
>>
>>
>> Totally agree; I was just making sure that I understood the problem. :)
>>
>> I think, with a small tweak in semantics, we can make everything work
>> without needing to patch existing code. Let me play around a little.
>
>
> It seems to me that we could handle this by ranking viable-but-ill-formed
> conversion sequences before non-viable ones in
> clang::isBetterOverloadCandidate. This problem is actually more general
> than __attribute__((overloadable)) in C -- there is one form of ill-formed
> implicit conversion sequence that we accept in C++ that leads to the same
> problem:
>
>   void f(char*, int);
>   void f(const char*, unsigned);
>   void g() { f("foo", 0); }
>
> Clang reports an ambiguity on line 3, but in C++11 onwards this code is
> valid because there is no implicit conversion sequence from a string
> literal to char*. (We provide one as an extension, and it's ranked worse
> than any other conversion.)
>
>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D24113
>>
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D24312: [CodeGen] Fix an assert in EmitNullConstant

2016-09-07 Thread Akira Hatanaka via cfe-commits
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a subscriber: cfe-commits.

r235815 changed CGRecordLowering::accumulateBases to ignore non-virtual bases 
of size 0, which prevented adding those non-virtual bases to CGRecordLayout's 
NonVirtualBases. EmitNullConstant calls 
CGRecordLayout::getNonVirtualBaseLLVMFieldNo to get the field number of the 
base, which causes an assert. This patch fixes the bug.

https://reviews.llvm.org/D24312

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/empty-classes.cpp

Index: test/CodeGenCXX/empty-classes.cpp
===
--- test/CodeGenCXX/empty-classes.cpp
+++ test/CodeGenCXX/empty-classes.cpp
@@ -96,3 +96,24 @@
   // Type checked at the top of the file.
   B b;
 };
+
+// This test used to crash when CGRecordLayout::getNonVirtualBaseLLVMFieldNo 
was called.
+namespace record_layout {
+struct X0 {
+  int x[0];
+};
+
+template
+struct X2 : X0 {
+};
+
+template
+struct X3 : X2 {
+  X3() : X2() {}
+};
+
+
+void test0() {
+  X3();
+}
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1532,7 +1532,8 @@
   cast(I.getType()->castAs()->getDecl());
 
 // Ignore empty bases.
-if (base->isEmpty())
+if (base->isEmpty() ||
+CGM.getContext().getASTRecordLayout(base).getNonVirtualSize().isZero())
   continue;
 
 unsigned fieldIndex = layout.getNonVirtualBaseLLVMFieldNo(base);


Index: test/CodeGenCXX/empty-classes.cpp
===
--- test/CodeGenCXX/empty-classes.cpp
+++ test/CodeGenCXX/empty-classes.cpp
@@ -96,3 +96,24 @@
   // Type checked at the top of the file.
   B b;
 };
+
+// This test used to crash when CGRecordLayout::getNonVirtualBaseLLVMFieldNo was called.
+namespace record_layout {
+struct X0 {
+  int x[0];
+};
+
+template
+struct X2 : X0 {
+};
+
+template
+struct X3 : X2 {
+  X3() : X2() {}
+};
+
+
+void test0() {
+  X3();
+}
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1532,7 +1532,8 @@
   cast(I.getType()->castAs()->getDecl());
 
 // Ignore empty bases.
-if (base->isEmpty())
+if (base->isEmpty() ||
+CGM.getContext().getASTRecordLayout(base).getNonVirtualSize().isZero())
   continue;
 
 unsigned fieldIndex = layout.getNonVirtualBaseLLVMFieldNo(base);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280847 - [Sema] Compare bad conversions in overload resolution.

2016-09-07 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Wed Sep  7 15:03:19 2016
New Revision: 280847

URL: http://llvm.org/viewvc/llvm-project?rev=280847=rev
Log:
[Sema] Compare bad conversions in overload resolution.

r280553 introduced an issue where we'd emit ambiguity errors for code
like:

```
void foo(int *, int);
void foo(unsigned int *, unsigned int);

void callFoo() {
  unsigned int i;
  foo(, 0); // ambiguous: int->unsigned int is worse than int->int,
  // but unsigned int*->unsigned int* is better than
  // int*->int*.
}
```

This patch fixes this issue by changing how we handle ill-formed (but
valid) implicit conversions. Candidates with said conversions now always
rank worse than candidates without them, and two candidates are
considered to be equally bad if they both have these conversions for
the same argument.

Additionally, this fixes a case in C++11 where we'd complain about an
ambiguity in a case like:

```
void f(char *, int);
void f(const char *, unsigned);
void g() { f("abc", 0); }
```

...Since conversion to char* from a string literal is considered
ill-formed in C++11 (and deprecated in C++03), but we accept it as an
extension.

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CodeGen/overloadable.c
cfe/trunk/test/SemaCXX/overload-call.cpp

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=280847=280846=280847=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Sep  7 15:03:19 2016
@@ -470,10 +470,11 @@ semantics:
 * A conversion from type ``T`` to a value of type ``U`` is permitted if ``T``
   and ``U`` are compatible types.  This conversion is given "conversion" rank.
 
-* A conversion from a pointer of type ``T*`` to a pointer of type ``U*``, where
-  ``T`` and ``U`` are incompatible, is allowed, but is ranked below all other
-  types of conversions. Please note: ``U`` lacking qualifiers that are present
-  on ``T`` is sufficient for ``T`` and ``U`` to be incompatible.
+* If no viable candidates are otherwise available, we allow a conversion from a
+  pointer of type ``T*`` to a pointer of type ``U*``, where ``T`` and ``U`` are
+  incompatible. This conversion is ranked below all other types of conversions.
+  Please note: ``U`` lacking qualifiers that are present on ``T`` is sufficient
+  for ``T`` and ``U`` to be incompatible.
 
 The declaration of ``overloadable`` functions is restricted to function
 declarations and definitions.  Most importantly, if any function with a given

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=280847=280846=280847=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Sep  7 15:03:19 2016
@@ -8600,13 +8600,40 @@ bool clang::isBetterOverloadCandidate(Se
   if (Cand1.IgnoreObjectArgument || Cand2.IgnoreObjectArgument)
 StartArg = 1;
 
+  auto IsIllFormedConversion = [&](const ImplicitConversionSequence ) {
+// We don't allow incompatible pointer conversions in C++.
+if (!S.getLangOpts().CPlusPlus)
+  return ICS.isStandard() &&
+ ICS.Standard.Second == ICK_Incompatible_Pointer_Conversion;
+
+// The only ill-formed conversion we allow in C++ is the string literal to
+// char* conversion, which is only considered ill-formed after C++11.
+return S.getLangOpts().CPlusPlus11 && !S.getLangOpts().WritableStrings &&
+   hasDeprecatedStringLiteralToCharPtrConversion(ICS);
+  };
+
+  // Define functions that don't require ill-formed conversions for a given
+  // argument to be better candidates than functions that do.
+  unsigned NumArgs = Cand1.NumConversions;
+  assert(Cand2.NumConversions == NumArgs && "Overload candidate mismatch");
+  bool HasBetterConversion = false;
+  for (unsigned ArgIdx = StartArg; ArgIdx < NumArgs; ++ArgIdx) {
+bool Cand1Bad = IsIllFormedConversion(Cand1.Conversions[ArgIdx]);
+bool Cand2Bad = IsIllFormedConversion(Cand2.Conversions[ArgIdx]);
+if (Cand1Bad != Cand2Bad) {
+  if (Cand1Bad)
+return false;
+  HasBetterConversion = true;
+}
+  }
+
+  if (HasBetterConversion)
+return true;
+
   // C++ [over.match.best]p1:
   //   A viable function F1 is defined to be a better function than another
   //   viable function F2 if for all arguments i, ICSi(F1) is not a worse
   //   conversion sequence than ICSi(F2), and then...
-  unsigned NumArgs = Cand1.NumConversions;
-  assert(Cand2.NumConversions == NumArgs && "Overload candidate mismatch");
-  bool HasBetterConversion = false;
   for (unsigned ArgIdx = StartArg; ArgIdx < NumArgs; ++ArgIdx) {
 switch 

Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

2016-09-07 Thread Eric Liu via cfe-commits
ioeric added inline comments.


Comment at: change-namespace/ChangeNamespace.cpp:125
@@ +124,3 @@
+// applying all existing Replaces first if there is conflict.
+void addOrMergeReplacement(const tooling::Replacement ,
+   tooling::Replacements *Replaces) {

alexshap wrote:
> khm, this seems to be a recurring pattern (if i'm not mistaken),
> looks like the interface of tooling::Replacements can be changed/improved 
> (although i don't know).
> Another (separate) observation is about duplicates. For example for 
> replacements in headers we can get into if(Err) branch pretty frequently 
> because the same replacement can be generated multiple times (if that header 
> is included into several *.cpp files). 
SGTM. An `addOrMerge()` interface for tooling::Replacements should be helpful. 


Comment at: change-namespace/ChangeNamespace.cpp:481
@@ +480,3 @@
+  continue;
+}
+FileToReplacements[FilePath] = *CleanReplacements;

Added a `FallbackStyle`.


Comment at: change-namespace/tool/ClangChangeNamespace.cpp:105
@@ +104,3 @@
+outs() << "== " << File << " ==\n";
+Rewrite.getEditBuffer(ID).write(llvm::outs());
+outs() << "\n\n";

I'd like to keep this for now for better readability. Will change the format in 
the future if necessary.


https://reviews.llvm.org/D24183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

2016-09-07 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 70588.
ioeric marked 9 inline comments as done.
ioeric added a comment.
Herald added a subscriber: beanz.

- Addressed reviewer comments.


https://reviews.llvm.org/D24183

Files:
  CMakeLists.txt
  change-namespace/CMakeLists.txt
  change-namespace/ChangeNamespace.cpp
  change-namespace/ChangeNamespace.h
  change-namespace/tool/CMakeLists.txt
  change-namespace/tool/ClangChangeNamespace.cpp
  test/CMakeLists.txt
  test/change-namespace/simple-move.cpp
  unittests/CMakeLists.txt
  unittests/change-namespace/CMakeLists.txt
  unittests/change-namespace/ChangeNamespaceTests.cpp

Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- /dev/null
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -0,0 +1,234 @@
+//===-- ChangeNamespaceTests.cpp - Change namespace unit tests ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ChangeNamespace.h"
+#include "unittests/Tooling/RewriterTestContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Format/Format.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace change_namespace {
+namespace {
+
+class ChangeNamespaceTest : public ::testing::Test {
+public:
+  std::string runChangeNamespaceOnCode(llvm::StringRef Code) {
+clang::RewriterTestContext Context;
+clang::FileID ID = Context.createInMemoryFile(FileName, Code);
+
+std::map FileToReplacements;
+change_namespace::ChangeNamespaceTool NamespaceTool(
+OldNamespace, NewNamespace, FilePattern, );
+ast_matchers::MatchFinder Finder;
+NamespaceTool.registerMatchers();
+std::unique_ptr Factory =
+tooling::newFrontendActionFactory();
+tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"},
+   FileName);
+formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite);
+return format(Context.getRewrittenText(ID));
+  }
+
+  std::string format(llvm::StringRef Code) {
+tooling::Replacements Replaces = format::reformat(
+format::getLLVMStyle(), Code, {tooling::Range(0, Code.size())});
+auto ChangedCode = tooling::applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(ChangedCode));
+if (!ChangedCode) {
+  llvm::errs() << llvm::toString(ChangedCode.takeError());
+  return "";
+}
+return *ChangedCode;
+  }
+
+protected:
+  std::string FileName = "input.cc";
+  std::string OldNamespace = "na::nb";
+  std::string NewNamespace = "x::y";
+  std::string FilePattern = "input.cc";
+};
+
+TEST_F(ChangeNamespaceTest, NoMatchingNamespace) {
+  std::string Code = "namespace na {\n"
+ "namespace nx {\n"
+ "class A {};\n"
+ "} // namespace nx\n"
+ "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+ "namespace nx {\n"
+ "class A {};\n"
+ "} // namespace nx\n"
+ "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, SimpleMoveWithoutTypeRefs) {
+  std::string Code = "namespace na {\n"
+ "namespace nb {\n"
+ "class A {};\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+  std::string Expected = "\n\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class A {};\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, SimpleMoveIntoAnotherNestedNamespace) {
+  NewNamespace = "na::nc";
+  std::string Code = "namespace na {\n"
+ "namespace nb {\n"
+ "class A {};\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+  std::string Expected = "namespace na {\n"
+ "\n"
+ "namespace nc {\n"
+ "class A {};\n"
+ "} // namespace 

[PATCH] D24311: Implement MS _rot intrinsics

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski created this revision.
agutowski added reviewers: rnk, thakis, Prazek, compnerd.
agutowski added a subscriber: cfe-commits.

https://reviews.llvm.org/D24311

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics-rotations.c

Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -447,61 +447,6 @@
   return (unsigned __int64)__in1 * (unsigned __int64)__in2;
 }
 /**\
-|* Bit Twiddling
-\**/
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotl8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value << _Shift) | (_Value >> (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_rotr8(unsigned char _Value, unsigned char _Shift) {
-  _Shift &= 0x7;
-  return _Shift ? (_Value >> _Shift) | (_Value << (8 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotl16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value << _Shift) | (_Value >> (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-_rotr16(unsigned short _Value, unsigned char _Shift) {
-  _Shift &= 0xf;
-  return _Shift ? (_Value >> _Shift) | (_Value << (16 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotl(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-_rotr(unsigned int _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotl(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (32 - _Shift)) : _Value;
-}
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
-_lrotr(unsigned long _Value, int _Shift) {
-  _Shift &= 0x1f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (32 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotl64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value << _Shift) | (_Value >> (64 - _Shift)) : _Value;
-}
-static
-__inline__ unsigned __int64 __DEFAULT_FN_ATTRS
-_rotr64(unsigned __int64 _Value, int _Shift) {
-  _Shift &= 0x3f;
-  return _Shift ? (_Value >> _Shift) | (_Value << (64 - _Shift)) : _Value;
-}
-/**\
 |* Bit Counting and Testing
 \**/
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -696,6 +696,52 @@
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI_rotr8:
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *LeftShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *RightShifted = Builder.CreateLShr(Val, Shift);
+Value *LeftShifted = Builder.CreateShl(Val, LeftShift);
+Value *Shifted = Builder.CreateOr(LeftShifted, RightShifted);
+
+return RValue::get(Shifted);
+  }
+  case Builtin::BI_rotl8:
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
+Value *Val = EmitScalarExpr(E->getArg(0));
+Value *Shift = EmitScalarExpr(E->getArg(1));
+
+llvm::Type *ArgType = Val->getType();
+Shift = Builder.CreateIntCast(Shift, ArgType, false);
+unsigned ArgWidth = cast(ArgType)->getBitWidth();
+Value *ArgTypeSize = llvm::ConstantInt::get(ArgType, ArgWidth);
+
+Value *Mask = llvm::ConstantInt::get(ArgType, ArgWidth - 1);
+Shift = Builder.CreateAnd(Shift, Mask);
+Value *RightShift = Builder.CreateSub(ArgTypeSize, Shift);
+
+Value *LeftShifted = Builder.CreateShl(Val, Shift);
+Value *RightShifted = Builder.CreateLShr(Val, RightShift);
+Value *Shifted = Builder.CreateOr(LeftShifted, RightShifted);
+
+return RValue::get(Shifted);
+  }
   case 

r280845 - Add a few more test for []-style uuid attributes.

2016-09-07 Thread Nico Weber via cfe-commits
Author: nico
Date: Wed Sep  7 14:41:35 2016
New Revision: 280845

URL: http://llvm.org/viewvc/llvm-project?rev=280845=rev
Log:
Add a few more test for []-style uuid attributes.

- Should diag on a function (clang-cl warns; it's an error in cl)
- Test the attribute on nested classes (clang-cl is more permissive and more
  self-consistent than cl here)

Modified:
cfe/trunk/test/Parser/ms-square-bracket-attributes.mm

Modified: cfe/trunk/test/Parser/ms-square-bracket-attributes.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/ms-square-bracket-attributes.mm?rev=280845=280844=280845=diff
==
--- cfe/trunk/test/Parser/ms-square-bracket-attributes.mm (original)
+++ cfe/trunk/test/Parser/ms-square-bracket-attributes.mm Wed Sep  7 14:41:35 
2016
@@ -94,6 +94,23 @@ struct struct_with_uuid2_trigraph;
 // expected-error@+1 {{uuid attribute contains a malformed GUID}}
 [uuid(0ZA0---C000-0049)] struct struct_with_uuid2;
 
+struct OuterClass {
+  // [] uuids and inner classes are weird in cl.exe: It warns that uuid on
+  // nested types has undefined behavior, and errors out __uuidof() claiming
+  // that the inner type has no assigned uuid.  Things work fine if 
__declspec()
+  // is used instead.  clang-cl handles this fine.
+  [uuid(1000----)] class InnerClass1;
+  [uuid(1000----)] class InnerClass2 {} ic;
+  [uuid(1000----)] static class InnerClass3 {} sic;
+  // Putting `static` in front of [...] causes parse errors in both cl and 
clang
+
+  // This is the only syntax to declare an inner class with []-style attributes
+  // that works in cl: Declare the inner class without an attribute, and then
+  // have the []-style attribute on the definition.
+  class InnerClass;
+};
+[uuid(1000----)] class OuterClass::InnerClass {};
+
 void use_it() {
   (void)__uuidof(struct_with_uuid);
   (void)__uuidof(struct_with_uuid_brace);
@@ -107,7 +124,17 @@ void use_it() {
   (void)__uuidof(struct_with_uuid2_macro);
   (void)__uuidof(struct_with_uuid2_macro_part);
   (void)__uuidof(struct_with_uuid2_trigraph);
+
+  (void)__uuidof(OuterClass::InnerClass);
+  (void)__uuidof(OuterClass::InnerClass1);
+  (void)__uuidof(OuterClass::InnerClass2);
+  (void)__uuidof(OuterClass::InnerClass3);
+  (void)__uuidof(OuterClass().ic);
+  (void)__uuidof(OuterClass::sic);
 }
+
+// expected-warning@+1 {{'uuid' attribute only applies to classes}}
+[uuid("00A0---C000-0049")] void f();
 }
 
 // clang supports these on toplevel decls, but not on local decls since this


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r280844 - Fix a few oversights in the clang-tidy VS plugin.

2016-09-07 Thread Zachary Turner via cfe-commits
Author: zturner
Date: Wed Sep  7 14:41:19 2016
New Revision: 280844

URL: http://llvm.org/viewvc/llvm-project?rev=280844=rev
Log:
Fix a few oversights in the clang-tidy VS plugin.

Over-zealous cleanup of using statements removed some that were
actually needed.  Also cleaned up a few warnings.

Modified:
clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckDatabase.cs
clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckTree.cs
clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyProperties.cs
clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyPropertyGrid.cs

Modified: clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckDatabase.cs
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckDatabase.cs?rev=280844=280843=280844=diff
==
--- clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckDatabase.cs (original)
+++ clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckDatabase.cs Wed Sep  7 
14:41:19 2016
@@ -50,7 +50,7 @@ namespace LLVM.ClangTidy
 foreach (var Check in Checks_)
 {
 if (Names.Contains(Check.Name))
-throw new ArgumentException(String.Format("Check {0} 
exists more than once!", Check.Name));
+continue;
 Names.Add(Check.Name);
 }
 }

Modified: clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckTree.cs
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckTree.cs?rev=280844=280843=280844=diff
==
--- clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckTree.cs (original)
+++ clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/CheckTree.cs Wed Sep  7 
14:41:19 2016
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
+using System.Linq;
 using System.Text;
 
 namespace LLVM.ClangTidy

Modified: clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyProperties.cs
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyProperties.cs?rev=280844=280843=280844=diff
==
--- clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyProperties.cs 
(original)
+++ clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyProperties.cs Wed 
Sep  7 14:41:19 2016
@@ -40,7 +40,6 @@ namespace LLVM.ClangTidy
 static ClangTidyProperties()
 {
 RootProperties_ = new ClangTidyProperties(null);
-PropertyDescriptor D;
 }
 
 public static ClangTidyProperties RootProperties

Modified: 
clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyPropertyGrid.cs
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyPropertyGrid.cs?rev=280844=280843=280844=diff
==
--- clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyPropertyGrid.cs 
(original)
+++ clang-tools-extra/trunk/clang-tidy-vs/ClangTidy/ClangTidyPropertyGrid.cs 
Wed Sep  7 14:41:19 2016
@@ -42,13 +42,6 @@ namespace LLVM.ClangTidy
 /// 
 List> PropertyChain_ = null;
 
-/// 
-/// A tree representing all the checks that the extension knows about, 
used
-/// when serializing a file to intelligently determine when to use 
wildcard
-/// include / exclude rules.
-/// 
-CheckTree Checks_;
-
 public ClangTidyPropertyGrid()
 {
 InitializeComponent();


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r280728 - Modules: Fix an assertion in DeclContext::buildLookup.

2016-09-07 Thread Manman Ren via cfe-commits
On Tue, Sep 6, 2016 at 6:54 PM, Richard Smith  wrote:

> On Tue, Sep 6, 2016 at 11:16 AM, Manman Ren via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: mren
>> Date: Tue Sep  6 13:16:54 2016
>> New Revision: 280728
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=280728=rev
>> Log:
>> Modules: Fix an assertion in DeclContext::buildLookup.
>>
>> When calling getMostRecentDecl, we can pull in more definitions from
>> a module. We call getPrimaryContext afterwards to make sure that
>> we buildLookup on a primary context.
>>
>> rdar://27926200
>>
>> Added:
>> cfe/trunk/test/Modules/Inputs/lookup-assert/
>> cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h
>> cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h
>> cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h
>> cfe/trunk/test/Modules/Inputs/lookup-assert/module.map
>> cfe/trunk/test/Modules/lookup-assert.m
>> Modified:
>> cfe/trunk/lib/AST/DeclBase.cpp
>>
>> Modified: cfe/trunk/lib/AST/DeclBase.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBa
>> se.cpp?rev=280728=280727=280728=diff
>> 
>> ==
>> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Sep  6 13:16:54 2016
>> @@ -1411,10 +1411,6 @@ DeclContext::lookup(DeclarationName Name
>>assert(DeclKind != Decl::LinkageSpec &&
>>   "Should not perform lookups into linkage specs!");
>>
>> -  const DeclContext *PrimaryContext = getPrimaryContext();
>> -  if (PrimaryContext != this)
>> -return PrimaryContext->lookup(Name);
>> -
>>// If we have an external source, ensure that any later redeclarations
>> of this
>>// context have been loaded, since they may add names to the result of
>> this
>>// lookup (or add external visible storage).
>> @@ -1422,6 +1418,12 @@ DeclContext::lookup(DeclarationName Name
>>if (Source)
>>  (void)cast(this)->getMostRecentDecl();
>>
>> +  // getMostRecentDecl can change the result of getPrimaryContext. Call
>> +  // getPrimaryContext afterwards.
>> +  const DeclContext *PrimaryContext = getPrimaryContext();
>> +  if (PrimaryContext != this)
>> +return PrimaryContext->lookup(Name);
>>
>
> This seems like a bug in getPrimaryContext() -- if there is a
> not-yet-loaded definition of the DeclContext, getPrimaryContext() should
> return that instead of returning a non-defining declaration. Why is
> ObjCInterfaceDecl::hasDefinition (indirectly called by getPrimaryContext)
> not loading the definition in this case?
>

In the testing case, we have a definition of the ObjC interface from
textually including a header file, but the definition is also in a module.
getPrimaryContext for ObjCInterface currently does not  pull from the
external source. Since getPrimaryContext  does not guarantee to pull from
the external source, I thought that is why we call getMostRecentDecl in
DeclContext::lookup.

Are you suggesting to pull from external source in getPrimaryContext?

Cheers,
Manman



>
>> +
>>if (hasExternalVisibleStorage()) {
>>  assert(Source && "external visible storage but no external source?");
>>
>>
>> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>> nputs/lookup-assert/Base.h?rev=280728=auto
>> 
>> ==
>> --- cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h Tue Sep  6
>> 13:16:54 2016
>> @@ -0,0 +1,4 @@
>> +@interface BaseInterface
>> +- (void) test;
>> +@end
>> +
>>
>> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>> nputs/lookup-assert/Derive.h?rev=280728=auto
>> 
>> ==
>> --- cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h Tue Sep  6
>> 13:16:54 2016
>> @@ -0,0 +1,3 @@
>> +#include "Base.h"
>> +@interface DerivedInterface : BaseInterface
>> +@end
>>
>> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>> nputs/lookup-assert/H3.h?rev=280728=auto
>> 
>> ==
>> --- cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h Tue Sep  6 13:16:54
>> 2016
>> @@ -0,0 +1 @@
>> +#include "Base.h"
>>
>> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/module.map
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>> nputs/lookup-assert/module.map?rev=280728=auto
>> 
>> ==
>> --- 

Modules: Suggestion on how to fix a regression caused by r259901

2016-09-07 Thread Manman via cfe-commits
Hi Richard,

We noticed a regression for this simple testing case:
rm -rf tmp3
clang -cc1 -fimplicit-module-maps -x objective-c -fmodules 
-fmodules-cache-path=tmp3 -emit-obj standalone.c -I Inputs/
—>
standalone.c:4:6: error: variable has incomplete type 'void'
void foo __P(());
 ^
standalone.c:4:9: error: expected ';' after top level declarator
void foo __P(());
^
;
2 errors generated.
clang -cc1 -fimplicit-module-maps -x objective-c -fmodules 
-fmodules-cache-path=tmp3 -emit-obj standalone.c -I Inputs/
—> This runs fine.

cat standalone.c
#import "C.h"
#import "A.h"

void foo __P(());

cat Inputs/module.map 
module X {
  header "A.h"
  export *
}
// Y imports X, it also uses “__P” as a parameter name
module Y {
  header "B.h"
  export *
}
// Z imports X and Y
module Z {
  header "C.h”
}

cat Inputs/A.h 
#define __P(protos) ()

cat Inputs/B.h
#import "A.h"
#import "B2.h”

cat Inputs/B2.h 
void test(int __P) {
}

cat Inputs/C.h 
#import "A.h"
#import “B.h”

r259901 causes the compiler to write out identifier “__P” without the macro 
information for module Y, which seems to be incorrect. Any suggestion on how to 
fix this?
Why the 2nd run works is related to global index. Global Index only considers 
interesting identifiers, so it skips module Y when calling ModuleManager::visit.

Cheers,
Manman

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23932: [XRay] ARM 32-bit no-Thumb support in Clang

2016-09-07 Thread Serge Rogatch via cfe-commits
rSerge added a comment.

I don't have commit access rights. Could someone commit?


https://reviews.llvm.org/D23932



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24268: Traversing template paramter lists of DeclaratorDecls and/or TagDecls.

2016-09-07 Thread Łukasz Anforowicz via cfe-commits
lukasza added a comment.

In https://reviews.llvm.org/D24268#535463, @rsmith wrote:

> This patch looks great, thank you! Do you have an SVN account or do you need 
> someone to commit this for you?


I don't have an SVN account.  Could you please commit the patch for me?


https://reviews.llvm.org/D24268



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24153: Add bunch of _Interlocked builtins

2016-09-07 Thread David Majnemer via cfe-commits
majnemer added a comment.

In https://reviews.llvm.org/D24153#535992, @rnk wrote:

> Looks good to me. David, do you remember any subtleties here? I seem to 
> recall there were some bugs in our intrin.h implementations, or 
> inconsistencies between us and MSVC.


I can't seem to recall anything major.  Our `_ReadBarrier` and `_WriteBarrier` 
simply map to `_ReadWriteBarrier`.  Our `__faststorefence` is just a normal 
fence instead of an interlocked operation.


https://reviews.llvm.org/D24153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

2016-09-07 Thread Alexander Shaposhnikov via cfe-commits
alexshap added inline comments.


Comment at: change-namespace/tool/ClangChangeNamespace.cpp:48
@@ +47,3 @@
+
+cl::opt OldNamespace("old_namespace", cl::desc("Old namespace."),
+  cl::cat(ChangeNamespaceCategory));

probably you need to add cl::Required
   cl::opt OldNamespace("old_namespace", cl::Required, 
cl::desc("Old namespace."),

cl::cat(ChangeNamespaceCategory));

and the same for NewNamespace.
 


https://reviews.llvm.org/D24183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24054: Do not validate pch when -fno-validate-pch is set

2016-09-07 Thread Yaxun Liu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280842: Do not validate pch when -fno-validate-pch is set 
(authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D24054?vs=70569=70580#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24054

Files:
  cfe/trunk/include/clang/Serialization/ASTReader.h
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/test/PCH/no-validate-pch.cl

Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -496,12 +496,16 @@
 /// against the preprocessor options in an existing preprocessor.
 ///
 /// \param Diags If non-null, produce diagnostics for any mismatches incurred.
+/// \param Validate If true, validate preprocessor options. If false, allow
+///macros defined by \p ExistingPPOpts to override those defined by
+///\p PPOpts in SuggestedPredefines.
 static bool checkPreprocessorOptions(const PreprocessorOptions ,
  const PreprocessorOptions ,
  DiagnosticsEngine *Diags,
  FileManager ,
  std::string ,
- const LangOptions ) {
+ const LangOptions ,
+ bool Validate = true) {
   // Check macro definitions.
   MacroDefinitionsMap ASTFileMacros;
   collectMacroDefinitions(PPOpts, ASTFileMacros);
@@ -517,7 +521,7 @@
 // Check whether we know anything about this macro name or not.
 llvm::StringMap >::iterator Known
   = ASTFileMacros.find(MacroName);
-if (Known == ASTFileMacros.end()) {
+if (!Validate || Known == ASTFileMacros.end()) {
   // FIXME: Check whether this identifier was referenced anywhere in the
   // AST file. If so, we should reject the AST file. Unfortunately, this
   // information isn't in the control block. What shall we do about it?
@@ -560,16 +564,16 @@
   }
 
   // Check whether we're using predefines.
-  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines) {
+  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines && Validate) {
 if (Diags) {
   Diags->Report(diag::err_pch_undef) << ExistingPPOpts.UsePredefines;
 }
 return true;
   }
 
   // Detailed record is important since it is used for the module cache hash.
   if (LangOpts.Modules &&
-  PPOpts.DetailedRecord != ExistingPPOpts.DetailedRecord) {
+  PPOpts.DetailedRecord != ExistingPPOpts.DetailedRecord && Validate) {
 if (Diags) {
   Diags->Report(diag::err_pch_pp_detailed_record) << PPOpts.DetailedRecord;
 }
@@ -618,6 +622,19 @@
   PP.getLangOpts());
 }
 
+bool SimpleASTReaderListener::ReadPreprocessorOptions(
+  const PreprocessorOptions ,
+  bool Complain,
+  std::string ) {
+  return checkPreprocessorOptions(PPOpts,
+  PP.getPreprocessorOpts(),
+  nullptr,
+  PP.getFileManager(),
+  SuggestedPredefines,
+  PP.getLangOpts(),
+  false);
+}
+
 /// Check the header search options deserialized from the control block
 /// against the header search options in an existing preprocessor.
 ///
@@ -8710,7 +8727,10 @@
   bool AllowConfigurationMismatch, bool ValidateSystemInputs,
   bool UseGlobalIndex,
   std::unique_ptr ReadTimer)
-: Listener(new PCHValidator(PP, *this)), DeserializationListener(nullptr),
+: Listener(DisableValidation ?
+cast(new SimpleASTReaderListener(PP)) :
+cast(new PCHValidator(PP, *this))),
+  DeserializationListener(nullptr),
   OwnsDeserializationListener(false), SourceMgr(PP.getSourceManager()),
   FileMgr(PP.getFileManager()), PCHContainerRdr(PCHContainerRdr),
   Diags(PP.getDiagnostics()), SemaObj(nullptr), PP(PP), Context(Context),
Index: cfe/trunk/include/clang/Serialization/ASTReader.h
===
--- cfe/trunk/include/clang/Serialization/ASTReader.h
+++ cfe/trunk/include/clang/Serialization/ASTReader.h
@@ -284,6 +284,21 @@
   void Error(const char *Msg);
 };
 
+/// \brief ASTReaderListenter implementation to set SuggestedPredefines of
+/// ASTReader which is required to use a pch file. This is the replacement
+/// of PCHValidator or SimplePCHValidator when using a pch file without
+/// validating it.
+class SimpleASTReaderListener : public ASTReaderListener {
+  Preprocessor 
+
+public:
+  SimpleASTReaderListener(Preprocessor )
+: PP(PP) {}
+
+  bool 

r280842 - Do not validate pch when -fno-validate-pch is set

2016-09-07 Thread Yaxun Liu via cfe-commits
Author: yaxunl
Date: Wed Sep  7 13:40:20 2016
New Revision: 280842

URL: http://llvm.org/viewvc/llvm-project?rev=280842=rev
Log:
Do not validate pch when -fno-validate-pch is set

There is a bug causing pch to be validated even though -fno-validate-pch is 
set. This patch fixes it.

ASTReader relies on ASTReaderListener to initialize SuggestedPredefines, which 
is required for compilations using PCH. Before this change, PCHValidator is the 
default ASTReaderListener. After this change, when -fno-validate-pch is set, 
PCHValidator is disabled, but we need a replacement ASTReaderListener to 
initialize SuggestedPredefines. Class SimpleASTReaderListener is implemented 
for this purpose.

This change only affects -fno-validate-pch. There is no functional change if 
-fno-validate-pch is not set.

If -fno-validate-pch is not set, conflicts in predefined macros between pch and 
current compiler instance causes error.

If -fno-validate-pch is set, predefine macros in current compiler override 
those in pch so that compilation can continue.

Differential Revision: https://reviews.llvm.org/D24054

Added:
cfe/trunk/test/PCH/no-validate-pch.cl
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=280842=280841=280842=diff
==
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Sep  7 13:40:20 2016
@@ -284,6 +284,21 @@ private:
   void Error(const char *Msg);
 };
 
+/// \brief ASTReaderListenter implementation to set SuggestedPredefines of
+/// ASTReader which is required to use a pch file. This is the replacement
+/// of PCHValidator or SimplePCHValidator when using a pch file without
+/// validating it.
+class SimpleASTReaderListener : public ASTReaderListener {
+  Preprocessor 
+
+public:
+  SimpleASTReaderListener(Preprocessor )
+: PP(PP) {}
+
+  bool ReadPreprocessorOptions(const PreprocessorOptions , bool 
Complain,
+   std::string ) override;
+};
+
 namespace serialization {
 
 class ReadMethodPoolVisitor;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=280842=280841=280842=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Sep  7 13:40:20 2016
@@ -496,12 +496,16 @@ collectMacroDefinitions(const Preprocess
 /// against the preprocessor options in an existing preprocessor.
 ///
 /// \param Diags If non-null, produce diagnostics for any mismatches incurred.
+/// \param Validate If true, validate preprocessor options. If false, allow
+///macros defined by \p ExistingPPOpts to override those defined by
+///\p PPOpts in SuggestedPredefines.
 static bool checkPreprocessorOptions(const PreprocessorOptions ,
  const PreprocessorOptions ,
  DiagnosticsEngine *Diags,
  FileManager ,
  std::string ,
- const LangOptions ) {
+ const LangOptions ,
+ bool Validate = true) {
   // Check macro definitions.
   MacroDefinitionsMap ASTFileMacros;
   collectMacroDefinitions(PPOpts, ASTFileMacros);
@@ -517,7 +521,7 @@ static bool checkPreprocessorOptions(con
 // Check whether we know anything about this macro name or not.
 llvm::StringMap >::iterator Known
   = ASTFileMacros.find(MacroName);
-if (Known == ASTFileMacros.end()) {
+if (!Validate || Known == ASTFileMacros.end()) {
   // FIXME: Check whether this identifier was referenced anywhere in the
   // AST file. If so, we should reject the AST file. Unfortunately, this
   // information isn't in the control block. What shall we do about it?
@@ -560,7 +564,7 @@ static bool checkPreprocessorOptions(con
   }
 
   // Check whether we're using predefines.
-  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines) {
+  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines && Validate) {
 if (Diags) {
   Diags->Report(diag::err_pch_undef) << ExistingPPOpts.UsePredefines;
 }
@@ -569,7 +573,7 @@ static bool checkPreprocessorOptions(con
 
   // Detailed record is important since it is used for the module cache hash.
   if (LangOpts.Modules &&
-  PPOpts.DetailedRecord != ExistingPPOpts.DetailedRecord) {
+  PPOpts.DetailedRecord != ExistingPPOpts.DetailedRecord && Validate) {
 if (Diags) {
   

Re: [PATCH] D24054: Do not validate pch when -fno-validate-pch is set

2016-09-07 Thread Anastasia Stulova via cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D24054



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24054: Do not validate pch when -fno-validate-pch is set

2016-09-07 Thread Richard Smith via cfe-commits
rsmith accepted this revision.
rsmith added a comment.

It's pretty gross that we're computing `SuggestedPredefines` in the 
`ASTReaderListener`, but sure, this seems OK for now.


https://reviews.llvm.org/D24054



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

2016-09-07 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a subscriber: alexshap.


Comment at: change-namespace/ChangeNamespace.cpp:125
@@ +124,3 @@
+// applying all existing Replaces first if there is conflict.
+void addOrMergeReplacement(const tooling::Replacement ,
+   tooling::Replacements *Replaces) {

khm, this seems to be a recurring pattern (if i'm not mistaken),
looks like the interface of tooling::Replacements can be changed/improved 
(although i don't know).
Another (separate) observation is about duplicates. For example for 
replacements in headers we can get into if(Err) branch pretty frequently 
because the same replacement can be generated multiple times (if that header is 
included into several *.cpp files). 


https://reviews.llvm.org/D24183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

2016-09-07 Thread Zachary Turner via cfe-commits
Author: zturner
Date: Wed Sep  7 13:28:42 2016
New Revision: 280839

URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev
Log:
Resubmit "Add a test for clang-tidy using the clang-cl driver."

This was originally reverted because the patch on the clang
tooling side was reverted.  That patch is being resubmitted,
so this patch is resubmitted as well.

Added:
clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp

Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=280839=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Wed Sep  7 
13:28:42 2016
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s -- --driver-mode=cl 
/DTEST1 /DFOO=foo /DBAR=bar | FileCheck 
-implicit-check-not="{{warning|error}}:" %s
+int *a = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#ifdef TEST1
+int *b = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#endif
+#define foo 1
+#define bar 1
+#if FOO
+int *c = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#endif
+#if BAR
+int *d = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#endif


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24158: Try contextually converting condition of constexpr if to Boolean value

2016-09-07 Thread Ismail Pazarbasi via cfe-commits
ismailp added inline comments.


Comment at: test/CodeGenCXX/cxx1z-constexpr-if.cpp:2
@@ -1,2 +1,3 @@
 // RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s 
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -DCHECK_CONVERSION -verify %s
 

rsmith wrote:
> ismailp wrote:
> > Is there a more elegant way to do this? The previous check didn't seem to 
> > work with diagnostics.
> Tests for semantic issues should go into test/SemaCXX (organized by category) 
> or test/CXX (organized by section of the standard). In this case, 
> test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp is probably the most 
> appropriate place (somewhere in its `namespace ccce`, perhaps).
Thanks! I've done that, and pushed.


Repository:
  rL LLVM

https://reviews.llvm.org/D24158



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-09-07 Thread Zachary Turner via cfe-commits
The unnecessary usings are added by Visual Studio, so I would rather leave
them.  For example, they often provide extension methods to containers so
that containers can be used with functional paradigms, and you expect to
see those methods in the Intellisense window when you type "Foo.", but
without the using statement they would not appear.  In general they don't
affect performance or compilation the same way that #includes do in C++, so
for the ones that are added automatically by Visual Studio I would rather
leave them alone.  I've cleaned up all the unnecessary ones that were not
automatically added though.

On Wed, Sep 7, 2016 at 4:27 AM Alexander Kornienko 
wrote:

> alexfh added inline comments.
>
> 
> Comment at: clang-tidy-vs/ClangTidy/ClangTidyPackage.cs:53
> @@ +52,3 @@
> +private void MenuItemCallback(object sender, EventArgs args)
> +{
> +}
> 
> Add a FIXME to actually implement this.
>
>
> https://reviews.llvm.org/D23848
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24158: Try contextually converting condition of constexpr if to Boolean value

2016-09-07 Thread Ismail Pazarbasi via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280838: Try contextually converting condition of constexpr 
if to Boolean value (authored by ismailp).

Changed prior to commit:
  https://reviews.llvm.org/D24158?vs=70561=70577#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24158

Files:
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp

Index: cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -46,6 +46,11 @@
 if constexpr (N) {} // expected-error {{cannot be narrowed}}
   }
   template void g<5>(); // expected-note {{instantiation of}}
+  void h() {
+if constexpr (4.3) {} // expected-error{{conversion from 'double' to 
'bool' is not allowed in a converted constant expression}}
+constexpr void *p = nullptr;
+if constexpr (p) {} // expected-error{{conversion from 'void *const' to 
'bool' is not allowed in a converted constant expression}}
+  }
 }
 
 namespace generic_lambda {
Index: cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -2,7 +2,15 @@
 
 void should_be_used_1();
 void should_be_used_2();
+void should_be_used_3();
 void should_not_be_used();
+
+struct A {
+  constexpr explicit operator bool() const {
+return true;
+  }
+};
+
 void f() {
   if constexpr (false)
 should_not_be_used();
@@ -15,7 +23,12 @@
 goto foo;
 foo: should_not_be_used();
   }
+  if constexpr (A())
+should_be_used_3();
+  else
+should_not_be_used();
 }
 
 // CHECK: should_be_used_1
 // CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -5164,12 +5164,18 @@
   //  implicitly converted to type T, where the converted
   //  expression is a constant expression and the implicit conversion
   //  sequence contains only [... list of conversions ...].
+  // C++1z [stmt.if]p2:
+  //  If the if statement is of the form if constexpr, the value of the
+  //  condition shall be a contextually converted constant expression of type
+  //  bool.
   ImplicitConversionSequence ICS =
-TryCopyInitialization(S, From, T,
-  /*SuppressUserConversions=*/false,
-  /*InOverloadResolution=*/false,
-  /*AllowObjcWritebackConversion=*/false,
-  /*AllowExplicit=*/false);
+  CCE == Sema::CCEK_ConstexprIf
+  ? TryContextuallyConvertToBool(S, From)
+  : TryCopyInitialization(S, From, T,
+  /*SuppressUserConversions=*/false,
+  /*InOverloadResolution=*/false,
+  /*AllowObjcWritebackConversion=*/false,
+  /*AllowExplicit=*/false);
   StandardConversionSequence *SCS = nullptr;
   switch (ICS.getKind()) {
   case ImplicitConversionSequence::StandardConversion:


Index: cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -46,6 +46,11 @@
 if constexpr (N) {} // expected-error {{cannot be narrowed}}
   }
   template void g<5>(); // expected-note {{instantiation of}}
+  void h() {
+if constexpr (4.3) {} // expected-error{{conversion from 'double' to 'bool' is not allowed in a converted constant expression}}
+constexpr void *p = nullptr;
+if constexpr (p) {} // expected-error{{conversion from 'void *const' to 'bool' is not allowed in a converted constant expression}}
+  }
 }
 
 namespace generic_lambda {
Index: cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -2,7 +2,15 @@
 
 void should_be_used_1();
 void should_be_used_2();
+void should_be_used_3();
 void should_not_be_used();
+
+struct A {
+  constexpr explicit operator bool() const {
+return true;
+  }
+};
+
 void f() {
   if constexpr (false)
 should_not_be_used();
@@ -15,7 +23,12 @@
 goto foo;
 foo: should_not_be_used();
   }
+  if constexpr (A())
+should_be_used_3();
+  else
+should_not_be_used();
 }
 
 // CHECK: should_be_used_1
 // CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: cfe/trunk/lib/Sema/SemaOverload.cpp

r280838 - Try contextually converting condition of constexpr if to Boolean value

2016-09-07 Thread Ismail Pazarbasi via cfe-commits
Author: ismailp
Date: Wed Sep  7 13:24:54 2016
New Revision: 280838

URL: http://llvm.org/viewvc/llvm-project?rev=280838=rev
Log:
Try contextually converting condition of constexpr if to Boolean value

Summary:
C++1z 6.4.1/p2:
 If the if statement is of the form if constexpr, the value of the
 condition shall be a contextually converted constant expression of type
 bool [...]
C++1z 5.20/p4:
 [...] A contextually converted constant expression of type bool is an
 expression, contextually converted to bool (Clause4), where the
 converted expression is a constant expression and the conversion
 sequence contains only the conversions above. [...]

Contextually converting result of an expression `e` to a Boolean value
requires `bool t(e)` to be well-formed.

An explicit conversion function is only considered as a user-defined
conversion for direct-initialization, which is essentially what
//contextually converted to bool// requires.

Also, fixes PR28470.

Reviewers: rsmith

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D24158

Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=280838=280837=280838=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Sep  7 13:24:54 2016
@@ -5164,12 +5164,18 @@ static ExprResult CheckConvertedConstant
   //  implicitly converted to type T, where the converted
   //  expression is a constant expression and the implicit conversion
   //  sequence contains only [... list of conversions ...].
+  // C++1z [stmt.if]p2:
+  //  If the if statement is of the form if constexpr, the value of the
+  //  condition shall be a contextually converted constant expression of type
+  //  bool.
   ImplicitConversionSequence ICS =
-TryCopyInitialization(S, From, T,
-  /*SuppressUserConversions=*/false,
-  /*InOverloadResolution=*/false,
-  /*AllowObjcWritebackConversion=*/false,
-  /*AllowExplicit=*/false);
+  CCE == Sema::CCEK_ConstexprIf
+  ? TryContextuallyConvertToBool(S, From)
+  : TryCopyInitialization(S, From, T,
+  /*SuppressUserConversions=*/false,
+  /*InOverloadResolution=*/false,
+  /*AllowObjcWritebackConversion=*/false,
+  /*AllowExplicit=*/false);
   StandardConversionSequence *SCS = nullptr;
   switch (ICS.getKind()) {
   case ImplicitConversionSequence::StandardConversion:

Modified: cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp?rev=280838=280837=280838=diff
==
--- cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp (original)
+++ cfe/trunk/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp Wed Sep  7 13:24:54 
2016
@@ -46,6 +46,11 @@ namespace ccce {
 if constexpr (N) {} // expected-error {{cannot be narrowed}}
   }
   template void g<5>(); // expected-note {{instantiation of}}
+  void h() {
+if constexpr (4.3) {} // expected-error{{conversion from 'double' to 
'bool' is not allowed in a converted constant expression}}
+constexpr void *p = nullptr;
+if constexpr (p) {} // expected-error{{conversion from 'void *const' to 
'bool' is not allowed in a converted constant expression}}
+  }
 }
 
 namespace generic_lambda {

Modified: cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp?rev=280838=280837=280838=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx1z-constexpr-if.cpp Wed Sep  7 13:24:54 2016
@@ -2,7 +2,15 @@
 
 void should_be_used_1();
 void should_be_used_2();
+void should_be_used_3();
 void should_not_be_used();
+
+struct A {
+  constexpr explicit operator bool() const {
+return true;
+  }
+};
+
 void f() {
   if constexpr (false)
 should_not_be_used();
@@ -15,7 +23,12 @@ void f() {
 goto foo;
 foo: should_not_be_used();
   }
+  if constexpr (A())
+should_be_used_3();
+  else
+should_not_be_used();
 }
 
 // CHECK: should_be_used_1
 // CHECK: should_be_used_2
+// CHECK: should_be_used_3


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280836 - [MS] Fix prologue this adjustment when 'this' is passed indirectly

2016-09-07 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Wed Sep  7 13:21:30 2016
New Revision: 280836

URL: http://llvm.org/viewvc/llvm-project?rev=280836=rev
Log:
[MS] Fix prologue this adjustment when 'this' is passed indirectly

Move the logic for doing this from the ABI argument lowering into
EmitParmDecl, which runs for all parameters. Our codegen is slightly
suboptimal in this case, as we may leave behind a dead store after
optimization, but it's 32-bit inalloca, and this fixes the bug in a
robust way.

Fixes PR30293

Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGenCXX/constructor-destructor-return-this.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp

cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-this-adjustment.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=280836=280835=280836=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Sep  7 13:21:30 2016
@@ -2309,13 +2309,6 @@ void CodeGenFunction::EmitFunctionProlog
 if (isPromoted)
   V = emitArgumentDemotion(*this, Arg, V);
 
-if (const CXXMethodDecl *MD =
-dyn_cast_or_null(CurCodeDecl)) {
-  if (MD->isVirtual() && Arg == CXXABIThisDecl)
-V = CGM.getCXXABI().
-adjustThisParameterInVirtualFunctionPrologue(*this, CurGD, V);
-}
-
 // Because of merging of function types from multiple decls it is
 // possible for the type of an argument to not match the corresponding
 // type in the function type. Since we are codegening the callee

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=280836=280835=280836=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Sep  7 13:21:30 2016
@@ -13,6 +13,7 @@
 
 #include "CodeGenFunction.h"
 #include "CGBlocks.h"
+#include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGDebugInfo.h"
 #include "CGOpenCLRuntime.h"
@@ -1769,6 +1770,24 @@ void CodeGenFunction::EmitParmDecl(const
   setBlockContextParameter(IPD, ArgNo, Arg.getDirectValue());
   return;
 }
+
+// Apply any prologue 'this' adjustments required by the ABI. Be careful to
+// handle the case where 'this' is passed indirectly as part of an inalloca
+// struct.
+if (const CXXMethodDecl *MD =
+dyn_cast_or_null(CurCodeDecl)) {
+  if (MD->isVirtual() && IPD == CXXABIThisDecl) {
+llvm::Value *This = Arg.isIndirect()
+? Builder.CreateLoad(Arg.getIndirectAddress())
+: Arg.getDirectValue();
+This = CGM.getCXXABI().adjustThisParameterInVirtualFunctionPrologue(
+*this, CurGD, This);
+if (Arg.isIndirect())
+  Builder.CreateStore(This, Arg.getIndirectAddress());
+else
+  Arg = ParamValue::forDirect(This);
+  }
+}
   }
 
   Address DeclPtr = Address::invalid();

Modified: cfe/trunk/test/CodeGenCXX/constructor-destructor-return-this.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/constructor-destructor-return-this.cpp?rev=280836=280835=280836=diff
==
--- cfe/trunk/test/CodeGenCXX/constructor-destructor-return-this.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/constructor-destructor-return-this.cpp Wed Sep  7 
13:21:30 2016
@@ -111,7 +111,7 @@ D::~D() { }
 // CHECKIOS5-LABEL: define %class.D* @_ZN1DD1Ev(%class.D* %this)
 
 // CHECKMS-LABEL: define x86_thiscallcc %class.D* @"\01??0D@@QAE@XZ"(%class.D* 
returned %this, i32 %is_most_derived)
-// CHECKMS-LABEL: define x86_thiscallcc void @"\01??1D@@UAE@XZ"(%class.D*)
+// CHECKMS-LABEL: define x86_thiscallcc void @"\01??1D@@UAE@XZ"(%class.D* 
%this)
 
 class E {
 public:

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp?rev=280836=280835=280836=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp Wed Sep  7 13:21:30 
2016
@@ -196,7 +196,7 @@ struct E : virtual C { int e; };
 struct F : D, E { ~F(); int f; };
 
 F::~F() {
-// CHECK-LABEL: define x86_thiscallcc void 
@"\01??1F@test2@@UAE@XZ"(%"struct.test2::F"*)
+// CHECK-LABEL: define x86_thiscallcc void 
@"\01??1F@test2@@UAE@XZ"(%"struct.test2::F"*{{[^,]*}})
 //  Do an adjustment from C vbase subobject to F as though 

Re: [PATCH] D24153: Add bunch of _Interlocked builtins

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski marked an inline comment as done.
agutowski added a comment.

https://reviews.llvm.org/D24153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24153: Add bunch of _Interlocked builtins

2016-09-07 Thread Albert Gutowski via cfe-commits
agutowski updated this revision to Diff 70572.
agutowski added a comment.

Mark _InterlockedIncrement and _InterlockedDecrement as non-volatile


https://reviews.llvm.org/D24153

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics.c
  test/CodeGen/pr27892.c

Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -602,177 +602,6 @@
 }
 #endif
 /**\
-|* Interlocked Exchange Add
-\**/
-static __inline__ char __DEFAULT_FN_ATTRS
-_InterlockedExchangeAdd8(char volatile *_Addend, char _Value) {
-  return __atomic_fetch_add(_Addend, _Value, __ATOMIC_SEQ_CST);
-}
-static __inline__ short __DEFAULT_FN_ATTRS
-_InterlockedExchangeAdd16(short volatile *_Addend, short _Value) {
-  return __atomic_fetch_add(_Addend, _Value, __ATOMIC_SEQ_CST);
-}
-#ifdef __x86_64__
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedExchangeAdd64(__int64 volatile *_Addend, __int64 _Value) {
-  return __atomic_fetch_add(_Addend, _Value, __ATOMIC_SEQ_CST);
-}
-#endif
-/**\
-|* Interlocked Exchange Sub
-\**/
-static __inline__ char __DEFAULT_FN_ATTRS
-_InterlockedExchangeSub8(char volatile *_Subend, char _Value) {
-  return __atomic_fetch_sub(_Subend, _Value, __ATOMIC_SEQ_CST);
-}
-static __inline__ short __DEFAULT_FN_ATTRS
-_InterlockedExchangeSub16(short volatile *_Subend, short _Value) {
-  return __atomic_fetch_sub(_Subend, _Value, __ATOMIC_SEQ_CST);
-}
-static __inline__ long __DEFAULT_FN_ATTRS
-_InterlockedExchangeSub(long volatile *_Subend, long _Value) {
-  return __atomic_fetch_sub(_Subend, _Value, __ATOMIC_SEQ_CST);
-}
-#ifdef __x86_64__
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedExchangeSub64(__int64 volatile *_Subend, __int64 _Value) {
-  return __atomic_fetch_sub(_Subend, _Value, __ATOMIC_SEQ_CST);
-}
-#endif
-/**\
-|* Interlocked Increment
-\**/
-static __inline__ short __DEFAULT_FN_ATTRS
-_InterlockedIncrement16(short volatile *_Value) {
-  return __atomic_add_fetch(_Value, 1, __ATOMIC_SEQ_CST);
-}
-#ifdef __x86_64__
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedIncrement64(__int64 volatile *_Value) {
-  return __atomic_add_fetch(_Value, 1, __ATOMIC_SEQ_CST);
-}
-#endif
-/**\
-|* Interlocked Decrement
-\**/
-static __inline__ short __DEFAULT_FN_ATTRS
-_InterlockedDecrement16(short volatile *_Value) {
-  return __atomic_sub_fetch(_Value, 1, __ATOMIC_SEQ_CST);
-}
-#ifdef __x86_64__
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedDecrement64(__int64 volatile *_Value) {
-  return __atomic_sub_fetch(_Value, 1, __ATOMIC_SEQ_CST);
-}
-#endif
-/**\
-|* Interlocked And
-\**/
-static __inline__ char __DEFAULT_FN_ATTRS
-_InterlockedAnd8(char volatile *_Value, char _Mask) {
-  return __atomic_fetch_and(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-static __inline__ short __DEFAULT_FN_ATTRS
-_InterlockedAnd16(short volatile *_Value, short _Mask) {
-  return __atomic_fetch_and(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-static __inline__ long __DEFAULT_FN_ATTRS
-_InterlockedAnd(long volatile *_Value, long _Mask) {
-  return __atomic_fetch_and(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-#ifdef __x86_64__
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedAnd64(__int64 volatile *_Value, __int64 _Mask) {
-  return __atomic_fetch_and(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-#endif
-/**\
-|* Interlocked Or
-\**/
-static __inline__ char __DEFAULT_FN_ATTRS
-_InterlockedOr8(char volatile *_Value, char _Mask) {
-  return __atomic_fetch_or(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-static __inline__ short __DEFAULT_FN_ATTRS
-_InterlockedOr16(short volatile *_Value, short _Mask) {
-  return __atomic_fetch_or(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-static __inline__ long __DEFAULT_FN_ATTRS
-_InterlockedOr(long volatile *_Value, long _Mask) {
-  return __atomic_fetch_or(_Value, _Mask, __ATOMIC_SEQ_CST);
-}
-#ifdef __x86_64__
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedOr64(__int64 volatile *_Value, __int64 _Mask) {
-  return __atomic_fetch_or(_Value, _Mask, 

Re: [PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

2016-09-07 Thread Eric Liu via cfe-commits
ioeric added inline comments.


Comment at: change-namespace/ChangeNamespace.cpp:21
@@ +20,3 @@
+inline std::string formatNamespace(llvm::StringRef NS) {
+  (void)NS.ltrim(':');
+  return NS.str();

hokein wrote:
> this statement doesn't do the intended thing (It won't change `NS`). The 
> result returned by `ltrim` is what you want here, I think.
Aha,  no wonder! Thanks!


Comment at: change-namespace/ChangeNamespace.cpp:480
@@ +479,3 @@
+Replaces = Replaces.merge(NewReplacements);
+format::FormatStyle Style = format::getStyle("file", FilePath, "google");
+// Clean up old namespaces if there is nothing in it after moving.

hokein wrote:
> omtcyfz wrote:
> > omtcyfz wrote:
> > > By the way, shouldn't this one be "LLVM"?
> > Alternatively, it might be nice to provide an option to specify desired 
> > formatting style.
> +1 on adding a `CodeStyle` command-line option.
Will do.


Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:49
@@ +48,3 @@
+formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite);
+return format(Context.getRewrittenText(ID));
+  }

hokein wrote:
> Looks like `formatAndApplyAllReplacements` has already formatted the code, 
> why do we need to format it again?
`formatAndApplyAllReplacements ` only formats around replacements. I added 
`format` to format the whole file so that I don't need to get every white  
space right in `Code` and `Expected`.


https://reviews.llvm.org/D24183



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24054: Do not validate pch when -fno-validate-pch is set

2016-09-07 Thread Yaxun Liu via cfe-commits
yaxunl updated the summary for this revision.
yaxunl updated this revision to Diff 70569.
yaxunl added a comment.
Herald added a subscriber: wdng.

Implemented a simple AST reader listener to replace PCHValidator. Added a test.


https://reviews.llvm.org/D24054

Files:
  include/clang/Serialization/ASTReader.h
  lib/Serialization/ASTReader.cpp
  test/PCH/no-validate-pch.cl

Index: test/PCH/no-validate-pch.cl
===
--- /dev/null
+++ test/PCH/no-validate-pch.cl
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -emit-pch -D TWO=2 -D X=4 -o %t %s -triple spir-unknown-unknown
+// RUN: %clang_cc1 -include-pch %t -D THREE=3 -D X=5 -O0 -U__OPTIMIZE__ -fno-validate-pch %s -triple spir-unknown-unknown 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -include-pch %t -D THREE=3 -D X=5 -D VALIDATE -O0 -fsyntax-only %s -triple spir-unknown-unknown 2>&1 | FileCheck --check-prefix=CHECK-VAL %s
+
+#ifndef HEADER
+#define HEADER
+// Header.
+
+#define ONE 1
+
+#else
+// Using the header.
+
+// CHECK: warning: 'X' macro redefined
+// CHECK: #define X 5
+// CHECK: note: previous definition is here
+// CHECK: #define X 4
+
+// CHECK-VAL: error: __OPTIMIZE__ predefined macro was enabled in PCH file but is currently disabled
+// CHECK-VAL: error: definition of macro 'X' differs between the precompiled header ('4') and the command line ('5')
+
+void test(void) {
+  int a = ONE;
+  int b = TWO;
+  int c = THREE;
+
+#ifndef VALIDATE
+#if X != 5
+#error Definition of X is not overridden!
+#endif
+
+#ifdef __OPTIMIZE__
+#error Optimization is not off!
+#endif
+#endif
+
+}
+
+#endif
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -496,12 +496,16 @@
 /// against the preprocessor options in an existing preprocessor.
 ///
 /// \param Diags If non-null, produce diagnostics for any mismatches incurred.
+/// \param Validate If true, validate preprocessor options. If false, allow
+///macros defined by \p ExistingPPOpts to override those defined by
+///\p PPOpts in SuggestedPredefines.
 static bool checkPreprocessorOptions(const PreprocessorOptions ,
  const PreprocessorOptions ,
  DiagnosticsEngine *Diags,
  FileManager ,
  std::string ,
- const LangOptions ) {
+ const LangOptions ,
+ bool Validate = true) {
   // Check macro definitions.
   MacroDefinitionsMap ASTFileMacros;
   collectMacroDefinitions(PPOpts, ASTFileMacros);
@@ -517,7 +521,7 @@
 // Check whether we know anything about this macro name or not.
 llvm::StringMap >::iterator Known
   = ASTFileMacros.find(MacroName);
-if (Known == ASTFileMacros.end()) {
+if (!Validate || Known == ASTFileMacros.end()) {
   // FIXME: Check whether this identifier was referenced anywhere in the
   // AST file. If so, we should reject the AST file. Unfortunately, this
   // information isn't in the control block. What shall we do about it?
@@ -560,16 +564,16 @@
   }
 
   // Check whether we're using predefines.
-  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines) {
+  if (PPOpts.UsePredefines != ExistingPPOpts.UsePredefines && Validate) {
 if (Diags) {
   Diags->Report(diag::err_pch_undef) << ExistingPPOpts.UsePredefines;
 }
 return true;
   }
 
   // Detailed record is important since it is used for the module cache hash.
   if (LangOpts.Modules &&
-  PPOpts.DetailedRecord != ExistingPPOpts.DetailedRecord) {
+  PPOpts.DetailedRecord != ExistingPPOpts.DetailedRecord && Validate) {
 if (Diags) {
   Diags->Report(diag::err_pch_pp_detailed_record) << PPOpts.DetailedRecord;
 }
@@ -618,6 +622,19 @@
   PP.getLangOpts());
 }
 
+bool SimpleASTReaderListener::ReadPreprocessorOptions(
+  const PreprocessorOptions ,
+  bool Complain,
+  std::string ) {
+  return checkPreprocessorOptions(PPOpts,
+  PP.getPreprocessorOpts(),
+  nullptr,
+  PP.getFileManager(),
+  SuggestedPredefines,
+  PP.getLangOpts(),
+  false);
+}
+
 /// Check the header search options deserialized from the control block
 /// against the header search options in an existing preprocessor.
 ///
@@ -8710,7 +8727,10 @@
   bool AllowConfigurationMismatch, bool ValidateSystemInputs,
   bool UseGlobalIndex,
   std::unique_ptr ReadTimer)
-: Listener(new PCHValidator(PP, *this)), 

Re: [PATCH] D24113: Allow implicit conversions between incompatible pointer types in overload resolution in C.

2016-09-07 Thread Richard Smith via cfe-commits
On Tue, Sep 6, 2016 at 10:55 PM, George Burgess IV <
george.burgess...@gmail.com> wrote:

> george.burgess.iv added a comment.
>
> > Although I think that users will expect atomic_add(unsigned int *, 5) to
> work.without error
>
>
> Totally agree; I was just making sure that I understood the problem. :)
>
> I think, with a small tweak in semantics, we can make everything work
> without needing to patch existing code. Let me play around a little.


It seems to me that we could handle this by ranking viable-but-ill-formed
conversion sequences before non-viable ones in
clang::isBetterOverloadCandidate. This problem is actually more general
than __attribute__((overloadable)) in C -- there is one form of ill-formed
implicit conversion sequence that we accept in C++ that leads to the same
problem:

  void f(char*, int);
  void f(const char*, unsigned);
  void g() { f("foo", 0); }

Clang reports an ambiguity on line 3, but in C++11 onwards this code is
valid because there is no implicit conversion sequence from a string
literal to char*. (We provide one as an extension, and it's ranked worse
than any other conversion.)


> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24113
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

2016-09-07 Thread Daniel Krupp via cfe-commits
dkrupp created this revision.
dkrupp added reviewers: xazax.hun, NoQ, dcoughlin, zaks.anna.
dkrupp added a subscriber: cfe-commits.

ArrayBoundChecker did not detect out of bounds memory access errors in case an 
array was allocated by the new expression.

1.  MallocChecker.cpp was updated to calculate the extent size in Bytes 
similarly how it was done for memory regions allocated by malloc. The size 
constraint is added for arrays and non-arrays allocated by new.

2.  ArrayBoundCheckerV2.cpp was updated to better handle accessing locations 
preceding a symbolic memory region (such as buf[-1] in test2(..) in 
out-of-bounds.cpp). So computeExtentBegin(..) was updated to assume that the 
extent of a symbolic region starts at 0 if we know the size of the extent (as 
is the case in case of malloc or new).

3. out-of-bounds.cpp contains the relevant test cases for C++.

https://reviews.llvm.org/D24307

Files:
  lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/out-of-bounds.cpp

Index: test/Analysis/out-of-bounds.cpp
===
--- /dev/null
+++ test/Analysis/out-of-bounds.cpp
@@ -0,0 +1,147 @@
+// RUN: %clang_cc1 -std=c++11 -Wno-array-bounds -analyze -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s
+
+// Tests doing an out-of-bounds access after the end of an array using:
+// - constant integer index
+// - constant integer size for buffer
+void test1(int x) {
+  int *buf = new int[100];
+  buf[100] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ok(int x) {
+  int *buf = new int[100];
+  buf[99] = 1; // no-warning
+}
+
+// Tests doing an out-of-bounds access after the end of an array using:
+// - indirect pointer to buffer
+// - constant integer index
+// - constant integer size for buffer
+void test1_ptr(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p[101] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ptr_ok(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p[99] = 1; // no-warning
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - indirect pointer to buffer, manipulated using simple pointer arithmetic
+// - constant integer index
+// - constant integer size for buffer
+void test1_ptr_arith(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 100;
+  p[0] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ptr_arith_ok(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 99;
+  p[0] = 1; // no-warning
+}
+
+void test1_ptr_arith_bad(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 99;
+  p[1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+void test1_ptr_arith_ok2(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p = p + 99;
+  p[-1] = 1; // no-warning
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - constant integer index
+// - constant integer size for buffer
+void test2(int x) {
+  int *buf = new int[100];
+  buf[-1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - indirect pointer to buffer
+// - constant integer index
+// - constant integer size for buffer
+void test2_ptr(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  p[-1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of an array using:
+// - indirect pointer to buffer, manipulated using simple pointer arithmetic
+// - constant integer index
+// - constant integer size for buffer
+void test2_ptr_arith(int x) {
+  int *buf = new int[100];
+  int *p = buf;
+  --p;
+  p[0] = 1; // expected-warning {{Out of bound memory access (accessed memory precedes memory block)}}
+}
+
+// Tests doing an out-of-bounds access before the start of a multi-dimensional
+// array using:
+// - constant integer indices
+// - constant integer sizes for the array
+void test2_multi(int x) {
+  auto buf = new int[100][100];
+  buf[0][-1] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of a multi-dimensional
+// array using:
+// - constant integer indices
+// - constant integer sizes for the array
+void test2_multi_b(int x) {
+  auto buf = new int[100][100];
+  buf[-1][0] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of a multi-dimensional
+// array using:
+// - constant integer indices
+// - constant integer sizes for the array
+void test2_multi_c(int x) {
+  auto buf = new int[100][100];
+  buf[100][0] = 1; // expected-warning{{Out of bound memory access}}
+}
+
+// Tests doing an out-of-bounds access before the start of a multi-dimensional
+// array using:
+// - constant integer indices
+// - constant integer sizes 

Re: [PATCH] D24158: Try contextually converting condition of constexpr if to Boolean value

2016-09-07 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: test/CodeGenCXX/cxx1z-constexpr-if.cpp:2
@@ -1,2 +1,3 @@
 // RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s 
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -DCHECK_CONVERSION -verify %s
 

ismailp wrote:
> Is there a more elegant way to do this? The previous check didn't seem to 
> work with diagnostics.
Tests for semantic issues should go into test/SemaCXX (organized by category) 
or test/CXX (organized by section of the standard). In this case, 
test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp is probably the most appropriate 
place (somewhere in its `namespace ccce`, perhaps).


https://reviews.llvm.org/D24158



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280828 - [clang-offload-bundler] Fix some Clang-tidy modernize-use-override and Include What You Use warnings; other minor fixes.

2016-09-07 Thread Eugene Zelenko via cfe-commits
Author: eugenezelenko
Date: Wed Sep  7 12:37:28 2016
New Revision: 280828

URL: http://llvm.org/viewvc/llvm-project?rev=280828=rev
Log:
[clang-offload-bundler] Fix some Clang-tidy modernize-use-override and Include 
What You Use warnings; other minor fixes.

Differential revision: https://reviews.llvm.org/D24165


Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=280828=280827=280828=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Wed Sep  7 
12:37:28 2016
@@ -1,4 +1,4 @@
-//===-- clang-offload-bundler/ClangOffloadBundler.cpp - Clang format tool 
-===//
+//===-- clang-offload-bundler/ClangOffloadBundler.cpp 
-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -15,22 +15,39 @@
 ///
 
//===--===//
 
-#include "clang/Basic/FileManager.h"
 #include "clang/Basic/Version.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Object/Binary.h"
-#include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Signals.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace llvm::object;
@@ -116,15 +133,22 @@ static bool hasHostKind(StringRef Target
 /// Generic file handler interface.
 class FileHandler {
 public:
+  FileHandler() {}
+
+  virtual ~FileHandler() {}
+
   /// Update the file handler with information from the header of the bundled
   /// file
   virtual void ReadHeader(MemoryBuffer ) = 0;
+
   /// Read the marker of the next bundled to be read in the file. The triple of
   /// the target associated with that bundle is returned. An empty string is
   /// returned if there are no more bundles to be read.
   virtual StringRef ReadBundleStart(MemoryBuffer ) = 0;
+
   /// Read the marker that closes the current bundle.
   virtual void ReadBundleEnd(MemoryBuffer ) = 0;
+
   /// Read the current bundle and write the result into the stream \a OS.
   virtual void ReadBundle(raw_fd_ostream , MemoryBuffer ) = 0;
 
@@ -132,17 +156,18 @@ public:
   /// gathered from \a Inputs.
   virtual void WriteHeader(raw_fd_ostream ,
ArrayRef Inputs) = 0;
+
   /// Write the marker that initiates a bundle for the triple \a TargetTriple 
to
   /// \a OS.
   virtual void WriteBundleStart(raw_fd_ostream , StringRef TargetTriple) = 
0;
+
   /// Write the marker that closes a bundle for the triple \a TargetTriple to 
\a
   /// OS. Return true if any error was found.
+
   virtual bool WriteBundleEnd(raw_fd_ostream , StringRef TargetTriple) = 0;
+
   /// Write the bundle from \a Input into \a OS.
   virtual void WriteBundle(raw_fd_ostream , MemoryBuffer ) = 0;
-
-  FileHandler() {}
-  virtual ~FileHandler() {}
 };
 
 /// Handler for binary files. The bundled file will have the following format
@@ -183,7 +208,6 @@ static uint64_t Read8byteIntegerFromBuff
 
 /// Write 8-byte integers to a buffer in little-endian format.
 static void Write8byteIntegerToBuffer(raw_fd_ostream , uint64_t Val) {
-
   for (unsigned i = 0; i < 8; ++i) {
 char Char = (char)(Val & 0xffu);
 OS.write(, 1);
@@ -198,9 +222,11 @@ class BinaryFileHandler final : public F
 uint64_t Size = 0u;
 /// Offset at which the bundle starts in the bundled file.
 uint64_t Offset = 0u;
+
 BundleInfo() {}
 BundleInfo(uint64_t Size, uint64_t Offset) : Size(Size), Offset(Offset) {}
   };
+
   /// Map between a triple and the corresponding bundle information.
   StringMap BundlesInfo;
 
@@ -208,7 +234,11 @@ class BinaryFileHandler final : public F
   StringMap::iterator CurBundleInfo;
 
 public:
-  void ReadHeader(MemoryBuffer ) {
+  BinaryFileHandler() : FileHandler() {}
+
+  ~BinaryFileHandler() final {}
+
+  void ReadHeader(MemoryBuffer ) final {
 StringRef FC = Input.getBuffer();
 
 // Initialize the current bundle with the 

Re: [PATCH] D24165: [clang-offload-bundler] Fix some Clang-tidy modernize-use-override and Include What You Use warnings; other minor fixes

2016-09-07 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280828: [clang-offload-bundler] Fix some Clang-tidy 
modernize-use-override and… (authored by eugenezelenko).

Changed prior to commit:
  https://reviews.llvm.org/D24165?vs=70084=70564#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24165

Files:
  cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -1,4 +1,4 @@
-//===-- clang-offload-bundler/ClangOffloadBundler.cpp - Clang format tool -===//
+//===-- clang-offload-bundler/ClangOffloadBundler.cpp -===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -15,22 +15,39 @@
 ///
 //===--===//
 
-#include "clang/Basic/FileManager.h"
 #include "clang/Basic/Version.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Object/Binary.h"
-#include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Signals.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 using namespace llvm;
 using namespace llvm::object;
@@ -116,33 +133,41 @@
 /// Generic file handler interface.
 class FileHandler {
 public:
+  FileHandler() {}
+
+  virtual ~FileHandler() {}
+
   /// Update the file handler with information from the header of the bundled
   /// file
   virtual void ReadHeader(MemoryBuffer ) = 0;
+
   /// Read the marker of the next bundled to be read in the file. The triple of
   /// the target associated with that bundle is returned. An empty string is
   /// returned if there are no more bundles to be read.
   virtual StringRef ReadBundleStart(MemoryBuffer ) = 0;
+
   /// Read the marker that closes the current bundle.
   virtual void ReadBundleEnd(MemoryBuffer ) = 0;
+
   /// Read the current bundle and write the result into the stream \a OS.
   virtual void ReadBundle(raw_fd_ostream , MemoryBuffer ) = 0;
 
   /// Write the header of the bundled file to \a OS based on the information
   /// gathered from \a Inputs.
   virtual void WriteHeader(raw_fd_ostream ,
ArrayRef Inputs) = 0;
+
   /// Write the marker that initiates a bundle for the triple \a TargetTriple to
   /// \a OS.
   virtual void WriteBundleStart(raw_fd_ostream , StringRef TargetTriple) = 0;
+
   /// Write the marker that closes a bundle for the triple \a TargetTriple to \a
   /// OS. Return true if any error was found.
+
   virtual bool WriteBundleEnd(raw_fd_ostream , StringRef TargetTriple) = 0;
+
   /// Write the bundle from \a Input into \a OS.
   virtual void WriteBundle(raw_fd_ostream , MemoryBuffer ) = 0;
-
-  FileHandler() {}
-  virtual ~FileHandler() {}
 };
 
 /// Handler for binary files. The bundled file will have the following format
@@ -183,7 +208,6 @@
 
 /// Write 8-byte integers to a buffer in little-endian format.
 static void Write8byteIntegerToBuffer(raw_fd_ostream , uint64_t Val) {
-
   for (unsigned i = 0; i < 8; ++i) {
 char Char = (char)(Val & 0xffu);
 OS.write(, 1);
@@ -198,17 +222,23 @@
 uint64_t Size = 0u;
 /// Offset at which the bundle starts in the bundled file.
 uint64_t Offset = 0u;
+
 BundleInfo() {}
 BundleInfo(uint64_t Size, uint64_t Offset) : Size(Size), Offset(Offset) {}
   };
+
   /// Map between a triple and the corresponding bundle information.
   StringMap BundlesInfo;
 
   /// Iterator for the bundle information that is being read.
   StringMap::iterator CurBundleInfo;
 
 public:
-  void ReadHeader(MemoryBuffer ) {
+  BinaryFileHandler() : FileHandler() {}
+
+  ~BinaryFileHandler() final {}
+
+  void ReadHeader(MemoryBuffer ) final {
 StringRef FC = Input.getBuffer();
 
 // Initialize the current bundle with the end of the container.
@@ -273,25 +303,28 @@
 // Set the iterator to where we will start to read.
 CurBundleInfo = BundlesInfo.begin();
   }
-  StringRef ReadBundleStart(MemoryBuffer ) {
+
+  StringRef ReadBundleStart(MemoryBuffer ) final {
 

Re: [PATCH] D24158: Try contextually converting condition of constexpr if to Boolean value

2016-09-07 Thread Ismail Pazarbasi via cfe-commits
ismailp added a comment.

Thank you for reviewing.

Sure, I'll try to look into those conversions as well.



Comment at: test/CodeGenCXX/cxx1z-constexpr-if.cpp:2
@@ -1,2 +1,3 @@
 // RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s 
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -DCHECK_CONVERSION -verify %s
 

Is there a more elegant way to do this? The previous check didn't seem to work 
with diagnostics.


https://reviews.llvm.org/D24158



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280827 - Add missing include. White space.

2016-09-07 Thread Vassil Vassilev via cfe-commits
Author: vvassilev
Date: Wed Sep  7 12:30:50 2016
New Revision: 280827

URL: http://llvm.org/viewvc/llvm-project?rev=280827=rev
Log:
Add missing  include. White space.

Modified:
cfe/trunk/include/clang/Lex/ModuleMap.h

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=280827=280826=280827=diff
==
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Wed Sep  7 12:30:50 2016
@@ -18,6 +18,7 @@
 
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -27,7 +28,7 @@
 #include 
 
 namespace clang {
-  
+
 class DirectoryEntry;
 class FileEntry;
 class FileManager;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24158: Try contextually converting condition of constexpr if to Boolean value

2016-09-07 Thread Ismail Pazarbasi via cfe-commits
ismailp updated this revision to Diff 70561.
ismailp added a comment.

- Added more tests


https://reviews.llvm.org/D24158

Files:
  lib/Sema/SemaOverload.cpp
  test/CodeGenCXX/cxx1z-constexpr-if.cpp

Index: test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,8 +1,17 @@
 // RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s 
--implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -DCHECK_CONVERSION -verify %s
 
 void should_be_used_1();
 void should_be_used_2();
+void should_be_used_3();
 void should_not_be_used();
+
+struct A {
+  constexpr explicit operator bool() const {
+return true;
+  }
+};
+
 void f() {
   if constexpr (false)
 should_not_be_used();
@@ -15,7 +24,17 @@
 goto foo;
 foo: should_not_be_used();
   }
+  if constexpr (A())
+should_be_used_3();
+  else
+should_not_be_used();
+#ifdef CHECK_CONVERSION
+  if constexpr (4.3) ; // expected-error{{conversion from 'double' to 'bool' 
is not allowed in a converted constant expression}}
+  constexpr void *p = nullptr;
+  if constexpr (p) ; // expected-error{{conversion from 'void *const' to 
'bool' is not allowed in a converted constant expression}}
+#endif
 }
 
 // CHECK: should_be_used_1
 // CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5140,12 +5140,18 @@
   //  implicitly converted to type T, where the converted
   //  expression is a constant expression and the implicit conversion
   //  sequence contains only [... list of conversions ...].
+  // C++1z [stmt.if]p2:
+  //  If the if statement is of the form if constexpr, the value of the
+  //  condition shall be a contextually converted constant expression of type
+  //  bool.
   ImplicitConversionSequence ICS =
-TryCopyInitialization(S, From, T,
-  /*SuppressUserConversions=*/false,
-  /*InOverloadResolution=*/false,
-  /*AllowObjcWritebackConversion=*/false,
-  /*AllowExplicit=*/false);
+  CCE == Sema::CCEK_ConstexprIf
+  ? TryContextuallyConvertToBool(S, From)
+  : TryCopyInitialization(S, From, T,
+  /*SuppressUserConversions=*/false,
+  /*InOverloadResolution=*/false,
+  /*AllowObjcWritebackConversion=*/false,
+  /*AllowExplicit=*/false);
   StandardConversionSequence *SCS = nullptr;
   switch (ICS.getKind()) {
   case ImplicitConversionSequence::StandardConversion:


Index: test/CodeGenCXX/cxx1z-constexpr-if.cpp
===
--- test/CodeGenCXX/cxx1z-constexpr-if.cpp
+++ test/CodeGenCXX/cxx1z-constexpr-if.cpp
@@ -1,8 +1,17 @@
 // RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+// RUN: %clang_cc1 -std=c++1z %s -DCHECK_CONVERSION -verify %s
 
 void should_be_used_1();
 void should_be_used_2();
+void should_be_used_3();
 void should_not_be_used();
+
+struct A {
+  constexpr explicit operator bool() const {
+return true;
+  }
+};
+
 void f() {
   if constexpr (false)
 should_not_be_used();
@@ -15,7 +24,17 @@
 goto foo;
 foo: should_not_be_used();
   }
+  if constexpr (A())
+should_be_used_3();
+  else
+should_not_be_used();
+#ifdef CHECK_CONVERSION
+  if constexpr (4.3) ; // expected-error{{conversion from 'double' to 'bool' is not allowed in a converted constant expression}}
+  constexpr void *p = nullptr;
+  if constexpr (p) ; // expected-error{{conversion from 'void *const' to 'bool' is not allowed in a converted constant expression}}
+#endif
 }
 
 // CHECK: should_be_used_1
 // CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5140,12 +5140,18 @@
   //  implicitly converted to type T, where the converted
   //  expression is a constant expression and the implicit conversion
   //  sequence contains only [... list of conversions ...].
+  // C++1z [stmt.if]p2:
+  //  If the if statement is of the form if constexpr, the value of the
+  //  condition shall be a contextually converted constant expression of type
+  //  bool.
   ImplicitConversionSequence ICS =
-TryCopyInitialization(S, From, T,
-  /*SuppressUserConversions=*/false,
-  /*InOverloadResolution=*/false,
-  /*AllowObjcWritebackConversion=*/false,
-  /*AllowExplicit=*/false);
+  CCE == Sema::CCEK_ConstexprIf
+  ? 

Re: [PATCH] D24235: [OpenCL] Improve double literal handling

2016-09-07 Thread Anastasia Stulova via cfe-commits
Anastasia added inline comments.


Comment at: lib/Sema/SemaExpr.cpp:832
@@ -831,2 +831,3 @@
   BTy->getKind() == BuiltinType::Float))
-E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
+  {
+if (getLangOpts().OpenCL &&

This should go on the previous line.


Comment at: lib/Sema/SemaExpr.cpp:837
@@ +836,3 @@
+   .getSupportedOpenCLOpts()
+   .cl_khr_fp64) ||
+  getOpenCLOptions().cl_khr_fp64)) {

Could we merge this and two lines above into one?


Comment at: lib/Sema/SemaExpr.cpp:840
@@ +839,3 @@
+E = ImpCastExprToType(E, Context.FloatTy, CK_FloatingCast).get();
+}
+else

I think the formatting is not right here.

Could you change to:
  } else {


Comment at: test/CodeGenOpenCL/fpmath.cl:28
@@ +27,3 @@
+void testdbllit(long *val) {
+  // CHECK-DBL: float 2.00e+01
+  printf("%f", 20.0);

Could you please add a check that double is generated in either CL2.0 or fp64 
mode.


https://reviews.llvm.org/D24235



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24286: Add MS __nop intrinsic to intrin.h

2016-09-07 Thread Reid Kleckner via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280826: Add MS __nop intrinsic to intrin.h (authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D24286?vs=70495=70555#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24286

Files:
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/test/Headers/ms-intrin.cpp

Index: cfe/trunk/test/Headers/ms-intrin.cpp
===
--- cfe/trunk/test/Headers/ms-intrin.cpp
+++ cfe/trunk/test/Headers/ms-intrin.cpp
@@ -52,6 +52,7 @@
   __cpuidex(info, 0, 0);
   _xgetbv(0);
   __halt();
+  __nop();
   __readmsr(0);
 
   // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -97,6 +97,7 @@
 void __movsd(unsigned long *, unsigned long const *, size_t);
 static __inline__
 void __movsw(unsigned short *, unsigned short const *, size_t);
+static __inline__
 void __nop(void);
 void __nvreg_restore_fence(void);
 void __nvreg_save_fence(void);
@@ -918,6 +919,10 @@
 __halt(void) {
   __asm__ volatile ("hlt");
 }
+static __inline__ void __DEFAULT_FN_ATTRS
+__nop(void) {
+  __asm__ volatile ("nop");
+}
 #endif
 
 
/**\


Index: cfe/trunk/test/Headers/ms-intrin.cpp
===
--- cfe/trunk/test/Headers/ms-intrin.cpp
+++ cfe/trunk/test/Headers/ms-intrin.cpp
@@ -52,6 +52,7 @@
   __cpuidex(info, 0, 0);
   _xgetbv(0);
   __halt();
+  __nop();
   __readmsr(0);
 
   // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -97,6 +97,7 @@
 void __movsd(unsigned long *, unsigned long const *, size_t);
 static __inline__
 void __movsw(unsigned short *, unsigned short const *, size_t);
+static __inline__
 void __nop(void);
 void __nvreg_restore_fence(void);
 void __nvreg_save_fence(void);
@@ -918,6 +919,10 @@
 __halt(void) {
   __asm__ volatile ("hlt");
 }
+static __inline__ void __DEFAULT_FN_ATTRS
+__nop(void) {
+  __asm__ volatile ("nop");
+}
 #endif
 
 /**\
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r280826 - Add MS __nop intrinsic to intrin.h

2016-09-07 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Wed Sep  7 11:55:12 2016
New Revision: 280826

URL: http://llvm.org/viewvc/llvm-project?rev=280826=rev
Log:
Add MS __nop intrinsic to intrin.h

Summary: There was no definition for __nop function - added inline
assembly.

Patch by Albert Gutowski!

Reviewers: rnk, thakis

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D24286

Modified:
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/test/Headers/ms-intrin.cpp

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=280826=280825=280826=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Wed Sep  7 11:55:12 2016
@@ -97,6 +97,7 @@ static __inline__
 void __movsd(unsigned long *, unsigned long const *, size_t);
 static __inline__
 void __movsw(unsigned short *, unsigned short const *, size_t);
+static __inline__
 void __nop(void);
 void __nvreg_restore_fence(void);
 void __nvreg_save_fence(void);
@@ -918,6 +919,10 @@ static __inline__ void __DEFAULT_FN_ATTR
 __halt(void) {
   __asm__ volatile ("hlt");
 }
+static __inline__ void __DEFAULT_FN_ATTRS
+__nop(void) {
+  __asm__ volatile ("nop");
+}
 #endif
 
 
/**\

Modified: cfe/trunk/test/Headers/ms-intrin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/ms-intrin.cpp?rev=280826=280825=280826=diff
==
--- cfe/trunk/test/Headers/ms-intrin.cpp (original)
+++ cfe/trunk/test/Headers/ms-intrin.cpp Wed Sep  7 11:55:12 2016
@@ -52,6 +52,7 @@ void f() {
   __cpuidex(info, 0, 0);
   _xgetbv(0);
   __halt();
+  __nop();
   __readmsr(0);
 
   // FIXME: Call these in 64-bit too once the intrinsics have been fixed to


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24153: Add bunch of _Interlocked builtins

2016-09-07 Thread Reid Kleckner via cfe-commits
rnk added a comment.

Looks good to me. David, do you remember any subtleties here? I seem to recall 
there were some bugs in our intrin.h implementations, or inconsistencies 
between us and MSVC.



Comment at: test/CodeGen/ms-intrinsics.c:297
@@ +296,3 @@
+// CHECK: define{{.*}}i16 @test_InterlockedIncrement16(i16*{{[a-z_ 
]*}}%Addend){{.*}}{
+// CHECK: [[TMP:%[0-9]+]] = atomicrmw volatile add i16* %Addend, i16 1 seq_cst
+// CHECK: [[RESULT:%[0-9]+]] = add i16 [[TMP]], 1

I think we should make these increments and decrements non-volatile. I'd like 
us to be able to optimize this kind of code to return 2:
  int f() {
int x = 0;
_InterlockedIncrement();
_InterlockedIncrement();
   return x; // always 2
  }


https://reviews.llvm.org/D24153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D23944: Parsing MS pragma intrinsic

2016-09-07 Thread Reid Kleckner via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL280825: Parsing MS pragma intrinsic (authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D23944?vs=69768=70551#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23944

Files:
  cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
  cfe/trunk/include/clang/Basic/IdentifierTable.h
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/lib/Parse/ParsePragma.cpp
  cfe/trunk/test/Preprocessor/pragma_microsoft.c

Index: cfe/trunk/include/clang/Basic/IdentifierTable.h
===
--- cfe/trunk/include/clang/Basic/IdentifierTable.h
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h
@@ -205,8 +205,7 @@
 
   /// \brief Return a value indicating whether this is a builtin function.
   ///
-  /// 0 is not-built-in.  1 is builtin-for-some-nonprimary-target.
-  /// 2+ are specific builtin functions.
+  /// 0 is not-built-in. 1+ are specific builtin functions.
   unsigned getBuiltinID() const {
 if (ObjCOrBuiltinID >= tok::NUM_OBJC_KEYWORDS)
   return ObjCOrBuiltinID - tok::NUM_OBJC_KEYWORDS;
Index: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
@@ -911,6 +911,10 @@
 def warn_pragma_pack_malformed : Warning<
   "expected integer or identifier in '#pragma pack' - ignored">,
   InGroup;
+// - #pragma intrinsic
+def warn_pragma_intrinsic_builtin : Warning<
+  "%0 is not a recognized builtin%select{|; consider including  to access non-builtin intrinsics}1">,
+  InGroup;
 // - #pragma unused
 def warn_pragma_unused_expected_var : Warning<
   "expected '#pragma unused' argument to be a variable name">,
Index: cfe/trunk/include/clang/Parse/Parser.h
===
--- cfe/trunk/include/clang/Parse/Parser.h
+++ cfe/trunk/include/clang/Parse/Parser.h
@@ -172,6 +172,7 @@
   std::unique_ptr MSCodeSeg;
   std::unique_ptr MSSection;
   std::unique_ptr MSRuntimeChecks;
+  std::unique_ptr MSIntrinsic;
   std::unique_ptr OptimizeHandler;
   std::unique_ptr LoopHintHandler;
   std::unique_ptr UnrollHintHandler;
Index: cfe/trunk/test/Preprocessor/pragma_microsoft.c
===
--- cfe/trunk/test/Preprocessor/pragma_microsoft.c
+++ cfe/trunk/test/Preprocessor/pragma_microsoft.c
@@ -162,3 +162,19 @@
 
 // Test that runtime_checks is parsed but ignored.
 #pragma runtime_checks("sc", restore) // no-warning
+
+// Test pragma intrinsic
+#pragma intrinsic(memset) // no-warning
+#pragma intrinsic(memcpy, strlen, strlen) // no-warning
+#pragma intrinsic() // no-warning
+#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+#pragma intrinsic(main) // expected-warning {{'main' is not a recognized builtin; consider including }}
+#pragma intrinsic( // expected-warning {{missing ')' after}}
+#pragma intrinsic(int) // expected-warning {{missing ')' after}}
+#pragma intrinsic(strcmp) asdf // expected-warning {{extra tokens at end}}
+
+#define __INTRIN_H  // there should be no notes after defining __INTRIN_H
+#pragma intrinsic(asdf) // expected-warning-re {{'asdf' is not a recognized builtin{{$
+#pragma intrinsic(memset) // no-warning
+#undef __INTRIN_H
+#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
Index: cfe/trunk/lib/Parse/ParsePragma.cpp
===
--- cfe/trunk/lib/Parse/ParsePragma.cpp
+++ cfe/trunk/lib/Parse/ParsePragma.cpp
@@ -161,6 +161,12 @@
   PragmaMSRuntimeChecksHandler() : EmptyPragmaHandler("runtime_checks") {}
 };
 
+struct PragmaMSIntrinsicHandler : public PragmaHandler {
+  PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
+  void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
+Token ) override;
+};
+
 }  // end namespace
 
 void Parser::initializePragmaHandlers() {
@@ -229,6 +235,8 @@
 PP.AddPragmaHandler(MSSection.get());
 MSRuntimeChecks.reset(new PragmaMSRuntimeChecksHandler());
 PP.AddPragmaHandler(MSRuntimeChecks.get());
+MSIntrinsic.reset(new PragmaMSIntrinsicHandler());
+PP.AddPragmaHandler(MSIntrinsic.get());
   }
 
   OptimizeHandler.reset(new PragmaOptimizeHandler(Actions));
@@ -297,6 +305,8 @@
 MSSection.reset();
 PP.RemovePragmaHandler(MSRuntimeChecks.get());
 MSRuntimeChecks.reset();
+PP.RemovePragmaHandler(MSIntrinsic.get());
+MSIntrinsic.reset();
   }
 
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
@@ -2127,3 +2137,53 @@
   PP.EnterTokenStream(std::move(TokenArray), 1,
   /*DisableMacroExpansion=*/false);
 }
+
+/// \brief Handle the Microsoft \#pragma intrinsic 

r280825 - Parsing MS pragma intrinsic

2016-09-07 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Wed Sep  7 11:38:32 2016
New Revision: 280825

URL: http://llvm.org/viewvc/llvm-project?rev=280825=rev
Log:
Parsing MS pragma intrinsic

Parse pragma intrinsic, display warning if the function isn't a builtin
function in clang and suggest including intrin.h.

Patch by Albert Gutowski!

Reviewers: aaron.ballman, rnk

Subscribers: aaron.ballman, cfe-commits

Differential Revision: https://reviews.llvm.org/D23944

Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Basic/IdentifierTable.h
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/test/Preprocessor/pragma_microsoft.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=280825=280824=280825=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Sep  7 11:38:32 
2016
@@ -911,6 +911,10 @@ def warn_pragma_invalid_action : Warning
 def warn_pragma_pack_malformed : Warning<
   "expected integer or identifier in '#pragma pack' - ignored">,
   InGroup;
+// - #pragma intrinsic
+def warn_pragma_intrinsic_builtin : Warning<
+  "%0 is not a recognized builtin%select{|; consider including  to 
access non-builtin intrinsics}1">,
+  InGroup;
 // - #pragma unused
 def warn_pragma_unused_expected_var : Warning<
   "expected '#pragma unused' argument to be a variable name">,

Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=280825=280824=280825=diff
==
--- cfe/trunk/include/clang/Basic/IdentifierTable.h (original)
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h Wed Sep  7 11:38:32 2016
@@ -205,8 +205,7 @@ public:
 
   /// \brief Return a value indicating whether this is a builtin function.
   ///
-  /// 0 is not-built-in.  1 is builtin-for-some-nonprimary-target.
-  /// 2+ are specific builtin functions.
+  /// 0 is not-built-in. 1+ are specific builtin functions.
   unsigned getBuiltinID() const {
 if (ObjCOrBuiltinID >= tok::NUM_OBJC_KEYWORDS)
   return ObjCOrBuiltinID - tok::NUM_OBJC_KEYWORDS;

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=280825=280824=280825=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Wed Sep  7 11:38:32 2016
@@ -172,6 +172,7 @@ class Parser : public CodeCompletionHand
   std::unique_ptr MSCodeSeg;
   std::unique_ptr MSSection;
   std::unique_ptr MSRuntimeChecks;
+  std::unique_ptr MSIntrinsic;
   std::unique_ptr OptimizeHandler;
   std::unique_ptr LoopHintHandler;
   std::unique_ptr UnrollHintHandler;

Modified: cfe/trunk/lib/Parse/ParsePragma.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParsePragma.cpp?rev=280825=280824=280825=diff
==
--- cfe/trunk/lib/Parse/ParsePragma.cpp (original)
+++ cfe/trunk/lib/Parse/ParsePragma.cpp Wed Sep  7 11:38:32 2016
@@ -161,6 +161,12 @@ struct PragmaMSRuntimeChecksHandler : pu
   PragmaMSRuntimeChecksHandler() : EmptyPragmaHandler("runtime_checks") {}
 };
 
+struct PragmaMSIntrinsicHandler : public PragmaHandler {
+  PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
+  void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer,
+Token ) override;
+};
+
 }  // end namespace
 
 void Parser::initializePragmaHandlers() {
@@ -229,6 +235,8 @@ void Parser::initializePragmaHandlers()
 PP.AddPragmaHandler(MSSection.get());
 MSRuntimeChecks.reset(new PragmaMSRuntimeChecksHandler());
 PP.AddPragmaHandler(MSRuntimeChecks.get());
+MSIntrinsic.reset(new PragmaMSIntrinsicHandler());
+PP.AddPragmaHandler(MSIntrinsic.get());
   }
 
   OptimizeHandler.reset(new PragmaOptimizeHandler(Actions));
@@ -297,6 +305,8 @@ void Parser::resetPragmaHandlers() {
 MSSection.reset();
 PP.RemovePragmaHandler(MSRuntimeChecks.get());
 MSRuntimeChecks.reset();
+PP.RemovePragmaHandler(MSIntrinsic.get());
+MSIntrinsic.reset();
   }
 
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
@@ -2127,3 +2137,53 @@ void PragmaUnrollHintHandler::HandlePrag
   PP.EnterTokenStream(std::move(TokenArray), 1,
   /*DisableMacroExpansion=*/false);
 }
+
+/// \brief Handle the Microsoft \#pragma intrinsic extension.
+///
+/// The syntax is:
+/// \code
+///  #pragma intrinsic(memset)
+///  #pragma intrinsic(strlen, memcpy)
+/// \endcode
+///
+/// Pragma intrisic tells the compiler to use a 

Re: [PATCH] D24075: [include-fixer] Support finding headers for the symbol under cursor.

2016-09-07 Thread Haojian Wu via cfe-commits
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rL280824: [include-fixer] Support finding headers for the 
symbol under cursor. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D24075?vs=70545=70549#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24075

Files:
  clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
  clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
  clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
  clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
  clang-tools-extra/trunk/test/include-fixer/query_symbol.cpp

Index: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
===
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
@@ -36,11 +36,17 @@
 
 (defcustom clang-include-fixer-init-string
   ""
-  "clang-include-fixer input format."
+  "clang-include-fixer init string."
   :group 'clang-include-fixer
   :type 'string
   :risky t)
 
+(defcustom clang-include-fixer-query-mode
+  nil
+  "clang-include-fixer query mode."
+  :group 'clang-include-fixer
+  :type 'boolean
+  :risky t)
 
 (defun clang-include-fixer-call-executable (callee
 include-fixer-parameter-a
@@ -197,11 +203,28 @@
   (message (concat "Calling the include fixer. "
"This might take some seconds. Please wait."))
 
-  (clang-include-fixer-call-executable
-   'clang-include-fixer-add-header
-   (concat "-db=" clang-include-fixer-input-format)
-   (concat "-input=" clang-include-fixer-init-string)
-   "-output-headers"))
+  (if clang-include-fixer-query-mode
+  (let (p1 p2)
+  (save-excursion
+(skip-chars-backward "-a-zA-Z0-9_:")
+(setq p1 (point))
+(skip-chars-forward "-a-zA-Z0-9_:")
+(setq p2 (point))
+(setq query-symbol (buffer-substring-no-properties p1 p2))
+(if (string= "" query-symbol)
+(message "Skip querying empty symbol.")
+  (clang-include-fixer-call-executable
+'clang-include-fixer-add-header
+(concat "-db=" clang-include-fixer-input-format)
+(concat "-input=" clang-include-fixer-init-string)
+(concat "-query-symbol=" (thing-at-point 'symbol))
+  
+(clang-include-fixer-call-executable
+  'clang-include-fixer-add-header
+  (concat "-db=" clang-include-fixer-input-format)
+  (concat "-input=" clang-include-fixer-init-string)
+  "-output-headers"))
+  )
 
 (provide 'clang-include-fixer)
 ;;; clang-include-fixer.el ends here
Index: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
===
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py
@@ -17,9 +17,10 @@
 
 import argparse
 import difflib
+import json
+import re
 import subprocess
 import vim
-import json
 
 # set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
 # on the path.
@@ -44,6 +45,10 @@
 if vim.eval('exists("g:clang_include_fixer_jump_to_include")') == "1":
   jump_to_include = vim.eval('g:clang_include_fixer_jump_to_include') != "0"
 
+query_mode = True
+if vim.eval('exists("g:clang_include_fixer_query_mode")') == "1":
+  query_mode = vim.eval('g:clang_include_fixer_query_mode') != "0"
+
 
 def GetUserSelection(message, headers, maximum_suggested_headers):
   eval_message = message + '\n'
@@ -105,6 +110,25 @@
   vim.current.window.cursor = (line_num, 0)
 
 
+# The vim internal implementation (expand("cword"/"cWORD")) doesn't support
+# our use case very well, we re-implement our own one.
+def get_symbol_under_cursor():
+  line = vim.eval("line(\".\")")
+  # column number in vim is 1-based.
+  col = int(vim.eval("col(\".\")")) - 1
+  line_text = vim.eval("getline({0})".format(line))
+  if len(line_text) == 0: return ""
+  symbol_pos_begin = col
+  p = re.compile('[a-zA-Z0-9:_]')
+  while symbol_pos_begin >= 0 and p.match(line_text[symbol_pos_begin]):
+symbol_pos_begin -= 1
+
+  symbol_pos_end = col
+  while symbol_pos_end < len(line_text) and p.match(line_text[symbol_pos_end]):
+symbol_pos_end += 1
+  return line_text[symbol_pos_begin+1:symbol_pos_end]
+
+
 def main():
   parser = argparse.ArgumentParser(
   description='Vim integration for clang-include-fixer')
@@ -118,9 +142,18 @@
   buf = vim.current.buffer
   text = '\n'.join(buf)
 
-  # Run command to get all headers.
-  command = [binary, "-stdin", "-output-headers", "-db=" + args.db,
- "-input=" + args.input, vim.current.buffer.name]
+  if query_mode:
+symbol = get_symbol_under_cursor()
+if len(symbol) == 0:
+  print "Skip 

[clang-tools-extra] r280824 - [include-fixer] Support finding headers for the symbol under cursor.

2016-09-07 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Wed Sep  7 11:34:35 2016
New Revision: 280824

URL: http://llvm.org/viewvc/llvm-project?rev=280824=rev
Log:
[include-fixer] Support finding headers for the symbol under cursor.

Summary:
* Add a `query-symbol` option to query symbol without parsing the source file.
* Update Vim & Emacs integration scripts.

Reviewers: bkramer, massberg

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D24075

Added:
clang-tools-extra/trunk/test/include-fixer/query_symbol.cpp
Modified:
clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.py

Modified: clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h?rev=280824=280823=280824=diff
==
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h 
(original)
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/SymbolInfo.h Wed Sep 
 7 11:34:35 2016
@@ -54,6 +54,8 @@ public:
  int LineNumber, const std::vector ,
  unsigned NumOccurrences = 0);
 
+  void SetFilePath(llvm::StringRef Path) { FilePath = Path; }
+
   /// \brief Get symbol name.
   llvm::StringRef getName() const { return Name; }
 

Modified: clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp?rev=280824=280823=280824=diff
==
--- clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/tool/ClangIncludeFixer.cpp Wed Sep  7 
11:34:35 2016
@@ -98,6 +98,12 @@ cl::opt Input("input",
cl::desc("String to initialize the database"),
cl::cat(IncludeFixerCategory));
 
+cl::opt
+QuerySymbol("query-symbol",
+ cl::desc("Query a given symbol (e.g. \"a::b::foo\") in\n"
+  "database directly without parsing the file."),
+ cl::cat(IncludeFixerCategory));
+
 cl::opt
 MinimizeIncludePaths("minimize-paths",
  cl::desc("Whether to minimize added include paths"),
@@ -236,6 +242,7 @@ int includeFixerMain(int argc, const cha
   tooling::ClangTool tool(options.getCompilations(),
   options.getSourcePathList());
 
+  llvm::StringRef SourceFilePath = options.getSourcePathList().front();
   // In STDINMode, we override the file content with the  input.
   // Since `tool.mapVirtualFile` takes `StringRef`, we define `Code` outside of
   // the if-block so that `Code` is not released after the if-block.
@@ -253,7 +260,7 @@ int includeFixerMain(int argc, const cha
 if (Code->getBufferSize() == 0)
   return 0;  // Skip empty files.
 
-tool.mapVirtualFile(options.getSourcePathList().front(), 
Code->getBuffer());
+tool.mapVirtualFile(SourceFilePath, Code->getBuffer());
   }
 
   if (!InsertHeader.empty()) {
@@ -314,10 +321,31 @@ int includeFixerMain(int argc, const cha
 
   // Set up data source.
   std::unique_ptr SymbolIndexMgr =
-  createSymbolIndexManager(options.getSourcePathList().front());
+  createSymbolIndexManager(SourceFilePath);
   if (!SymbolIndexMgr)
 return 1;
 
+  // Query symbol mode.
+  if (!QuerySymbol.empty()) {
+auto MatchedSymbols = SymbolIndexMgr->search(QuerySymbol);
+for (auto  : MatchedSymbols) {
+  std::string HeaderPath = Symbol.getFilePath().str();
+  Symbol.SetFilePath(((HeaderPath[0] == '"' || HeaderPath[0] == '<')
+  ? HeaderPath
+  : "\"" + HeaderPath + "\""));
+}
+
+// We leave an empty symbol range as we don't know the range of the symbol
+// being queried in this mode. include-fixer won't add namespace qualifiers
+// if the symbol range is empty, which also fits this case.
+IncludeFixerContext::QuerySymbolInfo Symbol;
+Symbol.RawIdentifier = QuerySymbol;
+auto Context =
+IncludeFixerContext(SourceFilePath, {Symbol}, MatchedSymbols);
+writeToJson(llvm::outs(), Context);
+return 0;
+  }
+
   // Now run our tool.
   std::vector Contexts;
   include_fixer::IncludeFixerActionFactory Factory(*SymbolIndexMgr, Contexts,

Modified: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el?rev=280824=280823=280824=diff
==
--- 

Re: [PATCH] D23284: Add -fdiagnostics-show-hotness

2016-09-07 Thread Adam Nemet via cfe-commits
anemet updated this revision to Diff 70547.
anemet updated the summary for this revision.
anemet added a comment.

Address Richard's comment


https://reviews.llvm.org/D23284

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CodeGenAction.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Frontend/Inputs/optimization-remark-with-hotness.proftext
  test/Frontend/optimization-remark-with-hotness.c

Index: test/Frontend/optimization-remark-with-hotness.c
===
--- /dev/null
+++ test/Frontend/optimization-remark-with-hotness.c
@@ -0,0 +1,45 @@
+// RUN: llvm-profdata merge \
+// RUN: %S/Inputs/optimization-remark-with-hotness.proftext   \
+// RUN: -o %t.profdata
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
+// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
+// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
+// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness -verify
+// The clang version of the previous test.
+// RUN: %clang -target x86_64-apple-macosx10.9 %s -o /dev/null \
+// RUN: -fprofile-instr-use=%t.profdata -Rpass=inline \
+// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness -Xclang -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
+// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
+// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
+// RUN: -Rpass-analysis=inline 2>&1 | FileCheck -check-prefix=HOTNESS_OFF %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
+// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
+// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
+// RUN: -Rpass-analysis=inline -Rno-pass-with-hotness  2>&1 | FileCheck \
+// RUN: -check-prefix=HOTNESS_OFF %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
+// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
+// RUN: -Rpass=inline -Rpass-analysis=inline -fdiagnostics-show-hotness  2>&1 \
+// RUN: | FileCheck -check-prefix=NO_PGO %s
+
+int foo(int x, int y) __attribute__((always_inline));
+int foo(int x, int y) { return x + y; }
+
+int sum = 0;
+
+void bar(int x) {
+  // HOTNESS_OFF: foo inlined into bar
+  // HOTNESS_OFF-NOT: hotness:
+  // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
+  // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 30)}}
+  // expected-remark@+1 {{foo inlined into bar (hotness: 30)}}
+  sum += foo(x, x - 2);
+}
+
+int main(int argc, const char *argv[]) {
+  for (int i = 0; i < 30; i++)
+// expected-remark@+1 {{bar should never be inlined}}
+bar(argc);
+  return sum;
+}
Index: test/Frontend/Inputs/optimization-remark-with-hotness.proftext
===
--- /dev/null
+++ test/Frontend/Inputs/optimization-remark-with-hotness.proftext
@@ -0,0 +1,25 @@
+foo
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+30
+
+bar
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+30
+
+main
+# Func Hash:
+4
+# Num Counters:
+2
+# Counter Values:
+1
+30
+
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -839,6 +839,12 @@
 NeedLocTracking = true;
   }
 
+  Opts.DiagnosticsWithHotness =
+  Args.hasArg(options::OPT_fdiagnostics_show_hotness);
+  if (Opts.DiagnosticsWithHotness &&
+  Opts.getProfileUse() == CodeGenOptions::ProfileNone)
+Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
+
   // If the user requested to use a sample profile for PGO, then the
   // backend will need to track source location information so the profile
   // can be incorporated into the IR.
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4896,6 +4896,7 @@
   claimNoWarnArgs(Args);
 
   Args.AddAllArgs(CmdArgs, options::OPT_R_Group);
+
   Args.AddAllArgs(CmdArgs, options::OPT_W_Group);
   if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false))
 CmdArgs.push_back("-pedantic");
@@ -5898,6 +5899,10 @@
 CmdArgs.push_back(A->getValue());
   }
 
+  if (Args.hasFlag(options::OPT_fdiagnostics_show_hotness,
+   options::OPT_fno_diagnostics_show_hotness, false))
+CmdArgs.push_back("-fdiagnostics-show-hotness");
+
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
Index: lib/CodeGen/CodeGenAction.cpp

Re: [PATCH] D23284: Add -fdiagnostics-show-hotness

2016-09-07 Thread Adam Nemet via cfe-commits
anemet marked an inline comment as done.
anemet added a comment.

https://reviews.llvm.org/D23284



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24075: [include-fixer] Support finding headers for the symbol under cursor.

2016-09-07 Thread Jens Massberg via cfe-commits
massberg added a comment.

The Emacs part looks good for me now.



Comment at: include-fixer/tool/clang-include-fixer.el:204
@@ -197,3 +203,3 @@
   (message (concat "Calling the include fixer. "
"This might take some seconds. Please wait."))
 

Then it fine for me, too.


https://reviews.llvm.org/D24075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24075: [include-fixer] Support finding headers for the symbol under cursor.

2016-09-07 Thread Haojian Wu via cfe-commits
hokein marked an inline comment as done.


Comment at: include-fixer/tool/clang-include-fixer.el:204
@@ -197,3 +203,3 @@
   (message (concat "Calling the include fixer. "
"This might take some seconds. Please wait."))
 

massberg wrote:
> Is this message still accurate when using the new query mode?
I think it is fine to keep it here.


https://reviews.llvm.org/D24075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D24075: [include-fixer] Support finding headers for the symbol under cursor.

2016-09-07 Thread Haojian Wu via cfe-commits
hokein updated this revision to Diff 70545.
hokein marked an inline comment as done.
hokein updated the summary for this revision.
hokein added a comment.

bool => boolean


https://reviews.llvm.org/D24075

Files:
  include-fixer/find-all-symbols/SymbolInfo.h
  include-fixer/tool/ClangIncludeFixer.cpp
  include-fixer/tool/clang-include-fixer.el
  include-fixer/tool/clang-include-fixer.py
  test/include-fixer/query_symbol.cpp

Index: test/include-fixer/query_symbol.cpp
===
--- /dev/null
+++ test/include-fixer/query_symbol.cpp
@@ -0,0 +1,13 @@
+// RUN: clang-include-fixer -db=fixed -input='foo= "foo.h","bar.h"' -query-symbol="foo" test.cpp -- | FileCheck %s
+
+// CHECK: "FilePath": "test.cpp",
+// CHECK-NEXT:"QuerySymbolInfos": [
+// CHECK-NEXT:   {"RawIdentifier": "foo",
+// CHECK-NEXT:"Range":{"Offset":0,"Length":0}}
+// CHECK-NEXT:],
+// CHECK-NEXT:"HeaderInfos": [
+// CHECK-NEXT:  {"Header": "\"foo.h\"",
+// CHECK-NEXT:   "QualifiedName": "foo"},
+// CHECK-NEXT:  {"Header": "\"bar.h\"",
+// CHECK-NEXT:   "QualifiedName": "foo"}
+// CHECK-NEXT:]
Index: include-fixer/tool/clang-include-fixer.py
===
--- include-fixer/tool/clang-include-fixer.py
+++ include-fixer/tool/clang-include-fixer.py
@@ -17,9 +17,10 @@
 
 import argparse
 import difflib
+import json
+import re
 import subprocess
 import vim
-import json
 
 # set g:clang_include_fixer_path to the path to clang-include-fixer if it is not
 # on the path.
@@ -44,6 +45,10 @@
 if vim.eval('exists("g:clang_include_fixer_jump_to_include")') == "1":
   jump_to_include = vim.eval('g:clang_include_fixer_jump_to_include') != "0"
 
+query_mode = True
+if vim.eval('exists("g:clang_include_fixer_query_mode")') == "1":
+  query_mode = vim.eval('g:clang_include_fixer_query_mode') != "0"
+
 
 def GetUserSelection(message, headers, maximum_suggested_headers):
   eval_message = message + '\n'
@@ -105,6 +110,25 @@
   vim.current.window.cursor = (line_num, 0)
 
 
+# The vim internal implementation (expand("cword"/"cWORD")) doesn't support
+# our use case very well, we re-implement our own one.
+def get_symbol_under_cursor():
+  line = vim.eval("line(\".\")")
+  # column number in vim is 1-based.
+  col = int(vim.eval("col(\".\")")) - 1
+  line_text = vim.eval("getline({0})".format(line))
+  if len(line_text) == 0: return ""
+  symbol_pos_begin = col
+  p = re.compile('[a-zA-Z0-9:_]')
+  while symbol_pos_begin >= 0 and p.match(line_text[symbol_pos_begin]):
+symbol_pos_begin -= 1
+
+  symbol_pos_end = col
+  while symbol_pos_end < len(line_text) and p.match(line_text[symbol_pos_end]):
+symbol_pos_end += 1
+  return line_text[symbol_pos_begin+1:symbol_pos_end]
+
+
 def main():
   parser = argparse.ArgumentParser(
   description='Vim integration for clang-include-fixer')
@@ -118,9 +142,18 @@
   buf = vim.current.buffer
   text = '\n'.join(buf)
 
-  # Run command to get all headers.
-  command = [binary, "-stdin", "-output-headers", "-db=" + args.db,
- "-input=" + args.input, vim.current.buffer.name]
+  if query_mode:
+symbol = get_symbol_under_cursor()
+if len(symbol) == 0:
+  print "Skip querying empty symbol."
+  return
+command = [binary, "-stdin", "-query-symbol="+get_symbol_under_cursor(),
+   "-db=" + args.db, "-input=" + args.input,
+   vim.current.buffer.name]
+  else:
+# Run command to get all headers.
+command = [binary, "-stdin", "-output-headers", "-db=" + args.db,
+   "-input=" + args.input, vim.current.buffer.name]
   stdout, stderr = execute(command, text)
   if stderr:
 print >> sys.stderr, "Error while running clang-include-fixer: " + stderr
Index: include-fixer/tool/clang-include-fixer.el
===
--- include-fixer/tool/clang-include-fixer.el
+++ include-fixer/tool/clang-include-fixer.el
@@ -36,11 +36,17 @@
 
 (defcustom clang-include-fixer-init-string
   ""
-  "clang-include-fixer input format."
+  "clang-include-fixer init string."
   :group 'clang-include-fixer
   :type 'string
   :risky t)
 
+(defcustom clang-include-fixer-query-mode
+  nil
+  "clang-include-fixer query mode."
+  :group 'clang-include-fixer
+  :type 'boolean
+  :risky t)
 
 (defun clang-include-fixer-call-executable (callee
 include-fixer-parameter-a
@@ -197,11 +203,28 @@
   (message (concat "Calling the include fixer. "
"This might take some seconds. Please wait."))
 
-  (clang-include-fixer-call-executable
-   'clang-include-fixer-add-header
-   (concat "-db=" clang-include-fixer-input-format)
-   (concat "-input=" clang-include-fixer-init-string)
-   "-output-headers"))
+  (if clang-include-fixer-query-mode
+  (let (p1 p2)
+  (save-excursion
+(skip-chars-backward "-a-zA-Z0-9_:")
+(setq p1 (point))
+

Re: [PATCH] D22227: [ubsan] Disable bounds-check for flexible array ivars

2016-09-07 Thread Vedant Kumar via cfe-commits
vsk added a comment.

Ping?


https://reviews.llvm.org/D7



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >