[PATCH] D32520: Support __fp16 vectors

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
Herald added subscribers: rengolin, aemerson.

Currently, clang miscompiles operations on __fp16 vectors.

For example, when the following code is compiled:

  typedef __fp16 half4 __attribute__ ((vector_size (8)));
  half4 hv0, hv1, hv2;
  
  void test() {
hv0 = hv1 + hv2;
  }

clang generates the following IR on ARM64:

  %1 = load <4 x half>, <4 x half>* @hv1, align 8
  %2 = load <4 x half>, <4 x half>* @hv2, align 8
  %3 = fadd <4 x half> %1, %2
  store <4 x half> %3, <4 x half>* @hv0, align 8

This isn't correct since __fp16 values in C or C++ expressions have to be 
promoted to float if __fp16 is not a natively supported type (see gcc's 
documentation).

https://gcc.gnu.org/onlinedocs/gcc/Half-Precision.html

The IR is incorrect on X86 too. The addition is done on <4xi16>vectors:

  %1 = load <4 x i16>, <4 x i16>* @hv1, align 8
  %2 = load <4 x i16>, <4 x i16>* @hv2, align 8
  %3 = add <4 x i16> %2, %1
  store <4 x i16> %3, <4 x i16>* @hv0, align 8

This patch makes the changes needed in Sema and IRGen to generate the correct 
IR on targets that set HalfArgsAndReturns to true but don't support __fp16 
natively (ARM and ARM64). It inserts implicit casts to promote __fp16 vector 
operands to float vectors and truncate the result back to a __fp16 vector.

I plan to fix X86 and other targets that don't set HalfArgsAndReturns to true 
in another patch.


https://reviews.llvm.org/D32520

Files:
  include/clang/Sema/Sema.h
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGen/fp16vec-ops.c
  test/Sema/fp16vec-sema.c

Index: test/Sema/fp16vec-sema.c
===
--- /dev/null
+++ test/Sema/fp16vec-sema.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef __fp16 half4 __attribute__ ((vector_size (8)));
+typedef float float4 __attribute__ ((vector_size (16)));
+typedef short short4 __attribute__ ((vector_size (8)));
+typedef int int4 __attribute__ ((vector_size (16)));
+
+half4 hv0, hv1;
+float4 fv0, fv1;
+short4 sv0;
+int4 iv0;
+
+void testFP16Vec(int c) {
+  hv0 = hv0 + hv1;
+  hv0 = hv0 - hv1;
+  hv0 = hv0 * hv1;
+  hv0 = hv0 / hv1;
+  hv0 = c ? hv0 : hv1;
+  hv0 += hv1;
+  hv0 -= hv1;
+  hv0 *= hv1;
+  hv0 /= hv1;
+  sv0 = hv0 == hv1;
+  sv0 = hv0 != hv1;
+  sv0 = hv0 < hv1;
+  sv0 = hv0 > hv1;
+  sv0 = hv0 <= hv1;
+  sv0 = hv0 >= hv1;
+  sv0 = hv0 || hv1; // FIXME: this should be an error in C.
+  sv0 = hv0 && hv1; // FIXME: this should be an error in C.
+
+  // Implicit conversion between half vectors and float vectors are not allowed.
+  hv0 = fv0; // expected-error{{assigning to}}
+  fv0 = hv0; // expected-error{{assigning to}}
+  hv0 = (half4)fv0; // expected-error{{invalid conversion between}}
+  fv0 = (float4)hv0; // expected-error{{invalid conversion between}}
+  hv0 = fv0 + fv1; // expected-error{{assigning to}}
+  fv0 = hv0 + hv1; // expected-error{{assigning to}}
+  hv0 = hv0 + fv1; // expected-error{{cannot convert between vector}}
+  hv0 = c ? hv0 : fv1; // expected-error{{cannot convert between vector}}
+  sv0 = hv0 == fv1; // expected-error{{cannot convert between vector}}
+  sv0 = hv0 < fv1; // expected-error{{cannot convert between vector}}
+  sv0 = hv0 || fv1; // expected-error{{cannot convert between vector}} expected-error{{invalid operands to binary expression}}
+  iv0 = hv0 == hv1; // expected-error{{assigning to}}
+
+  // clang currently disallows using these operators on vectors, which is
+  // allowed by gcc.
+  sv0 = !hv0; // expected-error{{invalid argument type}}
+  hv0++; // expected-error{{cannot increment value of type}}
+  ++hv0; // expected-error{{cannot increment value of type}}
+}
Index: test/CodeGen/fp16vec-ops.c
===
--- /dev/null
+++ test/CodeGen/fp16vec-ops.c
@@ -0,0 +1,162 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -triple arm64-apple-ios9 -emit-llvm -o - -fallow-half-arguments-and-returns %s | FileCheck %s --check-prefix=CHECK
+// RUN: %clang_cc1 -triple armv7-apple-ios9 -emit-llvm -o - -fallow-half-arguments-and-returns %s | FileCheck %s --check-prefix=CHECK
+
+typedef __fp16 half4 __attribute__ ((vector_size (8)));
+typedef short short4 __attribute__ ((vector_size (8)));
+
+half4 hv0, hv1;
+short4 sv0;
+
+// CHECK-LABEL: testFP16Vec0
+// CHECK: %[[V0:.*]] = load <4 x half>, <4 x half>* @hv0, align 8
+// CHECK: %[[CONV:.*]] = fpext <4 x half> %[[V0]] to <4 x float>
+// CHECK: %[[V1:.*]] = load <4 x half>, <4 x half>* @hv1, align 8
+// CHECK: %[[CONV1:.*]] = fpext <4 x half> %[[V1]] to <4 x float>
+// CHECK: %[[ADD:.*]] = fadd <4 x float> %[[CONV]], %[[CONV1]]
+// CHECK: %[[CONV2:.*]] = fptrunc <4 x float> %[[ADD]] to <4 x half>
+// CHECK: store <4 x half> %[[CONV2]], <4 x half>* @hv0, align 8
+// CHECK: %[[V2:.*]] = load <4 x half>, <4 x half>* @hv0, align 8
+// CHECK: %[[CONV3:.*]] = fpext <4 x half> %[[V2]] to <4 x float>
+// CHECK: %[[V3:.*]] = load <4 x 

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Leslie Zhai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL301384: [analyzer] Teach the MallocChecker about Glib API 
for two arguments (authored by xiangzhai).

Changed prior to commit:
  https://reviews.llvm.org/D30771?vs=96101=96670#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30771

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/gmalloc.c

Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -177,7 +177,10 @@
 II_wcsdup(nullptr), II_win_wcsdup(nullptr), II_g_malloc(nullptr),
 II_g_malloc0(nullptr), II_g_realloc(nullptr), II_g_try_malloc(nullptr), 
 II_g_try_malloc0(nullptr), II_g_try_realloc(nullptr), 
-II_g_free(nullptr), II_g_memdup(nullptr) {}
+II_g_free(nullptr), II_g_memdup(nullptr), II_g_malloc_n(nullptr), 
+II_g_malloc0_n(nullptr), II_g_realloc_n(nullptr), 
+II_g_try_malloc_n(nullptr), II_g_try_malloc0_n(nullptr), 
+II_g_try_realloc_n(nullptr) {}
 
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
@@ -241,7 +244,10 @@
  *II_if_nameindex, *II_if_freenameindex, *II_wcsdup,
  *II_win_wcsdup, *II_g_malloc, *II_g_malloc0, 
  *II_g_realloc, *II_g_try_malloc, *II_g_try_malloc0, 
- *II_g_try_realloc, *II_g_free, *II_g_memdup;
+ *II_g_try_realloc, *II_g_free, *II_g_memdup, 
+ *II_g_malloc_n, *II_g_malloc0_n, *II_g_realloc_n, 
+ *II_g_try_malloc_n, *II_g_try_malloc0_n, 
+ *II_g_try_realloc_n;
   mutable Optional KernelZeroFlagVal;
 
   void initIdentifierInfo(ASTContext ) const;
@@ -321,9 +327,12 @@
  bool ,
  bool ReturnsNullOnFailure = false) const;
 
-  ProgramStateRef ReallocMem(CheckerContext , const CallExpr *CE,
- bool FreesMemOnFailure,
- ProgramStateRef State) const;
+  ProgramStateRef ReallocMemAux(CheckerContext , const CallExpr *CE,
+bool FreesMemOnFailure,
+ProgramStateRef State, 
+bool SuffixWithN = false) const;
+  static SVal evalMulForBufferSize(CheckerContext , const Expr *Blocks,
+   const Expr *BlockBytes);
   static ProgramStateRef CallocMem(CheckerContext , const CallExpr *CE,
ProgramStateRef State);
 
@@ -569,6 +578,12 @@
   II_g_try_realloc = ("g_try_realloc");
   II_g_free = ("g_free");
   II_g_memdup = ("g_memdup");
+  II_g_malloc_n = ("g_malloc_n");
+  II_g_malloc0_n = ("g_malloc0_n");
+  II_g_realloc_n = ("g_realloc_n");
+  II_g_try_malloc_n = ("g_try_malloc_n");
+  II_g_try_malloc0_n = ("g_try_malloc0_n");
+  II_g_try_realloc_n = ("g_try_realloc_n");
 }
 
 bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext ) const {
@@ -617,7 +632,10 @@
   FunI == II_g_malloc || FunI == II_g_malloc0 || 
   FunI == II_g_realloc || FunI == II_g_try_malloc || 
   FunI == II_g_try_malloc0 || FunI == II_g_try_realloc ||
-  FunI == II_g_memdup)
+  FunI == II_g_memdup || FunI == II_g_malloc_n || 
+  FunI == II_g_malloc0_n || FunI == II_g_realloc_n || 
+  FunI == II_g_try_malloc_n || FunI == II_g_try_malloc0_n || 
+  FunI == II_g_try_realloc_n)
 return true;
 }
 
@@ -767,6 +785,17 @@
   return None;
 }
 
+SVal MallocChecker::evalMulForBufferSize(CheckerContext , const Expr *Blocks,
+ const Expr *BlockBytes) {
+  SValBuilder  = C.getSValBuilder();
+  SVal BlocksVal = C.getSVal(Blocks);
+  SVal BlockBytesVal = C.getSVal(BlockBytes);
+  ProgramStateRef State = C.getState();
+  SVal TotalSize = SB.evalBinOp(State, BO_Mul, BlocksVal, BlockBytesVal,
+SB.getContext().getSizeType());
+  return TotalSize;
+}
+
 void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext ) const {
   if (C.wasInlined)
 return;
@@ -813,10 +842,10 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
 } else if (FunI == II_realloc || FunI == II_g_realloc || 
FunI == II_g_try_realloc) {
-  State = ReallocMem(C, CE, false, State);
+  State = ReallocMemAux(C, CE, false, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_reallocf) {
-  State = ReallocMem(C, CE, true, State);
+  State = ReallocMemAux(C, CE, true, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_calloc) {
 

r301384 - [analyzer] Teach the MallocChecker about Glib API for two arguments

[Leslie Zhai via cfe-commits]

Author: xiangzhai
Date: Wed Apr 26 00:33:14 2017
New Revision: 301384

URL: http://llvm.org/viewvc/llvm-project?rev=301384=rev
Log:
[analyzer] Teach the MallocChecker about Glib API for two arguments

Reviewers: zaks.anna, NoQ, danielmarjamaki

Reviewed By: zaks.anna, NoQ, danielmarjamaki

Subscribers: cfe-commits, kalev, pwithnall

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/gmalloc.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=301384=301383=301384=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Apr 26 00:33:14 
2017
@@ -177,7 +177,10 @@ public:
 II_wcsdup(nullptr), II_win_wcsdup(nullptr), II_g_malloc(nullptr),
 II_g_malloc0(nullptr), II_g_realloc(nullptr), 
II_g_try_malloc(nullptr), 
 II_g_try_malloc0(nullptr), II_g_try_realloc(nullptr), 
-II_g_free(nullptr), II_g_memdup(nullptr) {}
+II_g_free(nullptr), II_g_memdup(nullptr), II_g_malloc_n(nullptr), 
+II_g_malloc0_n(nullptr), II_g_realloc_n(nullptr), 
+II_g_try_malloc_n(nullptr), II_g_try_malloc0_n(nullptr), 
+II_g_try_realloc_n(nullptr) {}
 
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
@@ -241,7 +244,10 @@ private:
  *II_if_nameindex, *II_if_freenameindex, *II_wcsdup,
  *II_win_wcsdup, *II_g_malloc, *II_g_malloc0, 
  *II_g_realloc, *II_g_try_malloc, *II_g_try_malloc0, 
- *II_g_try_realloc, *II_g_free, *II_g_memdup;
+ *II_g_try_realloc, *II_g_free, *II_g_memdup, 
+ *II_g_malloc_n, *II_g_malloc0_n, *II_g_realloc_n, 
+ *II_g_try_malloc_n, *II_g_try_malloc0_n, 
+ *II_g_try_realloc_n;
   mutable Optional KernelZeroFlagVal;
 
   void initIdentifierInfo(ASTContext ) const;
@@ -321,9 +327,12 @@ private:
  bool ,
  bool ReturnsNullOnFailure = false) const;
 
-  ProgramStateRef ReallocMem(CheckerContext , const CallExpr *CE,
- bool FreesMemOnFailure,
- ProgramStateRef State) const;
+  ProgramStateRef ReallocMemAux(CheckerContext , const CallExpr *CE,
+bool FreesMemOnFailure,
+ProgramStateRef State, 
+bool SuffixWithN = false) const;
+  static SVal evalMulForBufferSize(CheckerContext , const Expr *Blocks,
+   const Expr *BlockBytes);
   static ProgramStateRef CallocMem(CheckerContext , const CallExpr *CE,
ProgramStateRef State);
 
@@ -569,6 +578,12 @@ void MallocChecker::initIdentifierInfo(A
   II_g_try_realloc = ("g_try_realloc");
   II_g_free = ("g_free");
   II_g_memdup = ("g_memdup");
+  II_g_malloc_n = ("g_malloc_n");
+  II_g_malloc0_n = ("g_malloc0_n");
+  II_g_realloc_n = ("g_realloc_n");
+  II_g_try_malloc_n = ("g_try_malloc_n");
+  II_g_try_malloc0_n = ("g_try_malloc0_n");
+  II_g_try_realloc_n = ("g_try_realloc_n");
 }
 
 bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext ) const 
{
@@ -617,7 +632,10 @@ bool MallocChecker::isCMemFunction(const
   FunI == II_g_malloc || FunI == II_g_malloc0 || 
   FunI == II_g_realloc || FunI == II_g_try_malloc || 
   FunI == II_g_try_malloc0 || FunI == II_g_try_realloc ||
-  FunI == II_g_memdup)
+  FunI == II_g_memdup || FunI == II_g_malloc_n || 
+  FunI == II_g_malloc0_n || FunI == II_g_realloc_n || 
+  FunI == II_g_try_malloc_n || FunI == II_g_try_malloc0_n || 
+  FunI == II_g_try_realloc_n)
 return true;
 }
 
@@ -767,6 +785,17 @@ llvm::Optional MallocCh
   return None;
 }
 
+SVal MallocChecker::evalMulForBufferSize(CheckerContext , const Expr *Blocks,
+ const Expr *BlockBytes) {
+  SValBuilder  = C.getSValBuilder();
+  SVal BlocksVal = C.getSVal(Blocks);
+  SVal BlockBytesVal = C.getSVal(BlockBytes);
+  ProgramStateRef State = C.getState();
+  SVal TotalSize = SB.evalBinOp(State, BO_Mul, BlocksVal, BlockBytesVal,
+SB.getContext().getSizeType());
+  return TotalSize;
+}
+
 void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext ) const 
{
   if (C.wasInlined)
 return;
@@ -813,10 +842,10 @@ void MallocChecker::checkPostStmt(const
   State = ProcessZeroAllocation(C, CE, 0, State);
 } else if (FunI == II_realloc || FunI == II_g_realloc || 
  

r301382 - [Modules][ObjC] Check definition from canonical decl on designated initializers

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Wed Apr 26 00:06:20 2017
New Revision: 301382

URL: http://llvm.org/viewvc/llvm-project?rev=301382=rev
Log:
[Modules][ObjC] Check definition from canonical decl on designated initializers

Use definition from canonical decl when checking for designated
initializers. This is necessary since deserialization of a interface
might reuse the definition from the canonical one (see r281119).

rdar://problem/29360655

Added:
cfe/trunk/test/Modules/Inputs/objc-desig-init/
cfe/trunk/test/Modules/Inputs/objc-desig-init/A.h
cfe/trunk/test/Modules/Inputs/objc-desig-init/A2.h
cfe/trunk/test/Modules/Inputs/objc-desig-init/Base.h
cfe/trunk/test/Modules/Inputs/objc-desig-init/X.h
cfe/trunk/test/Modules/Inputs/objc-desig-init/module.modulemap
cfe/trunk/test/Modules/objc-designated-init-mod.m
Modified:
cfe/trunk/lib/AST/DeclObjC.cpp

Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=301382=301381=301382=diff
==
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Wed Apr 26 00:06:20 2017
@@ -539,9 +539,18 @@ void ObjCInterfaceDecl::getDesignatedIni
 
 bool ObjCInterfaceDecl::isDesignatedInitializer(Selector Sel,
   const ObjCMethodDecl **InitMethod) const 
{
+  bool HasCompleteDef = isThisDeclarationADefinition();
+  // During deserialization the data record for the ObjCInterfaceDecl could
+  // be made invariant by reusing the canonical decl. Take this into account
+  // when checking for the complete definition.
+  if (!HasCompleteDef && getCanonicalDecl()->hasDefinition() &&
+  getCanonicalDecl()->getDefinition() == getDefinition())
+HasCompleteDef = true;
+
   // Check for a complete definition and recover if not so.
-  if (!isThisDeclarationADefinition())
+  if (!HasCompleteDef)
 return false;
+
   if (data().ExternallyCompleted)
 LoadExternalDefinition();
 

Added: cfe/trunk/test/Modules/Inputs/objc-desig-init/A.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/objc-desig-init/A.h?rev=301382=auto
==
--- cfe/trunk/test/Modules/Inputs/objc-desig-init/A.h (added)
+++ cfe/trunk/test/Modules/Inputs/objc-desig-init/A.h Wed Apr 26 00:06:20 2017
@@ -0,0 +1 @@
+#import "A2.h"

Added: cfe/trunk/test/Modules/Inputs/objc-desig-init/A2.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/objc-desig-init/A2.h?rev=301382=auto
==
--- cfe/trunk/test/Modules/Inputs/objc-desig-init/A2.h (added)
+++ cfe/trunk/test/Modules/Inputs/objc-desig-init/A2.h Wed Apr 26 00:06:20 2017
@@ -0,0 +1,4 @@
+#import "Base.h"
+
+@interface A2 : Base
+@end

Added: cfe/trunk/test/Modules/Inputs/objc-desig-init/Base.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/objc-desig-init/Base.h?rev=301382=auto
==
--- cfe/trunk/test/Modules/Inputs/objc-desig-init/Base.h (added)
+++ cfe/trunk/test/Modules/Inputs/objc-desig-init/Base.h Wed Apr 26 00:06:20 
2017
@@ -0,0 +1,4 @@
+@class NSString;
+@interface Base
+- (id)initWithNibName:(NSString *)nibNameOrNil 
__attribute__((objc_designated_initializer));
+@end

Added: cfe/trunk/test/Modules/Inputs/objc-desig-init/X.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/objc-desig-init/X.h?rev=301382=auto
==
--- cfe/trunk/test/Modules/Inputs/objc-desig-init/X.h (added)
+++ cfe/trunk/test/Modules/Inputs/objc-desig-init/X.h Wed Apr 26 00:06:20 2017
@@ -0,0 +1,4 @@
+#import "A2.h"
+
+@interface X : A2
+@end

Added: cfe/trunk/test/Modules/Inputs/objc-desig-init/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/objc-desig-init/module.modulemap?rev=301382=auto
==
--- cfe/trunk/test/Modules/Inputs/objc-desig-init/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/objc-desig-init/module.modulemap Wed Apr 26 
00:06:20 2017
@@ -0,0 +1,9 @@
+module Base {
+  header "Base.h"
+  export *
+}
+
+module A {
+  header "A.h"
+  export *
+}

Added: cfe/trunk/test/Modules/objc-designated-init-mod.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/objc-designated-init-mod.m?rev=301382=auto
==
--- cfe/trunk/test/Modules/objc-designated-init-mod.m (added)
+++ cfe/trunk/test/Modules/objc-designated-init-mod.m Wed Apr 26 00:06:20 2017
@@ -0,0 +1,17 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I 
%S/Inputs/objc-desig-init %s 

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki accepted this revision.
danielmarjamaki added a comment.
This revision is now accepted and ready to land.

If you have svn write permission then please do it.

If you need svn write permission, please see 
http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


Repository:
  rL LLVM

https://reviews.llvm.org/D30771



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

[PATCH] D32519: [Sema] Avoid using a null type pointer (fixes PR32750)

[Nikola Smiljanić via Phabricator via cfe-commits]

nikola accepted this revision.
nikola added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: test/SemaCXX/MicrosoftExtensions.cpp:516
+template struct A {};
+template struct B : A { A::C::D d; }; // expected-error 
{{missing 'typename' prior to dependent type name 'A::C::D'}}
+}

nitpick: you don't need the space between angle brackets since you set the 
standard to C++14, I'm not sure what the convention here is but you don't need 
more than C++11...


https://reviews.llvm.org/D32519



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

[PATCH] D32519: [Sema] Avoid using a null type pointer (fixes PR32750)

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.

isMicrosoftMissingTypename() uses a Type pointer without first checking 
  
that it's non-null. PR32750 reports a case where the pointer is in fact 
  
null. This patch adds in a defensive check and a regression test.


https://reviews.llvm.org/D32519

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/MicrosoftExtensions.cpp


Index: test/SemaCXX/MicrosoftExtensions.cpp
===
--- test/SemaCXX/MicrosoftExtensions.cpp
+++ test/SemaCXX/MicrosoftExtensions.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -std=c++98 %s -triple i686-pc-win32 -fsyntax-only 
-Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions 
-fexceptions -fcxx-exceptions -DTEST1
 // RUN: %clang_cc1 -std=c++11 %s -triple i686-pc-win32 -fsyntax-only 
-Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions 
-fexceptions -fcxx-exceptions -DTEST1
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft 
-Wc++11-extensions -Wno-long-long -verify -fexceptions -fcxx-exceptions -DTEST2
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++14 
-fms-compatibility -verify -DTEST3
 
 #if TEST1
 
@@ -508,6 +509,13 @@
 // Check that __unaligned is not recognized if MS extensions are not enabled
 typedef char __unaligned *aligned_type; // expected-error {{expected ';' after 
top level declarator}}
 
+#elif TEST3
+
+namespace PR32750 {
+template struct A {};
+template struct B : A { A::C::D d; }; // expected-error 
{{missing 'typename' prior to dependent type name 'A::C::D'}}
+}
+
 #else
 
 #error Unknown test mode
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -615,7 +615,7 @@
 
 CXXRecordDecl *RD = cast(CurContext);
 for (const auto  : RD->bases())
-  if (Context.hasSameUnqualifiedType(QualType(Ty, 1), Base.getType()))
+  if (Ty && Context.hasSameUnqualifiedType(QualType(Ty, 1), 
Base.getType()))
 return true;
 return S->isFunctionPrototypeScope();
   }


Index: test/SemaCXX/MicrosoftExtensions.cpp
===
--- test/SemaCXX/MicrosoftExtensions.cpp
+++ test/SemaCXX/MicrosoftExtensions.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -std=c++98 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
 // RUN: %clang_cc1 -std=c++11 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fms-extensions -fexceptions -fcxx-exceptions -DTEST1
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -Wmicrosoft -Wc++11-extensions -Wno-long-long -verify -fexceptions -fcxx-exceptions -DTEST2
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++14 -fms-compatibility -verify -DTEST3
 
 #if TEST1
 
@@ -508,6 +509,13 @@
 // Check that __unaligned is not recognized if MS extensions are not enabled
 typedef char __unaligned *aligned_type; // expected-error {{expected ';' after top level declarator}}
 
+#elif TEST3
+
+namespace PR32750 {
+template struct A {};
+template struct B : A { A::C::D d; }; // expected-error {{missing 'typename' prior to dependent type name 'A::C::D'}}
+}
+
 #else
 
 #error Unknown test mode
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -615,7 +615,7 @@
 
 CXXRecordDecl *RD = cast(CurContext);
 for (const auto  : RD->bases())
-  if (Context.hasSameUnqualifiedType(QualType(Ty, 1), Base.getType()))
+  if (Ty && Context.hasSameUnqualifiedType(QualType(Ty, 1), Base.getType()))
 return true;
 return S->isFunctionPrototypeScope();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r301377 - [ubsan] Skip alignment checks on allocas with known alignment

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Tue Apr 25 21:17:21 2017
New Revision: 301377

URL: http://llvm.org/viewvc/llvm-project?rev=301377=rev
Log:
[ubsan] Skip alignment checks on allocas with known alignment

It's possible to determine the alignment of an alloca at compile-time.
Use this information to skip emitting some runtime alignment checks.

Testing: check-clang, check-ubsan.

This significantly reduces the amount of alignment checks we emit when
compiling X86ISelLowering.cpp. Here are the numbers from patched/unpatched
clangs based on r301361.

  --
  | Setup  | # of alignment checks |
  --
  | unpatched, -O0 | 47195 |
  | patched, -O0   | 30876 | (-34.6%)
  --

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/test/CodeGen/catch-undef-behavior.c
cfe/trunk/test/CodeGen/sanitize-recover.c
cfe/trunk/test/CodeGenCXX/ubsan-suppress-checks.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=301377=301376=301377=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Apr 25 21:17:21 2017
@@ -533,15 +533,6 @@ bool CodeGenFunction::sanitizePerformTyp
  SanOpts.has(SanitizerKind::Vptr);
 }
 
-/// Check if a runtime null check for \p Ptr can be omitted.
-static bool canOmitPointerNullCheck(llvm::Value *Ptr) {
-  // Note: do not perform any constant-folding in this function. That is best
-  // left to the IR builder.
-
-  // Pointers to alloca'd memory are non-null.
-  return isa(Ptr->stripPointerCastsNoFollowAliases());
-}
-
 void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
 llvm::Value *Ptr, QualType Ty,
 CharUnits Alignment,
@@ -560,11 +551,16 @@ void CodeGenFunction::EmitTypeCheck(Type
   SmallVector Checks;
   llvm::BasicBlock *Done = nullptr;
 
+  // Quickly determine whether we have a pointer to an alloca. It's possible
+  // to skip null checks, and some alignment checks, for these pointers. This
+  // can reduce compile-time significantly.
+  auto PtrToAlloca =
+  dyn_cast(Ptr->stripPointerCastsNoFollowAliases());
+
   bool AllowNullPointers = TCK == TCK_DowncastPointer || TCK == TCK_Upcast ||
TCK == TCK_UpcastToVirtualBase;
   if ((SanOpts.has(SanitizerKind::Null) || AllowNullPointers) &&
-  !SkippedChecks.has(SanitizerKind::Null) &&
-  !canOmitPointerNullCheck(Ptr)) {
+  !SkippedChecks.has(SanitizerKind::Null) && !PtrToAlloca) {
 // The glvalue must not be an empty glvalue.
 llvm::Value *IsNonNull = Builder.CreateIsNotNull(Ptr);
 
@@ -617,7 +613,8 @@ void CodeGenFunction::EmitTypeCheck(Type
   AlignVal = getContext().getTypeAlignInChars(Ty).getQuantity();
 
 // The glvalue must be suitably aligned.
-if (AlignVal > 1) {
+if (AlignVal > 1 &&
+(!PtrToAlloca || PtrToAlloca->getAlignment() < AlignVal)) {
   llvm::Value *Align =
   Builder.CreateAnd(Builder.CreatePtrToInt(Ptr, IntPtrTy),
 llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));

Modified: cfe/trunk/test/CodeGen/catch-undef-behavior.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/catch-undef-behavior.c?rev=301377=301376=301377=diff
==
--- cfe/trunk/test/CodeGen/catch-undef-behavior.c (original)
+++ cfe/trunk/test/CodeGen/catch-undef-behavior.c Tue Apr 25 21:17:21 2017
@@ -36,13 +36,7 @@ void foo() {
 
   // CHECK-COMMON:  %[[I8PTR:.*]] = bitcast i32* %[[PTR:.*]] to i8*
   // CHECK-COMMON-NEXT: %[[SIZE:.*]] = call i64 @llvm.objectsize.i64.p0i8(i8* 
%[[I8PTR]], i1 false, i1 false)
-  // CHECK-COMMON-NEXT: %[[CHECK0:.*]] = icmp uge i64 %[[SIZE]], 4
-
-  // CHECK-COMMON:  %[[PTRTOINT:.*]] = ptrtoint {{.*}}* %[[PTR]] to i64
-  // CHECK-COMMON-NEXT: %[[MISALIGN:.*]] = and i64 %[[PTRTOINT]], 3
-  // CHECK-COMMON-NEXT: %[[CHECK1:.*]] = icmp eq i64 %[[MISALIGN]], 0
-
-  // CHECK-COMMON:   %[[OK:.*]] = and i1 %[[CHECK0]], %[[CHECK1]]
+  // CHECK-COMMON-NEXT: %[[OK:.*]] = icmp uge i64 %[[SIZE]], 4
 
   // CHECK-UBSAN: br i1 %[[OK]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
   // CHECK-TRAP:  br i1 %[[OK]], {{.*}}

Modified: cfe/trunk/test/CodeGen/sanitize-recover.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sanitize-recover.c?rev=301377=301376=301377=diff
==
--- cfe/trunk/test/CodeGen/sanitize-recover.c (original)
+++ cfe/trunk/test/CodeGen/sanitize-recover.c Tue Apr 25 21:17:21 2017
@@ -22,15 +22,7 @@ void foo() {
   // PARTIAL:  %[[SIZE:.*]] = call i64 

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai added a comment.

Hi Artem,

Could I commit this patch? thanks!

Regards,
Leslie Zhai


Repository:
  rL LLVM

https://reviews.llvm.org/D30771



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

[PATCH] D32514: [asan] Unconditionally enable GC of globals on COFF

[Evgeniy Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

r301374


Repository:
  rL LLVM

https://reviews.llvm.org/D32514



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

r301374 - [asan] Unconditionally enable GC of globals on COFF.

[Evgeniy Stepanov via cfe-commits]

Author: eugenis
Date: Tue Apr 25 19:51:06 2017
New Revision: 301374

URL: http://llvm.org/viewvc/llvm-project?rev=301374=rev
Log:
[asan] Unconditionally enable GC of globals on COFF.

This change restores pre-r301225 behavior, where linker GC compatible global
instrumentation was used on COFF targets disregarding -f(no-)data-sections 
and/or
/Gw flags.

This instrumentation puts each global in a COMDAT with an ASan descriptor for 
that global.
It effectively enables -fdata-sections, but limits it to ASan-instrumented 
globals.

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/CodeGen/asan-globals-gc.cpp

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=301374=301373=301374=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Apr 25 19:51:06 2017
@@ -196,9 +196,8 @@ static void addSanitizerCoveragePass(con
 static bool asanUseGlobalsGC(const Triple , const CodeGenOptions ) {
   switch (T.getObjectFormat()) {
   case Triple::MachO:
-return true;
   case Triple::COFF:
-return CGOpts.DataSections;
+return true;
   case Triple::ELF:
 return CGOpts.DataSections && !CGOpts.DisableIntegratedAS;
   default:

Modified: cfe/trunk/test/CodeGen/asan-globals-gc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/asan-globals-gc.cpp?rev=301374=301373=301374=diff
==
--- cfe/trunk/test/CodeGen/asan-globals-gc.cpp (original)
+++ cfe/trunk/test/CodeGen/asan-globals-gc.cpp Tue Apr 25 19:51:06 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc %s | FileCheck %s --check-prefix=WITHOUT-GC
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc %s | FileCheck %s --check-prefix=WITH-GC
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc -fdata-sections %s | FileCheck %s --check-prefix=WITH-GC
 
 int global;


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

Re: [PATCH] D32514: [asan] Unconditionally enable GC of globals on COFF

[Nico Weber via cfe-commits]

See https://bugs.chromium.org/p/chromium/issues/detail?id=715315

Looks good, please land to get the tree back green while we investigate.

On Apr 25, 2017 8:51 PM, "Evgeniy Stepanov via Phabricator via cfe-commits"
 wrote:

> eugenis added a comment.
>
> Apparently the ODR detector in Asan on Windows has issues with full
> data-sections. This way we can have limited GC for user globals.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D32514
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32514: [asan] Unconditionally enable GC of globals on COFF

[Evgeniy Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Apparently the ODR detector in Asan on Windows has issues with full 
data-sections. This way we can have limited GC for user globals.


Repository:
  rL LLVM

https://reviews.llvm.org/D32514



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

[PATCH] D32515: [libcxx] [test] Changes to accommodate LWG 2904 "Make variant move-assignment more exception safe"

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter created this revision.

NOTE: TEST CHANGES ONLY.

These tests will not pass with the current implementation of `variant`, but 
should give the implementor of LWG2904 a head start.

Details:

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp:

- Make `CopyAssign`'s move operations noexcept so uses in variants continue to 
prefer copy-and-move over direct construction.
- Fix `makeEmpty`: Copy assignment syntax no longer invokes `MakeEmptyT`'s 
throwing move constructor, move assignment syntax still does.
- `test_copy_assignment_sfinae`: `variant` is now copy 
assignable.
- `test_copy_assignment_different_index`:
  - `CopyThrows` and `MoveThrows` have potentially-throwing move construction, 
so they are now copy constructed directly into variants instead of via indirect 
copy-and-move.
  - implement new `CopyCannotThrow` and verify that direct copy construction of 
such in variants is preferred over indirect copy-and-move when neither method 
throws.

test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp:

- `test_move_assignment_sfinae`: `variant` is now move 
assignable.

test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp:

- `test_T_assignment_performs_construction`:
  - assigning `ThrowsCtorT` into a variant now constructs a temporary and moves 
into the variant; the variant keeps its old value if the temporary construction 
throws.
  - test that direct construction is preferred to temporary-and-move when 
neither can throw.
  - test that direct construction is preferred to temporary-and-move when both 
can throw.

test/support/variant_test_helpers.hpp:
test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp:
test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp:

- Fix `makeEmpty`: Copy assignment syntax no longer invokes `MakeEmptyT`'s 
throwing move constructor, move assignment syntax still does.


https://reviews.llvm.org/D32515

Files:
  test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
  test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
  test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp
  test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp
  test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp
  test/support/variant_test_helpers.hpp

Index: test/support/variant_test_helpers.hpp
===
--- test/support/variant_test_helpers.hpp
+++ test/support/variant_test_helpers.hpp
@@ -69,9 +69,9 @@
 void makeEmpty(Variant& v) {
 Variant v2(std::in_place_type);
 try {
-v = v2;
+v = std::move(v2);
 assert(false);
-}  catch (...) {
+} catch (...) {
 assert(v.valueless_by_exception());
 }
 }
Index: test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp
===
--- test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp
+++ test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp
@@ -65,7 +65,7 @@
 template  void makeEmpty(Variant ) {
   Variant v2(std::in_place_type);
   try {
-v = v2;
+v = std::move(v2);
 assert(false);
   } catch (...) {
 assert(v.valueless_by_exception());
Index: test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp
===
--- test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp
+++ test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp
@@ -63,7 +63,7 @@
 template  void makeEmpty(Variant ) {
   Variant v2(std::in_place_type);
   try {
-v = v2;
+v = std::move(v2);
 assert(false);
   } catch (...) {
 assert(v.valueless_by_exception());
Index: test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp
===
--- test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp
+++ test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp
@@ -117,10 +117,8 @@
 static_assert(std::is_move_assignable::value, "");
   }
   {
-// variant only provides move assignment when both the move constructor
-// and move assignment operator are well formed.
 using V = std::variant;
-static_assert(!std::is_move_assignable::value, "");
+static_assert(std::is_move_assignable::value, "");
   }
   {
 using V = std::variant;
Index: test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
===
--- test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
+++ test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp
@@ -66,7 +66,7 @@
 ++alive;
 ++copy_construct;
   }
-  CopyAssign(CopyAssign 

[PATCH] D32514: [asan] Unconditionally enable GC of globals on COFF

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Why?


Repository:
  rL LLVM

https://reviews.llvm.org/D32514



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

[PATCH] D32514: [asan] Unconditionally enable GC of globals on COFF

[Evgeniy Stepanov via Phabricator via cfe-commits]

eugenis created this revision.

This change restores pre-r301225 behavior, where linker GC compatible global
instrumentation was used on COFF targets disregarding -f(no-)data-sections 
and/or
/Gw flags.


Repository:
  rL LLVM

https://reviews.llvm.org/D32514

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGen/asan-globals-gc.cpp


Index: test/CodeGen/asan-globals-gc.cpp
===
--- test/CodeGen/asan-globals-gc.cpp
+++ test/CodeGen/asan-globals-gc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc %s | FileCheck %s --check-prefix=WITHOUT-GC
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc %s | FileCheck %s --check-prefix=WITH-GC
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple 
x86_64-windows-msvc -fdata-sections %s | FileCheck %s --check-prefix=WITH-GC
 
 int global;
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -196,9 +196,8 @@
 static bool asanUseGlobalsGC(const Triple , const CodeGenOptions ) {
   switch (T.getObjectFormat()) {
   case Triple::MachO:
-return true;
   case Triple::COFF:
-return CGOpts.DataSections;
+return true;
   case Triple::ELF:
 return CGOpts.DataSections && !CGOpts.DisableIntegratedAS;
   default:


Index: test/CodeGen/asan-globals-gc.cpp
===
--- test/CodeGen/asan-globals-gc.cpp
+++ test/CodeGen/asan-globals-gc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-windows-msvc %s | FileCheck %s --check-prefix=WITHOUT-GC
+// RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-windows-msvc %s | FileCheck %s --check-prefix=WITH-GC
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-windows-msvc -fdata-sections %s | FileCheck %s --check-prefix=WITH-GC
 
 int global;
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -196,9 +196,8 @@
 static bool asanUseGlobalsGC(const Triple , const CodeGenOptions ) {
   switch (T.getObjectFormat()) {
   case Triple::MachO:
-return true;
   case Triple::COFF:
-return CGOpts.DataSections;
+return true;
   case Triple::ELF:
 return CGOpts.DataSections && !CGOpts.DisableIntegratedAS;
   default:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32389: [libclang] Expose some target information via the C API.

[Emilio Cobos Álvarez via Phabricator via cfe-commits]

emilio updated this revision to Diff 96652.
emilio added a comment.

Updated per comments, I used `clang_TargetInfo_dispose` following recent APIs 
instead of `clang_disposeTargetInfo`, let me know if I should change that.

Also, didn't add a new header for `CXTargetInfo`, since it's rather small, but 
can definitely do if needed.


Repository:
  rL LLVM

https://reviews.llvm.org/D32389

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/target-info.c
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXTranslationUnit.h
  clang/tools/libclang/libclang.exports

Index: clang/tools/libclang/libclang.exports
===
--- clang/tools/libclang/libclang.exports
+++ clang/tools/libclang/libclang.exports
@@ -79,6 +79,9 @@
 clang_TParamCommandComment_isParamPositionValid
 clang_TParamCommandComment_getDepth
 clang_TParamCommandComment_getIndex
+clang_TargetInfo_dispose
+clang_TargetInfo_getPointerWidth
+clang_TargetInfo_getTriple
 clang_Type_getAlignOf
 clang_Type_getClassType
 clang_Type_getSizeOf
@@ -250,6 +253,7 @@
 clang_getTokenSpelling
 clang_getTranslationUnitCursor
 clang_getTranslationUnitSpelling
+clang_getTranslationUnitTargetInfo
 clang_getTypeDeclaration
 clang_getTypeKindSpelling
 clang_getTypeSpelling
Index: clang/tools/libclang/CXTranslationUnit.h
===
--- clang/tools/libclang/CXTranslationUnit.h
+++ clang/tools/libclang/CXTranslationUnit.h
@@ -35,6 +35,10 @@
   clang::index::CommentToXMLConverter *CommentToXML;
 };
 
+struct CXTargetInfoImpl {
+  CXTranslationUnit TranslationUnit;
+};
+
 namespace clang {
 namespace cxtu {
 
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -26,6 +26,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticCategories.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -4015,6 +4016,48 @@
   return MakeCXCursor(CXXUnit->getASTContext().getTranslationUnitDecl(), TU);
 }
 
+CXTargetInfo clang_getTranslationUnitTargetInfo(CXTranslationUnit CTUnit) {
+  if (isNotUsableTU(CTUnit)) {
+LOG_BAD_TU(CTUnit);
+return nullptr;
+  }
+
+  CXTargetInfoImpl* impl = new CXTargetInfoImpl();
+  impl->TranslationUnit = CTUnit;
+  return impl;
+}
+
+CXString clang_TargetInfo_getTriple(CXTargetInfo TargetInfo) {
+  if (!TargetInfo)
+return cxstring::createEmpty();
+
+  CXTranslationUnit CTUnit = TargetInfo->TranslationUnit;
+  assert(!isNotUsableTU(CTUnit));
+
+  ASTUnit *CXXUnit = cxtu::getASTUnit(CTUnit);
+  std::string Triple =
+CXXUnit->getASTContext().getTargetInfo().getTriple().normalize();
+  return cxstring::createDup(Triple);
+}
+
+int clang_TargetInfo_getPointerWidth(CXTargetInfo TargetInfo) {
+  if (!TargetInfo)
+return -1;
+
+  CXTranslationUnit CTUnit = TargetInfo->TranslationUnit;
+  assert(!isNotUsableTU(CTUnit));
+
+  ASTUnit *CXXUnit = cxtu::getASTUnit(CTUnit);
+  return CXXUnit->getASTContext().getTargetInfo().getMaxPointerWidth();
+}
+
+void clang_TargetInfo_dispose(CXTargetInfo TargetInfo) {
+  if (!TargetInfo)
+return;
+
+  delete TargetInfo;
+}
+
 //===--===//
 // CXFile Operations.
 //===--===//
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -1560,6 +1560,51 @@
 }
 
 /**/
+/* Target information testing.*/
+/**/
+
+static int print_target_info(int argc, const char **argv) {
+  CXIndex Idx;
+  CXTranslationUnit TU;
+  CXTargetInfo TargetInfo;
+  CXString Triple;
+  const char *FileName;
+  enum CXErrorCode Err;
+  int PointerWidth;
+
+  if (argc == 0) {
+fprintf(stderr, "No filename specified\n");
+return 1;
+  }
+
+  FileName = argv[1];
+
+  Idx = clang_createIndex(0, 1);
+  Err = clang_parseTranslationUnit2(Idx, FileName, argv, argc, NULL, 0,
+getDefaultParsingOptions(), );
+  if (Err != CXError_Success) {
+fprintf(stderr, "Couldn't parse translation unit!\n");
+describeLibclangFailure(Err);
+clang_disposeIndex(Idx);
+return 1;
+  }
+
+  TargetInfo = clang_getTranslationUnitTargetInfo(TU);
+
+  Triple = clang_TargetInfo_getTriple(TargetInfo);
+  printf("TargetTriple: %s\n", clang_getCString(Triple));
+  

[PATCH] D32510: [libcxx] Fix C1XX implementation of DoNotOptimize

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter created this revision.

...in test_macros.h


https://reviews.llvm.org/D32510

Files:
  test/support/test_macros.h


Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -209,7 +209,8 @@
 #include 
 template 
 inline void DoNotOptimize(Tp const& value) {
-  const volatile void* volatile = __builtin_addressof(value);
+  const volatile void* volatile unused = __builtin_addressof(value);
+  static_cast(unused);
   _ReadWriteBarrier();
 }
 #endif


Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -209,7 +209,8 @@
 #include 
 template 
 inline void DoNotOptimize(Tp const& value) {
-  const volatile void* volatile = __builtin_addressof(value);
+  const volatile void* volatile unused = __builtin_addressof(value);
+  static_cast(unused);
   _ReadWriteBarrier();
 }
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[Nico Weber via cfe-commits]

On Tue, Apr 25, 2017 at 5:28 PM, David Majnemer 
wrote:

>
>
> On Tue, Apr 25, 2017 at 2:12 PM, Nico Weber  wrote:
>
>> On Tue, Apr 25, 2017 at 4:14 PM, David Majnemer > > wrote:
>>
>>>
>>>
>>> On Tue, Apr 25, 2017 at 11:42 AM, Nico Weber 
>>> wrote:
>>>
 On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer <
 david.majne...@gmail.com> wrote:

>
>
> On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber 
> wrote:
>
>> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer <
>> david.majne...@gmail.com> wrote:
>>
>>>
>>>
>>> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber 
>>> wrote:
>>>
 "Opting into the conforming mode, /permissive-, during the series
 of VS 2017 update is a commitment to keeping your code base clean and 
 to
 fixing non-conforming constructs we fix conformance issues in Visual 
 C++."
 [...] "By contrast /permissive- offers a useful conformance mode where
 input C++ code is interpreted according to ISO C++ rules but also 
 allows
 conforming extensions necessary to compile C++ on targets supported by
 Visual C++."

 I guess the second quote agrees with your interpretation.

 We already diag most of the things they already mention. The one
 thing we don't diag by default is Wmicrosoft-enum-forward-reference
 since that's only an Extension and not an ExtWarn. We don't expose
 -pedantic from clang-cl, so this seemed like a somewhat natural 
 mapping to
 me.

 Should /permissive- map to -Wmicrosoft instead and turn on the
 parts of -Wmicrosoft that are Extensions?

>>>
>>> Did you mean on or off?
>>>
>>
>> On.
>>
>>
>>> I think that their intent is that things like __declspec remain OK.
>>>
>>
>> Nothing in -Wmicrosoft warns on __declspec.
>>
>>
>>> They want to diagnose non-conforming extensions like crazy template
>>> stuff, bogus typedef syntax, bad main function definitions, etc.
>>>
>>
>> Right. The only thing it currently makes cl warn on that clang-cl
>> doesn't warn on by default is Wmicrosoft-enum-forward-reference,
>> which is an Extension warning, not an ExtWarn. So mapping /permissive- to
>> -Wmicrosoft would make clang-cl diagnose forward-declared enums like it
>> does with 2017 cl.
>>
>
> Ok, sounds like it diagnoses the same sorts of things. They diagnose
> as error though, I think we should too. What about
> -fdelayed-template-parsing? Shouldn't that be disabled?
>

 CL has added a /Zc:twoPhase for that (not yet released anywhere), and
 Hans added support for that to clang-cl a while ago. Some blog post (maybe
 the one I linked to?) says that they're thinking of possibly
 enabling /Zc:twoPhase when /permissive- is passed, but at the moment it's
 independent. (In part because /permissive- ships in VC2017 and /Zc:twoPhase
 hasn't been shipped yet).

>>>
>>> Ok.
>>>
>>>

 What's the advantage of making it an error?

>>>
>>> All the diagnostics they show in https://blogs.msdn.microsof
>>> t.com/vcblog/2016/11/16/permissive-switch/ diagnose as errors.
>>>
>>
>> But why should we match that? Having them be warnings seems like a better
>> user experience to me.
>>
>
> I think the thinking behind the flag is that they want folks to be able to
> make their code more standards compliant. I imagine that anybody who turns
> the flag on understands what they are getting themselves into.
>
> IMO, the biggest reason why it should match their diagnostic level is
> because builds could otherwise break if a project has /permissive- but
> clang-cl allows a build to continue while cl would not.
>

But you can just pass -Werror if you care about that, disable things you
don't like with -Wno-, etc. If we make /permissive- turn on some
ill-defined set of warnigns as errors, we now need to keep that list of
diagnostics closely synced up to what cl does, and it makes things harder
for both users and us, without a clear benefit.


>
>
>>
>>
>>>
>>>
 If it's a warning, you can pass -Werror separately if you want. And
 SFINAE'ing on this seems like asking for trouble.

>>>
>>> I said nothing about permitting SFINAE on these errors.
>>>
>>>


>
>
>>
>>
>>>
>>>
 Should we just ignore /permissive- and possibly make some of our
 -Wmicrosoft Extensions ExtWarns instead?

 On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
 david.majne...@gmail.com> wrote:

> -pedantic means "Issue all the warnings demanded by strict ISO C
> and ISO C++; reject all programs that use forbidden extensions, and 

[PATCH] D32385: [libcxx] Implement LWG 2900 "The copy and move constructors of optional are not constexpr"

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 96644.
CaseyCarter added a comment.

Assigning an empty optional to a non-empty optional needs to destroy the 
contained value, so we need to also require `is_trivially_destructible` to 
implement trivial assignments.


https://reviews.llvm.org/D32385

Files:
  include/optional
  test/std/utilities/optional/optional.object/optional.object.ctor/copy.pass.cpp
  test/std/utilities/optional/optional.object/optional.object.ctor/move.pass.cpp

Index: test/std/utilities/optional/optional.object/optional.object.ctor/move.pass.cpp
===
--- test/std/utilities/optional/optional.object/optional.object.ctor/move.pass.cpp
+++ test/std/utilities/optional/optional.object/optional.object.ctor/move.pass.cpp
@@ -10,7 +10,10 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// optional(optional&& rhs);
+// constexpr optional(const optional&& rhs);
+//   If is_trivially_move_constructible_v is true,
+//this constructor shall be a constexpr constructor.
+
 
 #include 
 #include 
@@ -131,6 +134,31 @@
 #endif
 }
 
+constexpr bool test_constexpr()
+{
+{
+using T = int;
+optional o1{};
+optional o2 = std::move(o1);
+static_cast(o2);
+optional o3{T{42}};
+optional o4 = std::move(o3);
+static_cast(o4);
+}
+{
+struct T {
+constexpr T(int) {}
+T(T&&) = default;
+};
+optional o1{};
+optional o2 = std::move(o1);
+static_cast(o2);
+optional o3{T{42}};
+optional o4 = std::move(o3);
+static_cast(o4);
+}
+return true;
+}
 
 int main()
 {
@@ -198,4 +226,7 @@
 {
 test_reference_extension();
 }
+{
+static_assert(test_constexpr(), "");
+}
 }
Index: test/std/utilities/optional/optional.object/optional.object.ctor/copy.pass.cpp
===
--- test/std/utilities/optional/optional.object/optional.object.ctor/copy.pass.cpp
+++ test/std/utilities/optional/optional.object/optional.object.ctor/copy.pass.cpp
@@ -10,7 +10,9 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// optional(const optional& rhs);
+// constexpr optional(const optional& rhs);
+//   If is_trivially_copy_constructible_v is true,
+//this constructor shall be a constexpr constructor.
 
 #include 
 #include 
@@ -104,6 +106,31 @@
 #endif
 }
 
+constexpr bool test_constexpr()
+{
+{
+using T = int;
+optional o1{};
+optional o2 = o1;
+static_cast(o2);
+optional o3{T{42}};
+optional o4 = o3;
+static_cast(o4);
+}
+{
+struct T {
+constexpr T(int) {}
+};
+optional o1{};
+optional o2 = o1;
+static_cast(o2);
+optional o3{T{42}};
+optional o4 = o3;
+static_cast(o4);
+}
+return true;
+}
+
 int main()
 {
 test();
@@ -152,4 +179,7 @@
 {
 test_reference_extension();
 }
+{
+static_assert(test_constexpr(), "");
+}
 }
Index: include/optional
===
--- include/optional
+++ include/optional
@@ -436,46 +436,122 @@
 }
 };
 
-template ::value>
-struct __optional_storage;
-
-template 
-struct __optional_storage<_Tp, true> : __optional_storage_base<_Tp>
+template ::value>
+struct __optional_copy_base : __optional_storage_base<_Tp>
 {
 using __optional_storage_base<_Tp>::__optional_storage_base;
 };
 
 template 
-struct __optional_storage<_Tp, false> : __optional_storage_base<_Tp>
+struct __optional_copy_base<_Tp, false> : __optional_storage_base<_Tp>
 {
-using value_type = _Tp;
 using __optional_storage_base<_Tp>::__optional_storage_base;
 
 _LIBCPP_INLINE_VISIBILITY
-__optional_storage() = default;
+__optional_copy_base() = default;
 
 _LIBCPP_INLINE_VISIBILITY
-__optional_storage(const __optional_storage& __opt)
+__optional_copy_base(const __optional_copy_base& __opt)
 {
 this->__construct_from(__opt);
 }
 
 _LIBCPP_INLINE_VISIBILITY
-__optional_storage(__optional_storage&& __opt)
+__optional_copy_base(__optional_copy_base&&) = default;
+_LIBCPP_INLINE_VISIBILITY
+__optional_copy_base& operator=(const __optional_copy_base&) = default;
+_LIBCPP_INLINE_VISIBILITY
+__optional_copy_base& operator=(__optional_copy_base&&) = default;
+};
+
+template ::value>
+struct __optional_move_base : __optional_copy_base<_Tp>
+{
+using __optional_copy_base<_Tp>::__optional_copy_base;
+};
+
+template 
+struct __optional_move_base<_Tp, false> : __optional_copy_base<_Tp>
+{
+using value_type = _Tp;
+using __optional_copy_base<_Tp>::__optional_copy_base;
+
+_LIBCPP_INLINE_VISIBILITY
+__optional_move_base() = default;
+_LIBCPP_INLINE_VISIBILITY
+__optional_move_base(const __optional_move_base&) = default;

[clang-tools-extra] r301365 - [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

[Jakub Kuderski via cfe-commits]

Author: kuhar
Date: Tue Apr 25 17:38:39 2017
New Revision: 301365

URL: http://llvm.org/viewvc/llvm-project?rev=301365=rev
Log:
[clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

Summary:
When running run-clang-tidy.py with -fix it tries to apply found replacements 
at the end.
If there are errors running clang-apply-replacements, the script currently 
crashes or displays no error at all.

This patch checks for errors running clang-apply-replacements the same way 
clang-tidy binary is handled.

Another option would be probably checking for clang-apply-replacements (when 
-fix is passed) even before running clang-tidy.

Reviewers: Prazek, alexfh, bkramer, mfherbst

Reviewed By: Prazek, alexfh

Subscribers: kimgr, cfe-commits

Tags: #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=301365=301364=301365=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Tue Apr 25 
17:38:39 2017
@@ -34,6 +34,7 @@ Compilation database setup:
 http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
 """
 
+from __future__ import print_function
 import argparse
 import json
 import multiprocessing
@@ -45,6 +46,7 @@ import subprocess
 import sys
 import tempfile
 import threading
+import traceback
 
 
 def find_compilation_database(path):
@@ -52,7 +54,7 @@ def find_compilation_database(path):
   result = './'
   while not os.path.isfile(os.path.join(result, path)):
 if os.path.realpath(result) == '/':
-  print 'Error: could not find compilation database.'
+  print('Error: could not find compilation database.')
   sys.exit(1)
 result += '../'
   return os.path.realpath(result)
@@ -87,6 +89,17 @@ def get_tidy_invocation(f, clang_tidy_bi
   return start
 
 
+def check_clang_apply_replacements_binary(args):
+  """Checks if invoking supplied clang-apply-replacements binary works."""
+  try:
+subprocess.check_call([args.clang_apply_replacements_binary, '--version'])
+  except:
+print('Unable to run clang-apply-replacements. Is clang-apply-replacements 
'
+  'binary correctly specified?', file=sys.stderr)
+traceback.print_exc()
+sys.exit(1)
+
+
 def apply_fixes(args, tmpdir):
   """Calls clang-apply-fixes on a given directory. Deletes the dir when 
done."""
   invocation = [args.clang_apply_replacements_binary]
@@ -94,7 +107,6 @@ def apply_fixes(args, tmpdir):
 invocation.append('-format')
   invocation.append(tmpdir)
   subprocess.call(invocation)
-  shutil.rmtree(tmpdir)
 
 
 def run_tidy(args, tmpdir, build_path, queue):
@@ -164,9 +176,9 @@ def main():
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-print subprocess.check_output(invocation)
+print(subprocess.check_output(invocation))
   except:
-print >>sys.stderr, "Unable to run clang-tidy."
+print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)
 
   # Load the database and extract all files.
@@ -179,6 +191,7 @@ def main():
 
   tmpdir = None
   if args.fix:
+check_clang_apply_replacements_binary(args)
 tmpdir = tempfile.mkdtemp()
 
   # Build up a big regexy filter from all command line arguments.
@@ -204,14 +217,25 @@ def main():
   except KeyboardInterrupt:
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
-print '\nCtrl-C detected, goodbye.'
+print('\nCtrl-C detected, goodbye.')
 if args.fix:
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
   if args.fix:
-print 'Applying fixes ...'
-apply_fixes(args, tmpdir)
+print('Applying fixes ...')
+successfully_applied = False
+
+try:
+  apply_fixes(args, tmpdir)
+  successfully_applied = True
+except:
+  print('Error applying fixes.\n', file=sys.stderr)
+  traceback.print_exc()
+
+shutil.rmtree(tmpdir)
+if not successfully_applied:
+  sys.exit(1)
 
 if __name__ == '__main__':
   main()


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

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

[Jakub Kuderski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL301365: [clang-tidy] run-clang-tidy.py: check if 
clang-apply-replacements succeeds (authored by kuhar).

Changed prior to commit:
  https://reviews.llvm.org/D32294?vs=96097=96642#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32294

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -34,6 +34,7 @@
 http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
 """
 
+from __future__ import print_function
 import argparse
 import json
 import multiprocessing
@@ -45,14 +46,15 @@
 import sys
 import tempfile
 import threading
+import traceback
 
 
 def find_compilation_database(path):
   """Adjusts the directory until a compilation database is found."""
   result = './'
   while not os.path.isfile(os.path.join(result, path)):
 if os.path.realpath(result) == '/':
-  print 'Error: could not find compilation database.'
+  print('Error: could not find compilation database.')
   sys.exit(1)
 result += '../'
   return os.path.realpath(result)
@@ -87,14 +89,24 @@
   return start
 
 
+def check_clang_apply_replacements_binary(args):
+  """Checks if invoking supplied clang-apply-replacements binary works."""
+  try:
+subprocess.check_call([args.clang_apply_replacements_binary, '--version'])
+  except:
+print('Unable to run clang-apply-replacements. Is clang-apply-replacements '
+  'binary correctly specified?', file=sys.stderr)
+traceback.print_exc()
+sys.exit(1)
+
+
 def apply_fixes(args, tmpdir):
   """Calls clang-apply-fixes on a given directory. Deletes the dir when done."""
   invocation = [args.clang_apply_replacements_binary]
   if args.format:
 invocation.append('-format')
   invocation.append(tmpdir)
   subprocess.call(invocation)
-  shutil.rmtree(tmpdir)
 
 
 def run_tidy(args, tmpdir, build_path, queue):
@@ -164,9 +176,9 @@
 if args.checks:
   invocation.append('-checks=' + args.checks)
 invocation.append('-')
-print subprocess.check_output(invocation)
+print(subprocess.check_output(invocation))
   except:
-print >>sys.stderr, "Unable to run clang-tidy."
+print("Unable to run clang-tidy.", file=sys.stderr)
 sys.exit(1)
 
   # Load the database and extract all files.
@@ -179,6 +191,7 @@
 
   tmpdir = None
   if args.fix:
+check_clang_apply_replacements_binary(args)
 tmpdir = tempfile.mkdtemp()
 
   # Build up a big regexy filter from all command line arguments.
@@ -204,14 +217,25 @@
   except KeyboardInterrupt:
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
-print '\nCtrl-C detected, goodbye.'
+print('\nCtrl-C detected, goodbye.')
 if args.fix:
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
   if args.fix:
-print 'Applying fixes ...'
-apply_fixes(args, tmpdir)
+print('Applying fixes ...')
+successfully_applied = False
+
+try:
+  apply_fixes(args, tmpdir)
+  successfully_applied = True
+except:
+  print('Error applying fixes.\n', file=sys.stderr)
+  traceback.print_exc()
+
+shutil.rmtree(tmpdir)
+if not successfully_applied:
+  sys.exit(1)
 
 if __name__ == '__main__':
   main()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:118
+  let Content = [{
+``noescape`` placed on a block parameter is used to inform the compiler that 
the block passed to a function cannot escape: that is, the block will not be 
invoked after the function returns. To ensure that the block does not escape, 
clang imposes the following restrictions on usage of non-escaping blocks:
+

aaron.ballman wrote:
> Please wrap to 80 columns.
> 
> "block parameter" is a bit strange -- I assumed that meant parameter to a 
> block, not a function parameter of block type. May want to clarify.
> 
> Should also clarify "the block will not be invoked after the function 
> returns." Does this code produce undefined behavior?
> ```
>   typedef void (^BlockTy)();
>   void nonescapingFunc(__attribute__((noescape)) BlockTy);
>   void escapingFunc(BlockTy);
> 
>   void callerFunc(__attribute__((noescape)) BlockTy block) {
> nonescapingFunc(block); // OK
> escapingFunc(block);// error: parameter is not annotated with noescape
>   }
> 
>   void f(void) {
> BlockTy Blk = ^{};
> callerFunc(Blk);
> Blk();
>   }
> ```
The code above doesn't produce undefined behavior as long as escapingFunc 
doesn't cause the block to escape. It is OK to declare a variable to hold the 
block and invoke it later after callerFunc returns. If a block literal is 
directly passed to callerFunc, the block will not be invoked after the function 
returns.

```
void f(void) {
  BlockTy Blk = ^{};
  callerFunc(Blk); // Blk can be invoked after callerFunc returns.
  Blk();
  callerFunc(^{}); // the block passed cannot be invoked after callerFunc 
returns.
}```


https://reviews.llvm.org/D32210



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

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 96637.
ahatanak marked 4 inline comments as done.
ahatanak added a comment.

Address review comments.


https://reviews.llvm.org/D32210

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/SemaObjCXX/noescape.mm

Index: test/SemaObjCXX/noescape.mm
===
--- /dev/null
+++ test/SemaObjCXX/noescape.mm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -std=c++11 %s
+
+typedef void (^BlockTy)();
+
+void noescapeFunc0(id, __attribute__((noescape)) BlockTy);
+void noescapeFunc1(id, [[clang::noescape]] BlockTy);
+void invalidFunc(int __attribute__((noescape))); // expected-warning {{'noescape' attribute ignored on parameter of non-pointer type}}
+int __attribute__((noescape)) g; // expected-warning {{'noescape' attribute only applies to parameters}}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1516,6 +1516,19 @@
Attr.getAttributeSpellingListIndex()));
 }
 
+static void handleNoEscapeAttr(Sema , Decl *D, const AttributeList ) {
+  // noescape only applies to pointer types.
+  QualType T = cast(D)->getType();
+  if (!T->isAnyPointerType() && !T->isBlockPointerType() &&
+  !T->isReferenceType() && !T->isArrayType() && !T->isMemberPointerType()) {
+S.Diag(Attr.getLoc(), diag::warn_attribute_noescape_non_pointer) << T;
+return;
+  }
+
+  D->addAttr(::new (S.Context) NoEscapeAttr(
+  Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
+}
+
 static void handleAssumeAlignedAttr(Sema , Decl *D,
 const AttributeList ) {
   Expr *E = Attr.getArgAsExpr(0),
@@ -6040,6 +6053,9 @@
   case AttributeList::AT_ReturnsNonNull:
 handleReturnsNonNullAttr(S, D, Attr);
 break;
+  case AttributeList::AT_NoEscape:
+handleNoEscapeAttr(S, D, Attr);
+break;
   case AttributeList::AT_AssumeAligned:
 handleAssumeAlignedAttr(S, D, Attr);
 break;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3109,6 +3109,10 @@
   "code; pointer may be assumed to always convert to true">,
   InGroup;
 
+def warn_attribute_noescape_non_pointer : Warning<
+  "'noescape' attribute ignored on parameter of non-pointer type %0">,
+  InGroup;
+
 def warn_null_pointer_compare : Warning<
 "comparison of %select{address of|function|array}0 '%1' %select{not |}2"
 "equal to a null pointer is always %select{true|false}2">,
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -112,6 +112,40 @@
   }];
 }
 
+def NoEscapeDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+``noescape`` placed on a function parameter of block type is used to inform the
+compiler that the block cannot escape: that is, no reference to the block
+derived from the parameter value will survive after the function returns.
+
+For example:
+
+.. code-block:: c
+
+  typedef void (^BlockTy)();
+  BlockTy g;
+
+  void nonescapingFunc(__attribute__((noescape)) BlockTy block) {
+block(); // OK.
+  }
+
+  void escapingFunc(__attribute__((noescape)) BlockTy block) {
+g = block; // Not OK. g can be invoked after the function returns.
+  }
+
+  void caller() {
+escapingFunc(^{});
+g();
+  }
+
+The compiler may do optimizations or issue improved diagnostics using this
+knowledge. Users are responsible for making sure parameters annotated with
+``noescape`` do not actuallly escape.
+
+  }];
+}
+
 def CarriesDependencyDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1337,6 +1337,12 @@
   let Documentation = [Undocumented];
 }
 
+def NoEscape : InheritableAttr {
+  let Spellings = [GNU<"noescape">, CXX11<"clang", "noescape">];
+  let Subjects = SubjectList<[ParmVar]>;
+  let Documentation = [NoEscapeDocs];
+}
+
 def AssumeAligned : InheritableAttr {
   let Spellings = [GCC<"assume_aligned">];
   let Subjects = SubjectList<[ObjCMethod, Function]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

kuhar wrote:
> Prazek wrote:
> > JDevlieghere wrote:
> > > It's highly recommended to put some kind of error message in the 
> > > assertion statement, as per the LLVM coding standards.
> > would it be better to change it to
> > !innerCtorCall && !MakePairCall && "No .."
> I don't think that this logic would work here. `!first && !second` ensures 
> both are null.
Yea, I forgot how to De Morgan


https://reviews.llvm.org/D32395



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

Re: r289018 - [Headers] Enable #include_next on Darwin

[Bruno Cardoso Lopes via cfe-commits]

On Tue, Apr 25, 2017 at 2:34 PM, Aaron Ballman  wrote:
> On Tue, Apr 25, 2017 at 5:33 PM, Bruno Cardoso Lopes
>  wrote:
>> On Tue, Apr 25, 2017 at 6:29 AM, Aaron Ballman  
>> wrote:
>>> On Wed, Dec 7, 2016 at 9:13 PM, Bruno Cardoso Lopes via cfe-commits
>>>  wrote:
 Author: bruno
 Date: Wed Dec  7 20:13:56 2016
 New Revision: 289018

 URL: http://llvm.org/viewvc/llvm-project?rev=289018=rev
 Log:
 [Headers] Enable #include_next on Darwin

 Allows darwin targets to provide additional definitions and
 implementation specifc values for float.h

 rdar://problem/21961491

 Added:
 cfe/trunk/test/Headers/Inputs/usr/
 cfe/trunk/test/Headers/Inputs/usr/include/
 cfe/trunk/test/Headers/Inputs/usr/include/float.h
 cfe/trunk/test/Headers/float-darwin.c
 Modified:
 cfe/trunk/lib/Headers/float.h
>>>
>>> This commit appears to have caused a regression:
>>> https://bugs.llvm.org//show_bug.cgi?id=31504
>>>
>>> I am running into this on a Snow Leopard system as well, where the
>>> Integration tests are now failing because float.h cannot be found by
>>> the system #include_next.
>>
>> What's actually happening here is that Snow Leopard (or any < 10.7)
>> used to ship a /usr/include/float.h, which has by itself another
>> include_next (which is the one failing), see:
>>
>> --
>> /* This file exists soley to keep Metrowerks' compilers happy.  The version
>>used by GCC can be found in /usr/lib/gcc, although it's
>>not very informative.  */
>>
>> #ifndef _FLOAT_H_
>>
>> #if defined(__GNUC__)
>> #include_next 
>> --
>>
>> We don't have any reliable way to check for the SDK version at this
>> level, the current macros we have tell a history about deployment
>> target, but that won't always solve the issue here. Can you try this
>> workaround below and let me know if works?
>>
>> --
>> #if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
>>__STDC_HOSTED__ && __has_include_next()
>>
>> #ifdef __APPLE__
>> #define _FLOAT_H_ // Avoid #include_next'ing float.h content on MacOSX < 10.7
>> #endif
>>
>> #  include_next 
>> --
>>
>>> I was thinking of modifying lib/Headers/float.h to use the SDK version as 
>>> mentioned in the bug report, but I share the reporter's question: why was 
>>> this change needed in the first place? I couldn't find a review for the 
>>> commit, and don't have access to the rdar link.
>>
>> We need it to have flexibility to implement additional stuff for
>> float.h on future SDKs. There was no good reason for not upstreaming
>> it. I never got to the point of dealing with this specific issue
>> though, thanks for pinging.
>
> What good timing. I was about to post this as a possible fix:
>
> Index: lib/Headers/float.h
> ===
> --- lib/Headers/float.h (revision 135915)
> +++ lib/Headers/float.h (working copy)
> @@ -31 +31 @@
> - * Also fall back on Darwin to allow additional definitions and
> + * Also fall back on Darwin 10.7 or later to allow additional definitions and
> @@ -34 +34,4 @@
> -#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
> +#if ((defined(__APPLE__) &&
>  \
> +  defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) &&
>  \
> +  __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1070) ||
>  \
> + (defined(__MINGW32__) || defined(_MSC_VER))) &&
>  \
>
> Would this be an improvement over your suggestion?

This would be fine for most cases I believe, but if you're trying to
deploy to a target >= 1070 while using Snow Leopard you would still
get the error.

> ~Aaron
>
>>
>> --
>> Bruno Cardoso Lopes
>> http://www.brunocardoso.cc



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r301364 - [Hexagon] Handle -O4 when checking optimization level

[Krzysztof Parzyszek via cfe-commits]

Author: kparzysz
Date: Tue Apr 25 16:31:55 2017
New Revision: 301364

URL: http://llvm.org/viewvc/llvm-project?rev=301364=rev
Log:
[Hexagon] Handle -O4 when checking optimization level

Modified:
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=301364=301363=301364=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Tue Apr 25 16:31:55 2017
@@ -411,7 +411,8 @@ unsigned HexagonToolChain::getOptimizati
 
   if (A->getOption().matches(options::OPT_O0))
 return 0;
-  if (A->getOption().matches(options::OPT_Ofast))
+  if (A->getOption().matches(options::OPT_Ofast) ||
+  A->getOption().matches(options::OPT_O4))
 return 3;
   assert(A->getNumValues() != 0);
   StringRef S(A->getValue());


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

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar marked 2 inline comments as done.
kuhar added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

Prazek wrote:
> JDevlieghere wrote:
> > It's highly recommended to put some kind of error message in the assertion 
> > statement, as per the LLVM coding standards.
> would it be better to change it to
> !innerCtorCall && !MakePairCall && "No .."
I don't think that this logic would work here. `!first && !second` ensures both 
are null.


https://reviews.llvm.org/D32395



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

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96634.
kuhar added a comment.

Use igoringImplicit instead of ignoringParenImpCasts. Options moved to the 
anonymous namespace.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,51 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The examples below illustrate the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
+
+  struct Y {
+Y(std::pair&&) {}
+  };
+  std::vector y;
+  y.push_back(std::make_pair(2, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: y.emplace_back(std::make_pair(2, 3));
+  // make_pair cannot be removed here, as Y is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===
--- docs/clang-tidy/checks/modernize-use-emplace.rst
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -36,8 +36,7 @@
 
 std::vector> w;
 w.emplace_back(21, 37);
-// This will be fixed to w.emplace_back(21L, 37L); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+w.emplace_back(21L, 37L);
 
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,10 +15,16 @@
 namespace tidy {
 namespace modernize {
 
-static const auto DefaultContainersWithPushBack =
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+
+const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
-static const auto DefaultSmartPointers =
+const auto DefaultSmartPointers =
 "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+} // namespace
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, 

Re: r289018 - [Headers] Enable #include_next on Darwin

[Aaron Ballman via cfe-commits]

On Tue, Apr 25, 2017 at 5:33 PM, Bruno Cardoso Lopes
 wrote:
> On Tue, Apr 25, 2017 at 6:29 AM, Aaron Ballman  wrote:
>> On Wed, Dec 7, 2016 at 9:13 PM, Bruno Cardoso Lopes via cfe-commits
>>  wrote:
>>> Author: bruno
>>> Date: Wed Dec  7 20:13:56 2016
>>> New Revision: 289018
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=289018=rev
>>> Log:
>>> [Headers] Enable #include_next on Darwin
>>>
>>> Allows darwin targets to provide additional definitions and
>>> implementation specifc values for float.h
>>>
>>> rdar://problem/21961491
>>>
>>> Added:
>>> cfe/trunk/test/Headers/Inputs/usr/
>>> cfe/trunk/test/Headers/Inputs/usr/include/
>>> cfe/trunk/test/Headers/Inputs/usr/include/float.h
>>> cfe/trunk/test/Headers/float-darwin.c
>>> Modified:
>>> cfe/trunk/lib/Headers/float.h
>>
>> This commit appears to have caused a regression:
>> https://bugs.llvm.org//show_bug.cgi?id=31504
>>
>> I am running into this on a Snow Leopard system as well, where the
>> Integration tests are now failing because float.h cannot be found by
>> the system #include_next.
>
> What's actually happening here is that Snow Leopard (or any < 10.7)
> used to ship a /usr/include/float.h, which has by itself another
> include_next (which is the one failing), see:
>
> --
> /* This file exists soley to keep Metrowerks' compilers happy.  The version
>used by GCC can be found in /usr/lib/gcc, although it's
>not very informative.  */
>
> #ifndef _FLOAT_H_
>
> #if defined(__GNUC__)
> #include_next 
> --
>
> We don't have any reliable way to check for the SDK version at this
> level, the current macros we have tell a history about deployment
> target, but that won't always solve the issue here. Can you try this
> workaround below and let me know if works?
>
> --
> #if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
>__STDC_HOSTED__ && __has_include_next()
>
> #ifdef __APPLE__
> #define _FLOAT_H_ // Avoid #include_next'ing float.h content on MacOSX < 10.7
> #endif
>
> #  include_next 
> --
>
>> I was thinking of modifying lib/Headers/float.h to use the SDK version as 
>> mentioned in the bug report, but I share the reporter's question: why was 
>> this change needed in the first place? I couldn't find a review for the 
>> commit, and don't have access to the rdar link.
>
> We need it to have flexibility to implement additional stuff for
> float.h on future SDKs. There was no good reason for not upstreaming
> it. I never got to the point of dealing with this specific issue
> though, thanks for pinging.

What good timing. I was about to post this as a possible fix:

Index: lib/Headers/float.h
===
--- lib/Headers/float.h (revision 135915)
+++ lib/Headers/float.h (working copy)
@@ -31 +31 @@
- * Also fall back on Darwin to allow additional definitions and
+ * Also fall back on Darwin 10.7 or later to allow additional definitions and
@@ -34 +34,4 @@
-#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
+#if ((defined(__APPLE__) &&
 \
+  defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) &&
 \
+  __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1070) ||
 \
+ (defined(__MINGW32__) || defined(_MSC_VER))) &&
 \

Would this be an improvement over your suggestion?

~Aaron

>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r289018 - [Headers] Enable #include_next on Darwin

[Bruno Cardoso Lopes via cfe-commits]

On Tue, Apr 25, 2017 at 6:29 AM, Aaron Ballman  wrote:
> On Wed, Dec 7, 2016 at 9:13 PM, Bruno Cardoso Lopes via cfe-commits
>  wrote:
>> Author: bruno
>> Date: Wed Dec  7 20:13:56 2016
>> New Revision: 289018
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=289018=rev
>> Log:
>> [Headers] Enable #include_next on Darwin
>>
>> Allows darwin targets to provide additional definitions and
>> implementation specifc values for float.h
>>
>> rdar://problem/21961491
>>
>> Added:
>> cfe/trunk/test/Headers/Inputs/usr/
>> cfe/trunk/test/Headers/Inputs/usr/include/
>> cfe/trunk/test/Headers/Inputs/usr/include/float.h
>> cfe/trunk/test/Headers/float-darwin.c
>> Modified:
>> cfe/trunk/lib/Headers/float.h
>
> This commit appears to have caused a regression:
> https://bugs.llvm.org//show_bug.cgi?id=31504
>
> I am running into this on a Snow Leopard system as well, where the
> Integration tests are now failing because float.h cannot be found by
> the system #include_next.

What's actually happening here is that Snow Leopard (or any < 10.7)
used to ship a /usr/include/float.h, which has by itself another
include_next (which is the one failing), see:

--
/* This file exists soley to keep Metrowerks' compilers happy.  The version
   used by GCC can be found in /usr/lib/gcc, although it's
   not very informative.  */

#ifndef _FLOAT_H_

#if defined(__GNUC__)
#include_next 
--

We don't have any reliable way to check for the SDK version at this
level, the current macros we have tell a history about deployment
target, but that won't always solve the issue here. Can you try this
workaround below and let me know if works?

--
#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
   __STDC_HOSTED__ && __has_include_next()

#ifdef __APPLE__
#define _FLOAT_H_ // Avoid #include_next'ing float.h content on MacOSX < 10.7
#endif

#  include_next 
--

> I was thinking of modifying lib/Headers/float.h to use the SDK version as 
> mentioned in the bug report, but I share the reporter's question: why was 
> this change needed in the first place? I couldn't find a review for the 
> commit, and don't have access to the rdar link.

We need it to have flexibility to implement additional stuff for
float.h on future SDKs. There was no good reason for not upstreaming
it. I never got to the point of dealing with this specific issue
though, thanks for pinging.

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[David Majnemer via cfe-commits]

On Tue, Apr 25, 2017 at 2:12 PM, Nico Weber  wrote:

> On Tue, Apr 25, 2017 at 4:14 PM, David Majnemer 
> wrote:
>
>>
>>
>> On Tue, Apr 25, 2017 at 11:42 AM, Nico Weber  wrote:
>>
>>> On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer <
>>> david.majne...@gmail.com> wrote:
>>>


 On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber 
 wrote:

> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer <
> david.majne...@gmail.com> wrote:
>
>>
>>
>> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber 
>> wrote:
>>
>>> "Opting into the conforming mode, /permissive-, during the series of
>>> VS 2017 update is a commitment to keeping your code base clean and to
>>> fixing non-conforming constructs we fix conformance issues in Visual 
>>> C++."
>>> [...] "By contrast /permissive- offers a useful conformance mode where
>>> input C++ code is interpreted according to ISO C++ rules but also allows
>>> conforming extensions necessary to compile C++ on targets supported by
>>> Visual C++."
>>>
>>> I guess the second quote agrees with your interpretation.
>>>
>>> We already diag most of the things they already mention. The one
>>> thing we don't diag by default is Wmicrosoft-enum-forward-reference
>>> since that's only an Extension and not an ExtWarn. We don't expose
>>> -pedantic from clang-cl, so this seemed like a somewhat natural mapping 
>>> to
>>> me.
>>>
>>> Should /permissive- map to -Wmicrosoft instead and turn on the parts
>>> of -Wmicrosoft that are Extensions?
>>>
>>
>> Did you mean on or off?
>>
>
> On.
>
>
>> I think that their intent is that things like __declspec remain OK.
>>
>
> Nothing in -Wmicrosoft warns on __declspec.
>
>
>> They want to diagnose non-conforming extensions like crazy template
>> stuff, bogus typedef syntax, bad main function definitions, etc.
>>
>
> Right. The only thing it currently makes cl warn on that clang-cl
> doesn't warn on by default is Wmicrosoft-enum-forward-reference,
> which is an Extension warning, not an ExtWarn. So mapping /permissive- to
> -Wmicrosoft would make clang-cl diagnose forward-declared enums like it
> does with 2017 cl.
>

 Ok, sounds like it diagnoses the same sorts of things. They diagnose as
 error though, I think we should too. What about -fdelayed-template-parsing?
 Shouldn't that be disabled?

>>>
>>> CL has added a /Zc:twoPhase for that (not yet released anywhere), and
>>> Hans added support for that to clang-cl a while ago. Some blog post (maybe
>>> the one I linked to?) says that they're thinking of possibly
>>> enabling /Zc:twoPhase when /permissive- is passed, but at the moment it's
>>> independent. (In part because /permissive- ships in VC2017 and /Zc:twoPhase
>>> hasn't been shipped yet).
>>>
>>
>> Ok.
>>
>>
>>>
>>> What's the advantage of making it an error?
>>>
>>
>> All the diagnostics they show in https://blogs.msdn.microsof
>> t.com/vcblog/2016/11/16/permissive-switch/ diagnose as errors.
>>
>
> But why should we match that? Having them be warnings seems like a better
> user experience to me.
>

I think the thinking behind the flag is that they want folks to be able to
make their code more standards compliant. I imagine that anybody who turns
the flag on understands what they are getting themselves into.

IMO, the biggest reason why it should match their diagnostic level is
because builds could otherwise break if a project has /permissive- but
clang-cl allows a build to continue while cl would not.


>
>
>>
>>
>>> If it's a warning, you can pass -Werror separately if you want. And
>>> SFINAE'ing on this seems like asking for trouble.
>>>
>>
>> I said nothing about permitting SFINAE on these errors.
>>
>>
>>>
>>>


>
>
>>
>>
>>> Should we just ignore /permissive- and possibly make some of our
>>> -Wmicrosoft Extensions ExtWarns instead?
>>>
>>> On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
>>> david.majne...@gmail.com> wrote:
>>>
 -pedantic means "Issue all the warnings demanded by strict ISO C
 and ISO C++; reject all programs that use forbidden extensions, and 
 some
 other programs that do not follow ISO C and ISO C++."
 I believe it is more akin to -fno-ms-compatibility as it disables
 compatibility hacks.

 On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber 
 wrote:

> It does sound pretty similar to me from the blog post. I think
> this is a decent place to start from.
>
> On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
> cfe-commits"  wrote:
>
>> majnemer requested 

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

This should work correctly with alignment attributes, I think?  IIRC those just 
change the return value of getDeclAlign().

I agree it's a little fragile, but I don't have a good suggestion to avoid that.


https://reviews.llvm.org/D32456



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

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[Nico Weber via cfe-commits]

On Tue, Apr 25, 2017 at 4:14 PM, David Majnemer 
wrote:

>
>
> On Tue, Apr 25, 2017 at 11:42 AM, Nico Weber  wrote:
>
>> On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer > > wrote:
>>
>>>
>>>
>>> On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber  wrote:
>>>
 On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer <
 david.majne...@gmail.com> wrote:

>
>
> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber 
> wrote:
>
>> "Opting into the conforming mode, /permissive-, during the series of
>> VS 2017 update is a commitment to keeping your code base clean and to
>> fixing non-conforming constructs we fix conformance issues in Visual 
>> C++."
>> [...] "By contrast /permissive- offers a useful conformance mode where
>> input C++ code is interpreted according to ISO C++ rules but also allows
>> conforming extensions necessary to compile C++ on targets supported by
>> Visual C++."
>>
>> I guess the second quote agrees with your interpretation.
>>
>> We already diag most of the things they already mention. The one
>> thing we don't diag by default is Wmicrosoft-enum-forward-reference
>> since that's only an Extension and not an ExtWarn. We don't expose
>> -pedantic from clang-cl, so this seemed like a somewhat natural mapping 
>> to
>> me.
>>
>> Should /permissive- map to -Wmicrosoft instead and turn on the parts
>> of -Wmicrosoft that are Extensions?
>>
>
> Did you mean on or off?
>

 On.


> I think that their intent is that things like __declspec remain OK.
>

 Nothing in -Wmicrosoft warns on __declspec.


> They want to diagnose non-conforming extensions like crazy template
> stuff, bogus typedef syntax, bad main function definitions, etc.
>

 Right. The only thing it currently makes cl warn on that clang-cl
 doesn't warn on by default is Wmicrosoft-enum-forward-reference, which
 is an Extension warning, not an ExtWarn. So mapping /permissive- to
 -Wmicrosoft would make clang-cl diagnose forward-declared enums like it
 does with 2017 cl.

>>>
>>> Ok, sounds like it diagnoses the same sorts of things. They diagnose as
>>> error though, I think we should too. What about -fdelayed-template-parsing?
>>> Shouldn't that be disabled?
>>>
>>
>> CL has added a /Zc:twoPhase for that (not yet released anywhere), and
>> Hans added support for that to clang-cl a while ago. Some blog post (maybe
>> the one I linked to?) says that they're thinking of possibly
>> enabling /Zc:twoPhase when /permissive- is passed, but at the moment it's
>> independent. (In part because /permissive- ships in VC2017 and /Zc:twoPhase
>> hasn't been shipped yet).
>>
>
> Ok.
>
>
>>
>> What's the advantage of making it an error?
>>
>
> All the diagnostics they show in https://blogs.msdn.
> microsoft.com/vcblog/2016/11/16/permissive-switch/ diagnose as errors.
>

But why should we match that? Having them be warnings seems like a better
user experience to me.


>
>
>> If it's a warning, you can pass -Werror separately if you want. And
>> SFINAE'ing on this seems like asking for trouble.
>>
>
> I said nothing about permitting SFINAE on these errors.
>
>
>>
>>
>>>
>>>


>
>
>> Should we just ignore /permissive- and possibly make some of our
>> -Wmicrosoft Extensions ExtWarns instead?
>>
>> On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
>> david.majne...@gmail.com> wrote:
>>
>>> -pedantic means "Issue all the warnings demanded by strict ISO C and
>>> ISO C++; reject all programs that use forbidden extensions, and some 
>>> other
>>> programs that do not follow ISO C and ISO C++."
>>> I believe it is more akin to -fno-ms-compatibility as it disables
>>> compatibility hacks.
>>>
>>> On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber 
>>> wrote:
>>>
 It does sound pretty similar to me from the blog post. I think this
 is a decent place to start from.

 On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
 cfe-commits"  wrote:

> majnemer requested changes to this revision.
> majnemer added a comment.
> This revision now requires changes to proceed.
>
> I don't think this is correct. GDR (of Microsoft) says the
> behavior is different: https://www.reddit.com/r/cpp/comm
> 
>   LOG(INFO) << "n_window_index: " << n_window_index;
> ents/5dh7j5/visual_c_introduces_permissive_for_conformance/d
> a5fxjj/
> 

r301361 - [Hexagon] Set -ffp-contract=fast at -O3 unless explicitly specified

[Krzysztof Parzyszek via cfe-commits]

Author: kparzysz
Date: Tue Apr 25 15:51:51 2017
New Revision: 301361

URL: http://llvm.org/viewvc/llvm-project?rev=301361=rev
Log:
[Hexagon] Set -ffp-contract=fast at -O3 unless explicitly specified

Modified:
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
cfe/trunk/lib/Driver/ToolChains/Hexagon.h
cfe/trunk/test/Driver/hexagon-toolchain-elf.c

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=301361=301360=301361=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Tue Apr 25 15:51:51 2017
@@ -402,6 +402,39 @@ Tool *HexagonToolChain::buildLinker() co
   return new tools::hexagon::Linker(*this);
 }
 
+unsigned HexagonToolChain::getOptimizationLevel(
+const llvm::opt::ArgList ) const {
+  // Copied in large part from lib/Frontend/CompilerInvocation.cpp.
+  Arg *A = DriverArgs.getLastArg(options::OPT_O_Group);
+  if (!A)
+return 0;
+
+  if (A->getOption().matches(options::OPT_O0))
+return 0;
+  if (A->getOption().matches(options::OPT_Ofast))
+return 3;
+  assert(A->getNumValues() != 0);
+  StringRef S(A->getValue());
+  if (S == "s" || S == "z" || S.empty())
+return 2;
+  if (S == "g")
+return 1;
+
+  unsigned OptLevel;
+  if (S.getAsInteger(10, OptLevel))
+return 0;
+  return OptLevel;
+}
+
+void HexagonToolChain::addClangTargetOptions(const ArgList ,
+ ArgStringList ) const {
+  if (DriverArgs.hasArg(options::OPT_ffp_contract))
+return;
+  unsigned OptLevel = getOptimizationLevel(DriverArgs);
+  if (OptLevel >= 3)
+CC1Args.push_back("-ffp-contract=fast");
+}
+
 void HexagonToolChain::AddClangSystemIncludeArgs(const ArgList ,
  ArgStringList ) const 
{
   if (DriverArgs.hasArg(options::OPT_nostdinc) ||

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.h?rev=301361=301360=301361=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.h Tue Apr 25 15:51:51 2017
@@ -61,11 +61,15 @@ protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
 
+  unsigned getOptimizationLevel(const llvm::opt::ArgList ) const;
+
 public:
   HexagonToolChain(const Driver , const llvm::Triple ,
const llvm::opt::ArgList );
   ~HexagonToolChain() override;
 
+  void addClangTargetOptions(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;

Modified: cfe/trunk/test/Driver/hexagon-toolchain-elf.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-toolchain-elf.c?rev=301361=301360=301361=diff
==
--- cfe/trunk/test/Driver/hexagon-toolchain-elf.c (original)
+++ cfe/trunk/test/Driver/hexagon-toolchain-elf.c Tue Apr 25 15:51:51 2017
@@ -97,6 +97,22 @@
 // CHECK024: "-cc1" {{.*}} "-target-cpu" "hexagonv62"
 // CHECK024: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v62/crt0
 
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -O3 \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK025 %s
+// CHECK025: "-ffp-contract=fast"
+// CHECK025: hexagon-link
+
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -O3 -ffp-contract=off \
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK026 %s
+// CHECK026-NOT: "-ffp-contract=fast"
+// CHECK026: hexagon-link
+
 // 
-
 // Test Linker related args
 // 
-


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

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D32456#737273, @efriedma wrote:

> Err, that's not what I meant...
>
> If the alignment of a global in LLVM IR is "zero", it doesn't really mean 
> zero.  It means "guess the alignment I want based on the specified type of 
> the global".  And that's not a game we ever want to play when generating IR 
> from clang.


The alternative mentioned in PR32630 is to decrease the alignment of the global 
when -fsanitize=alignment is enabled. Is this less risky than not specifying 
the alignment at all? I assumed that it was not.

Hm, giving this more thought I'm not happy with how fragile this patch seems. 
Changing the way we set alignment to hack around constant folding doesn't seem 
great. And this patch does not interact properly with extern definitions which 
have '__attribute__((aligned(N)))' set.


https://reviews.llvm.org/D32456



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

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

JDevlieghere wrote:
> It's highly recommended to put some kind of error message in the assertion 
> statement, as per the LLVM coding standards.
would it be better to change it to
!innerCtorCall && !MakePairCall && "No .."



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:22
+}
+} // namespace
+

extend the anonymous namespace to have static strings (and remove static)



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:90
+  auto makePair = ignoringImplicit(
+  callExpr(callee(expr(ignoringParenImpCasts(
+  declRefExpr(unless(hasExplicitTemplateArgs()),

Why ignoringParenImpCasts instead of ingnoringImplicit?


https://reviews.llvm.org/D32395



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

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Err, that's not what I meant...

If the alignment of a global in LLVM IR is "zero", it doesn't really mean zero. 
 It means "guess the alignment I want based on the specified type of the 
global".  And that's not a game we ever want to play when generating IR from 
clang.


https://reviews.llvm.org/D32456



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

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D32456#737228, @efriedma wrote:

> > If the alignment isn't explicitly set, it is initialized to 0.
>
> That's what I thought.  I don't like this; clang should explicitly set an 
> alignment for every global.


Ok, I will set this aside for now and think of an alternate approach to address 
PR32630.


https://reviews.llvm.org/D32456



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

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[David Majnemer via cfe-commits]

On Tue, Apr 25, 2017 at 11:42 AM, Nico Weber  wrote:

> On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer 
> wrote:
>
>>
>>
>> On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber  wrote:
>>
>>> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer <
>>> david.majne...@gmail.com> wrote:
>>>


 On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber 
 wrote:

> "Opting into the conforming mode, /permissive-, during the series of
> VS 2017 update is a commitment to keeping your code base clean and to
> fixing non-conforming constructs we fix conformance issues in Visual C++."
> [...] "By contrast /permissive- offers a useful conformance mode where
> input C++ code is interpreted according to ISO C++ rules but also allows
> conforming extensions necessary to compile C++ on targets supported by
> Visual C++."
>
> I guess the second quote agrees with your interpretation.
>
> We already diag most of the things they already mention. The one thing
> we don't diag by default is Wmicrosoft-enum-forward-reference since
> that's only an Extension and not an ExtWarn. We don't expose -pedantic 
> from
> clang-cl, so this seemed like a somewhat natural mapping to me.
>
> Should /permissive- map to -Wmicrosoft instead and turn on the parts
> of -Wmicrosoft that are Extensions?
>

 Did you mean on or off?

>>>
>>> On.
>>>
>>>
 I think that their intent is that things like __declspec remain OK.

>>>
>>> Nothing in -Wmicrosoft warns on __declspec.
>>>
>>>
 They want to diagnose non-conforming extensions like crazy template
 stuff, bogus typedef syntax, bad main function definitions, etc.

>>>
>>> Right. The only thing it currently makes cl warn on that clang-cl
>>> doesn't warn on by default is Wmicrosoft-enum-forward-reference, which
>>> is an Extension warning, not an ExtWarn. So mapping /permissive- to
>>> -Wmicrosoft would make clang-cl diagnose forward-declared enums like it
>>> does with 2017 cl.
>>>
>>
>> Ok, sounds like it diagnoses the same sorts of things. They diagnose as
>> error though, I think we should too. What about -fdelayed-template-parsing?
>> Shouldn't that be disabled?
>>
>
> CL has added a /Zc:twoPhase for that (not yet released anywhere), and Hans
> added support for that to clang-cl a while ago. Some blog post (maybe the
> one I linked to?) says that they're thinking of possibly
> enabling /Zc:twoPhase when /permissive- is passed, but at the moment it's
> independent. (In part because /permissive- ships in VC2017 and /Zc:twoPhase
> hasn't been shipped yet).
>

Ok.


>
> What's the advantage of making it an error?
>

All the diagnostics they show in
https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/
diagnose as errors.


> If it's a warning, you can pass -Werror separately if you want. And
> SFINAE'ing on this seems like asking for trouble.
>

I said nothing about permitting SFINAE on these errors.


>
>
>>
>>
>>>
>>>


> Should we just ignore /permissive- and possibly make some of our
> -Wmicrosoft Extensions ExtWarns instead?
>
> On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
> david.majne...@gmail.com> wrote:
>
>> -pedantic means "Issue all the warnings demanded by strict ISO C and
>> ISO C++; reject all programs that use forbidden extensions, and some 
>> other
>> programs that do not follow ISO C and ISO C++."
>> I believe it is more akin to -fno-ms-compatibility as it disables
>> compatibility hacks.
>>
>> On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber 
>> wrote:
>>
>>> It does sound pretty similar to me from the blog post. I think this
>>> is a decent place to start from.
>>>
>>> On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
>>> cfe-commits"  wrote:
>>>
 majnemer requested changes to this revision.
 majnemer added a comment.
 This revision now requires changes to proceed.

 I don't think this is correct. GDR (of Microsoft) says the behavior
 is different: https://www.reddit.com/r/cpp/comm
 
   LOG(INFO) << "n_window_index: " << n_window_index;
 ents/5dh7j5/visual_c_introduces_permissive_for_conformance/da5fxjj/
 


 https://reviews.llvm.org/D32435



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

>>>
>>
>

>>>
>>
>

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96622.
kuhar marked 4 inline comments as done.
kuhar added a comment.

Added const where possible, moved from if-else to ternary.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,51 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The examples below illustrate the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
+
+  struct Y {
+Y(std::pair&&) {}
+  };
+  std::vector y;
+  y.push_back(std::make_pair(2, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: y.emplace_back(std::make_pair(2, 3));
+  // make_pair cannot be removed here, as Y is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===
--- docs/clang-tidy/checks/modernize-use-emplace.rst
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -36,8 +36,7 @@
 
 std::vector> w;
 w.emplace_back(21, 37);
-// This will be fixed to w.emplace_back(21L, 37L); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+w.emplace_back(21L, 37L);
 
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,12 @@
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+} // namespace
+
 static const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
@@ -80,43 +86,64 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Thanks for the suggestions, Jonas. Fixed.


https://reviews.llvm.org/D32395



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

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> If the alignment isn't explicitly set, it is initialized to 0.

That's what I thought.  I don't like this; clang should explicitly set an 
alignment for every global.


https://reviews.llvm.org/D32456



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

r301348 - Minor fix for distribute_parallel_for_num_threads_codegen on AARCH64

[Carlo Bertolli via cfe-commits]

Author: cbertol
Date: Tue Apr 25 13:59:37 2017
New Revision: 301348

URL: http://llvm.org/viewvc/llvm-project?rev=301348=rev
Log:
Minor fix for distribute_parallel_for_num_threads_codegen on AARCH64

Modified:
cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp

Modified: cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp?rev=301348=301347=301348=diff
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp Tue 
Apr 25 13:59:37 2017
@@ -114,7 +114,7 @@ int main() {
 // CHECK: call {{.*}}void {{.+}} @__kmpc_fork_teams({{.+}}, i32 0, {{.+}}* 
[[T_OMP_TEAMS_OUTLINED_3:@.+]] to {{.+}})
 
 // CHECK: define{{.+}} void [[T_OMP_TEAMS_OUTLINED_3]](
-// CHECK-DAG:   [[CALL_RES:%.+]] = invoke{{.+}} i8 
[[S_TY_CHAR_OP:@.+]]([[S_TY]]* {{.+}})
+// CHECK-DAG:   [[CALL_RES:%.+]] = invoke{{.*}} i8 
[[S_TY_CHAR_OP:@.+]]([[S_TY]]* {{.+}})
 // CHECK-DAG:   [[CALL_RES_SEXT:%.+]] = sext i8 [[CALL_RES]] to {{.+}}
 // CHECK:   call {{.*}}void @__kmpc_push_num_threads([[IDENT_T_TY]]* 
[[DEF_LOC_2]], i32 {{.+}}, i32 [[CALL_RES_SEXT]])
 // CHECK:   call {{.*}}void {{.*}} @__kmpc_fork_call(


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

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[Nico Weber via cfe-commits]

On Tue, Apr 25, 2017 at 2:06 PM, David Majnemer 
wrote:

>
>
> On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber  wrote:
>
>> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer <
>> david.majne...@gmail.com> wrote:
>>
>>>
>>>
>>> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber 
>>> wrote:
>>>
 "Opting into the conforming mode, /permissive-, during the series of VS
 2017 update is a commitment to keeping your code base clean and to fixing
 non-conforming constructs we fix conformance issues in Visual C++." [...]
 "By contrast /permissive- offers a useful conformance mode where input C++
 code is interpreted according to ISO C++ rules but also allows conforming
 extensions necessary to compile C++ on targets supported by Visual C++."

 I guess the second quote agrees with your interpretation.

 We already diag most of the things they already mention. The one thing
 we don't diag by default is Wmicrosoft-enum-forward-reference since
 that's only an Extension and not an ExtWarn. We don't expose -pedantic from
 clang-cl, so this seemed like a somewhat natural mapping to me.

 Should /permissive- map to -Wmicrosoft instead and turn on the parts of
 -Wmicrosoft that are Extensions?

>>>
>>> Did you mean on or off?
>>>
>>
>> On.
>>
>>
>>> I think that their intent is that things like __declspec remain OK.
>>>
>>
>> Nothing in -Wmicrosoft warns on __declspec.
>>
>>
>>> They want to diagnose non-conforming extensions like crazy template
>>> stuff, bogus typedef syntax, bad main function definitions, etc.
>>>
>>
>> Right. The only thing it currently makes cl warn on that clang-cl doesn't
>> warn on by default is Wmicrosoft-enum-forward-reference, which is an
>> Extension warning, not an ExtWarn. So mapping /permissive- to -Wmicrosoft
>> would make clang-cl diagnose forward-declared enums like it does with 2017
>> cl.
>>
>
> Ok, sounds like it diagnoses the same sorts of things. They diagnose as
> error though, I think we should too. What about -fdelayed-template-parsing?
> Shouldn't that be disabled?
>

CL has added a /Zc:twoPhase for that (not yet released anywhere), and Hans
added support for that to clang-cl a while ago. Some blog post (maybe the
one I linked to?) says that they're thinking of possibly
enabling /Zc:twoPhase when /permissive- is passed, but at the moment it's
independent. (In part because /permissive- ships in VC2017 and /Zc:twoPhase
hasn't been shipped yet).

What's the advantage of making it an error? If it's a warning, you can pass
-Werror separately if you want. And SFINAE'ing on this seems like asking
for trouble.


>
>
>>
>>
>>>
>>>
 Should we just ignore /permissive- and possibly make some of our
 -Wmicrosoft Extensions ExtWarns instead?

 On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
 david.majne...@gmail.com> wrote:

> -pedantic means "Issue all the warnings demanded by strict ISO C and
> ISO C++; reject all programs that use forbidden extensions, and some other
> programs that do not follow ISO C and ISO C++."
> I believe it is more akin to -fno-ms-compatibility as it disables
> compatibility hacks.
>
> On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber 
> wrote:
>
>> It does sound pretty similar to me from the blog post. I think this
>> is a decent place to start from.
>>
>> On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
>> cfe-commits"  wrote:
>>
>>> majnemer requested changes to this revision.
>>> majnemer added a comment.
>>> This revision now requires changes to proceed.
>>>
>>> I don't think this is correct. GDR (of Microsoft) says the behavior
>>> is different: https://www.reddit.com/r/cpp/comm
>>> 
>>>   LOG(INFO) << "n_window_index: " << n_window_index;
>>> ents/5dh7j5/visual_c_introduces_permissive_for_conformance/da5fxjj/
>>> 
>>>
>>>
>>> https://reviews.llvm.org/D32435
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>

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

r301344 - [PGO/tests] Update comment to reflect reality.

[Davide Italiano via cfe-commits]

Author: davide
Date: Tue Apr 25 13:04:31 2017
New Revision: 301344

URL: http://llvm.org/viewvc/llvm-project?rev=301344=rev
Log:
[PGO/tests] Update comment to reflect reality.

Modified:
cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c

Modified: cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c?rev=301344=301343=301344=diff
==
--- cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c (original)
+++ cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c Tue Apr 25 13:04:31 2017
@@ -32,7 +32,7 @@ void unroll() {
 baz(i);
 }
 
-// Checks that icp is invoked.
+// Check that icp is not invoked (both -O2 and ThinLTO).
 // O2-LABEL: define void @icp
 // THINLTO-LABEL: define void @icp
 // O2-NOT: if.true.direct_targ


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

[PATCH] D32499: Further delay calling DeclMustBeEmitted until it's safe.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor created this revision.

As discussed in https://reviews.llvm.org/D30793, we have some unsafe calls to 
`isConsumerInterestedIn()`. This patch implements Richard's suggestion (from 
the inline comment) that we should track if we just deserialized an 
declaration. If we just deserialized, we can skip the unsafe call because we 
know it's interesting. If we didn't just deserialize the declaration, calling 
`isConsumerInterestedIn()` should be safe.


https://reviews.llvm.org/D32499

Files:
  include/clang/Serialization/ASTReader.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderDecl.cpp

Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -3612,7 +3612,8 @@
   assert(Record.getIdx() == Record.size());
 
   // Load any relevant update records.
-  PendingUpdateRecords.push_back(std::make_pair(ID, D));
+  PendingUpdateRecords.push_back(
+  PendingUpdateRecord(ID, D, /*JustLoaded=*/true));
 
   // Load the categories after recursive loading is finished.
   if (ObjCInterfaceDecl *Class = dyn_cast(D))
@@ -3657,20 +3658,24 @@
   }
 }
 
-void ASTReader::loadDeclUpdateRecords(serialization::DeclID ID, Decl *D) {
+void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
   // The declaration may have been modified by files later in the chain.
   // If this is the case, read the record containing the updates from each file
   // and pass it to ASTDeclReader to make the modifications.
+  serialization::GlobalDeclID ID = Record.ID;
+  Decl *D = Record.D;
   ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
   DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
   if (UpdI != DeclUpdateOffsets.end()) {
 auto UpdateOffsets = std::move(UpdI->second);
 DeclUpdateOffsets.erase(UpdI);
 
-// FIXME: This call to isConsumerInterestedIn is not safe because
-// we could be deserializing declarations at the moment. We should
-// delay calling this in the same way as done in D30793.
-bool WasInteresting = isConsumerInterestedIn(Context, D, false);
+// Check if this decl was interesting to the consumer. If we just loaded
+// the declaration, then we know it was interesting and we skip the call
+// to isConsumerInterestedIn because it is unsafe to call in the
+// current ASTReader state.
+bool WasInteresting =
+Record.JustLoaded || isConsumerInterestedIn(Context, D, false);
 for (auto  : UpdateOffsets) {
   ModuleFile *F = FileAndOffset.first;
   uint64_t Offset = FileAndOffset.second;
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2757,7 +2757,8 @@
   // If we've already loaded the decl, perform the updates when we finish
   // loading this block.
   if (Decl *D = GetExistingDecl(ID))
-PendingUpdateRecords.push_back(std::make_pair(ID, D));
+PendingUpdateRecords.push_back(
+PendingUpdateRecord(ID, D, /*JustLoaded=*/false));
   break;
 }
 
@@ -3087,7 +3088,8 @@
 // If we've already loaded the decl, perform the updates when we finish
 // loading this block.
 if (Decl *D = GetExistingDecl(ID))
-  PendingUpdateRecords.push_back(std::make_pair(ID, D));
+  PendingUpdateRecords.push_back(
+  PendingUpdateRecord(ID, D, /*JustLoaded=*/false));
   }
   break;
 }
@@ -8940,7 +8942,7 @@
 while (!PendingUpdateRecords.empty()) {
   auto Update = PendingUpdateRecords.pop_back_val();
   ReadingKindTracker ReadingKind(Read_Decl, *this);
-  loadDeclUpdateRecords(Update.first, Update.second);
+  loadDeclUpdateRecords(Update);
 }
   }
 
Index: include/clang/Serialization/ASTReader.h
===
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -478,10 +478,18 @@
   /// in the chain.
   DeclUpdateOffsetsMap DeclUpdateOffsets;
 
+  struct PendingUpdateRecord {
+Decl *D;
+serialization::GlobalDeclID ID;
+// Whether the declaration was just deserialized.
+bool JustLoaded;
+PendingUpdateRecord(serialization::GlobalDeclID ID, Decl *D,
+bool JustLoaded)
+: D(D), ID(ID), JustLoaded(JustLoaded) {}
+  };
   /// \brief Declaration updates for already-loaded declarations that we need
   /// to apply once we finish processing an import.
-  llvm::SmallVector, 16>
-  PendingUpdateRecords;
+  llvm::SmallVector PendingUpdateRecords;
 
   enum class PendingFakeDefinitionKind { NotFake, Fake, FakeLoaded };
 
@@ -1279,7 +1287,7 @@
 
   RecordLocation DeclCursorForID(serialization::DeclID ID,
  SourceLocation 

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[David Majnemer via cfe-commits]

On Tue, Apr 25, 2017 at 8:46 AM, Nico Weber  wrote:

> On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer  > wrote:
>
>>
>>
>> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber  wrote:
>>
>>> "Opting into the conforming mode, /permissive-, during the series of VS
>>> 2017 update is a commitment to keeping your code base clean and to fixing
>>> non-conforming constructs we fix conformance issues in Visual C++." [...]
>>> "By contrast /permissive- offers a useful conformance mode where input C++
>>> code is interpreted according to ISO C++ rules but also allows conforming
>>> extensions necessary to compile C++ on targets supported by Visual C++."
>>>
>>> I guess the second quote agrees with your interpretation.
>>>
>>> We already diag most of the things they already mention. The one thing
>>> we don't diag by default is Wmicrosoft-enum-forward-reference since
>>> that's only an Extension and not an ExtWarn. We don't expose -pedantic from
>>> clang-cl, so this seemed like a somewhat natural mapping to me.
>>>
>>> Should /permissive- map to -Wmicrosoft instead and turn on the parts of
>>> -Wmicrosoft that are Extensions?
>>>
>>
>> Did you mean on or off?
>>
>
> On.
>
>
>> I think that their intent is that things like __declspec remain OK.
>>
>
> Nothing in -Wmicrosoft warns on __declspec.
>
>
>> They want to diagnose non-conforming extensions like crazy template
>> stuff, bogus typedef syntax, bad main function definitions, etc.
>>
>
> Right. The only thing it currently makes cl warn on that clang-cl doesn't
> warn on by default is Wmicrosoft-enum-forward-reference, which is an
> Extension warning, not an ExtWarn. So mapping /permissive- to -Wmicrosoft
> would make clang-cl diagnose forward-declared enums like it does with 2017
> cl.
>

Ok, sounds like it diagnoses the same sorts of things. They diagnose as
error though, I think we should too. What about -fdelayed-template-parsing?
Shouldn't that be disabled?


>
>
>>
>>
>>> Should we just ignore /permissive- and possibly make some of our
>>> -Wmicrosoft Extensions ExtWarns instead?
>>>
>>> On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer <
>>> david.majne...@gmail.com> wrote:
>>>
 -pedantic means "Issue all the warnings demanded by strict ISO C and
 ISO C++; reject all programs that use forbidden extensions, and some other
 programs that do not follow ISO C and ISO C++."
 I believe it is more akin to -fno-ms-compatibility as it disables
 compatibility hacks.

 On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber 
 wrote:

> It does sound pretty similar to me from the blog post. I think this is
> a decent place to start from.
>
> On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
> cfe-commits"  wrote:
>
>> majnemer requested changes to this revision.
>> majnemer added a comment.
>> This revision now requires changes to proceed.
>>
>> I don't think this is correct. GDR (of Microsoft) says the behavior
>> is different: https://www.reddit.com/r/cpp/comm
>> 
>>   LOG(INFO) << "n_window_index: " << n_window_index;
>> ents/5dh7j5/visual_c_introduces_permissive_for_conformance/da5fxjj/
>> 
>>
>>
>> https://reviews.llvm.org/D32435
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>

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

r301339 - [PGO] Update test now that we don't call IndirectCallPromotion.

[Davide Italiano via cfe-commits]

Author: davide
Date: Tue Apr 25 12:48:10 2017
New Revision: 301339

URL: http://llvm.org/viewvc/llvm-project?rev=301339=rev
Log:
[PGO] Update test now that we don't call IndirectCallPromotion.

Modified:
cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c

Modified: cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c?rev=301339=301338=301339=diff
==
--- cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c (original)
+++ cfe/trunk/test/CodeGen/pgo-sample-thinlto-summary.c Tue Apr 25 12:48:10 2017
@@ -32,10 +32,10 @@ void unroll() {
 baz(i);
 }
 
-// Checks if icp is invoked by normal compile, but not thinlto compile.
+// Checks that icp is invoked.
 // O2-LABEL: define void @icp
 // THINLTO-LABEL: define void @icp
-// O2: if.true.direct_targ
+// O2-NOT: if.true.direct_targ
 // ThinLTO-NOT: if.true.direct_targ
 void icp(void (*p)()) {
   p();


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

[PATCH] D32372: Arrays of unknown bound in constant expressions

[Robert Haberlach via Phabricator via cfe-commits]

Arcoth updated this revision to Diff 96603.
Arcoth added a comment.

Small fix.


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return arr[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+void g(int i) {
+int arr[i];
+constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{runtime bound}}
+}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{unknown bound}}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -495,7 +495,7 @@
 // FIXME: Force the precision of the source value down so we don't
 // print digits which are usually useless (we don't really care here if
 // we truncate a digit by accident in edge cases).  Ideally,
-// APFloat::toString would automatically print the shortest 
+// APFloat::toString would automatically print the shortest
 // representation which rounds to the correct value, but it's a bit
 // tricky to implement.
 unsigned precision =
@@ -720,7 +720,7 @@
   private:
 OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId,
 unsigned ExtraNotes, bool IsCCEDiag) {
-
+
   if (EvalStatus.Diag) {
 // If we have a prior diagnostic, it will be noting that the expression
 // isn't a constant expression. This diagnostic is more important,
@@ -773,7 +773,7 @@
   unsigned ExtraNotes = 0) {
   return Diag(Loc, DiagId, ExtraNotes, false);
 }
-
+
 OptionalDiagnostic FFDiag(const Expr *E, diag::kind DiagId
   = diag::note_invalid_subexpr_in_const_expr,
 unsigned ExtraNotes = 0) {
@@ -2946,6 +2946,17 @@
   return CompleteObject();
 }
 
+// The complete object can be an array of unknown bound, in which case we
+// have to find the most recent declaration and adjust the type accordingly.
+if (Info.Ctx.getAsIncompleteArrayType(BaseType)) {
+  QualType MostRecentType =
+ cast(D->getMostRecentDecl())->getType();
+  if (auto CAT = Info.Ctx.getAsConstantArrayType(MostRecentType))
+BaseType = MostRecentType;
+  else
+return CompleteObject();
+}
+
 // Accesses of volatile-qualified objects are not allowed.
 if (BaseType.isVolatileQualified()) {
   if (Info.getLangOpts().CPlusPlus) {
@@ -4098,13 +4109,13 @@
 
   if (Info.getLangOpts().CPlusPlus11) {
 const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
-
+
 // If this function is not constexpr because it is an inherited
 // non-constexpr constructor, diagnose that directly.
 auto *CD = dyn_cast(DiagDecl);
 if (CD && CD->isInheritingConstructor()) {
   auto *Inherited = CD->getInheritedConstructor().getConstructor();
-  if (!Inherited->isConstexpr()) 
+  if (!Inherited->isConstexpr())
 DiagDecl = CD = Inherited;
 }
 
@@ -4635,7 +4646,7 @@
   return false;
 This = 
 Args = Args.slice(1);
-  } else if (MD && MD->isLambdaStaticInvoker()) {   
+  } else if (MD && MD->isLambdaStaticInvoker()) {
 // Map the static invoker for the lambda back to the call operator.
 // Conveniently, we don't have to slice out the 'this' argument (as is
 // being done for the non-static case), since a static member function
@@ -4670,7 +4681,7 @@
   FD = LambdaCallOp;
   }
 
-  
+
 } else
   return Error(E);
 
@@ -5501,7 +5512,7 @@
   // Update 'Result' to refer to the data member/field of the closure object
   // that represents the '*this' capture.
   if (!HandleLValueMember(Info, E, Result,
- Info.CurrentCall->LambdaThisCaptureField)) 
+ Info.CurrentCall->LambdaThisCaptureField))
 return false;
   // If we captured '*this' by reference, replace the field with its referent.
   if (Info.CurrentCall->LambdaThisCaptureField->getType()
@@ -5545,6 +5556,16 @@
   if (!EvaluateInteger(IExp, Offset, Info) || !EvalPtrOK)
 return false;
 
+  // 

[PATCH] D28670: [ObjC] Disallow vector parameters and return values in Objective-C methods on older X86 targets

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D28670



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

[PATCH] D32348: [libclang] Check for a record declaration before a template specialization.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@emilio, you can request to merge this into 4.0.1 by using a script called 
merge-request.sh 
(http://lists.llvm.org/pipermail/llvm-dev/2017-March/111530.html).


Repository:
  rL LLVM

https://reviews.llvm.org/D32348



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

[PATCH] D32372: Arrays of unknown bound in constant expressions

[Robert Haberlach via Phabricator via cfe-commits]

Arcoth updated this revision to Diff 96594.
Arcoth marked 6 inline comments as done.
Arcoth added a comment.

Adjusted.

Implemented the changes suggested by Richard. The array-to-pointer decay 
is now well-formed; the check was instead moved to the pointer 
arithmetic facility.


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return arr[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+void g(int i) {
+int arr[i];
+constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{runtime bound}}
+}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{unknown bound}}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -495,7 +495,7 @@
 // FIXME: Force the precision of the source value down so we don't
 // print digits which are usually useless (we don't really care here if
 // we truncate a digit by accident in edge cases).  Ideally,
-// APFloat::toString would automatically print the shortest 
+// APFloat::toString would automatically print the shortest
 // representation which rounds to the correct value, but it's a bit
 // tricky to implement.
 unsigned precision =
@@ -720,7 +720,7 @@
   private:
 OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId,
 unsigned ExtraNotes, bool IsCCEDiag) {
-
+
   if (EvalStatus.Diag) {
 // If we have a prior diagnostic, it will be noting that the expression
 // isn't a constant expression. This diagnostic is more important,
@@ -773,7 +773,7 @@
   unsigned ExtraNotes = 0) {
   return Diag(Loc, DiagId, ExtraNotes, false);
 }
-
+
 OptionalDiagnostic FFDiag(const Expr *E, diag::kind DiagId
   = diag::note_invalid_subexpr_in_const_expr,
 unsigned ExtraNotes = 0) {
@@ -2946,6 +2946,17 @@
   return CompleteObject();
 }
 
+// The complete object can be an array of unknown bound, in which case we
+// have to find the most recent declaration and adjust the type accordingly.
+if (Info.Ctx.getAsIncompleteArrayType(BaseType)) {
+  QualType MostRecentType =
+ cast(D->getMostRecentDecl())->getType();
+  if (auto CAT = Info.Ctx.getAsConstantArrayType(MostRecentType))
+BaseType = MostRecentType;
+  else
+return CompleteObject();
+}
+
 // Accesses of volatile-qualified objects are not allowed.
 if (BaseType.isVolatileQualified()) {
   if (Info.getLangOpts().CPlusPlus) {
@@ -4098,13 +4109,13 @@
 
   if (Info.getLangOpts().CPlusPlus11) {
 const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
-
+
 // If this function is not constexpr because it is an inherited
 // non-constexpr constructor, diagnose that directly.
 auto *CD = dyn_cast(DiagDecl);
 if (CD && CD->isInheritingConstructor()) {
   auto *Inherited = CD->getInheritedConstructor().getConstructor();
-  if (!Inherited->isConstexpr()) 
+  if (!Inherited->isConstexpr())
 DiagDecl = CD = Inherited;
 }
 
@@ -4635,7 +4646,7 @@
   return false;
 This = 
 Args = Args.slice(1);
-  } else if (MD && MD->isLambdaStaticInvoker()) {   
+  } else if (MD && MD->isLambdaStaticInvoker()) {
 // Map the static invoker for the lambda back to the call operator.
 // Conveniently, we don't have to slice out the 'this' argument (as is
 // being done for the non-static case), since a static member function
@@ -4670,7 +4681,7 @@
   FD = LambdaCallOp;
   }
 
-  
+
 } else
   return Error(E);
 
@@ -5501,7 +5512,7 @@
   // Update 'Result' to refer to the data member/field of the closure object
   // that represents the '*this' capture.
   if (!HandleLValueMember(Info, E, Result,
- Info.CurrentCall->LambdaThisCaptureField)) 
+ Info.CurrentCall->LambdaThisCaptureField))
 return false;
   // If we captured '*this' by reference, replace 

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision now requires changes to proceed.

This looks great, thanks a lot!


Repository:
  rL LLVM

https://reviews.llvm.org/D30771



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

r301328 - [libclang] Check for a record declaration before a template specialization

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Apr 25 11:59:07 2017
New Revision: 301328

URL: http://llvm.org/viewvc/llvm-project?rev=301328=rev
Log:
[libclang] Check for a record declaration before a template specialization

Fixes PR32539.

Patch by Emilio Cobos Álvarez!

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

Modified:
cfe/trunk/test/Index/print-type.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/test/Index/print-type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/print-type.cpp?rev=301328=301327=301328=diff
==
--- cfe/trunk/test/Index/print-type.cpp (original)
+++ cfe/trunk/test/Index/print-type.cpp Tue Apr 25 11:59:07 2017
@@ -71,6 +71,11 @@ struct Specialization;
 Specialization templRefParam;
 auto autoTemplRefParam = templRefParam;
 
+template
+struct DefaultedTypeExample {};
+
+typedef DefaultedTypeExample DefaultedTypeAlias;
+
 // RUN: c-index-test -test-print-type %s -std=c++14 | FileCheck %s
 // CHECK: Namespace=outer:1:11 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
 // CHECK: ClassTemplate=Foo:4:8 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
@@ -115,7 +120,7 @@ auto autoTemplRefParam = templRefParam;
 // CHECK: TemplateRef=Baz:9:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: IntegerLiteral= [type=int] [typekind=Int] [isPOD=1]
 // CHECK: TemplateRef=Foo:4:8 [type=] [typekind=Invalid] [isPOD=0]
-// CHECK: FieldDecl=qux:29:38 (Definition) [type=Qux] [typekind=Unexposed] [templateargs/4= [type=int] 
[typekind=Int] [type=char *] [typekind=Pointer] [type=Foo] 
[typekind=Unexposed] [type=outer::inner::Bar::FooType] [typekind=Typedef]] 
[canonicaltype=outer::Qux] 
[canonicaltypekind=Record] [canonicaltemplateargs/4= [type=int] [typekind=Int] 
[type=char *] [typekind=Pointer] [type=outer::Foo] [typekind=Record] 
[type=int] [typekind=Int]] [isPOD=1]
+// CHECK: FieldDecl=qux:29:38 (Definition) [type=Qux] [typekind=Unexposed] [templateargs/4= [type=int] 
[typekind=Int] [type=char *] [typekind=Pointer] [type=outer::Foo] 
[typekind=Record] [type=int] [typekind=Int]] [canonicaltype=outer::Qux] [canonicaltypekind=Record] 
[canonicaltemplateargs/4= [type=int] [typekind=Int] [type=char *] 
[typekind=Pointer] [type=outer::Foo] [typekind=Record] [type=int] 
[typekind=Int]] [isPOD=1]
 // CHECK: TemplateRef=Qux:12:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: TemplateRef=Foo:4:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: FunctionTemplate=tbar:36:3 [type=T (int)] [typekind=FunctionProto] 
[canonicaltype=type-parameter-0-0 (int)] [canonicaltypekind=FunctionProto] 
[resulttype=T] [resulttypekind=Unexposed] [isPOD=0]
@@ -177,3 +182,4 @@ auto autoTemplRefParam = templRefParam;
 // CHECK: VarDecl=autoTemplRefParam:72:6 (Definition) 
[type=Specialization] [typekind=Auto] [templateargs/1= 
[type=Specialization &] [typekind=LValueReference]] 
[canonicaltype=Specialization] 
[canonicaltypekind=Record] [canonicaltemplateargs/1= [type=Specialization 
&] [typekind=LValueReference]] [isPOD=1]
 // CHECK: UnexposedExpr=templRefParam:71:40 [type=const 
Specialization] [typekind=Unexposed] const 
[templateargs/1= [type=Specialization &] [typekind=LValueReference]] 
[canonicaltype=const Specialization] 
[canonicaltypekind=Record] [canonicaltemplateargs/1= [type=Specialization 
&] [typekind=LValueReference]] [isPOD=1]
 // CHECK: DeclRefExpr=templRefParam:71:40 
[type=Specialization] [typekind=Unexposed] 
[templateargs/1= [type=Specialization &] [typekind=LValueReference]] 
[canonicaltype=Specialization] 
[canonicaltypekind=Record] [canonicaltemplateargs/1= [type=Specialization 
&] [typekind=LValueReference]] [isPOD=1]
+// CHECK: TypedefDecl=DefaultedTypeAlias:77:35 (Definition) 
[type=DefaultedTypeAlias] [typekind=Typedef] [templateargs/2= [type=int] 
[typekind=Int] [type=int] [typekind=Int]] 
[canonicaltype=DefaultedTypeExample] [canonicaltypekind=Record] 
[canonicaltemplateargs/2= [type=int] [typekind=Int] [type=int] [typekind=Int]] 
[isPOD=0]

Modified: cfe/trunk/tools/libclang/CXType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXType.cpp?rev=301328=301327=301328=diff
==
--- cfe/trunk/tools/libclang/CXType.cpp (original)
+++ cfe/trunk/tools/libclang/CXType.cpp Tue Apr 25 11:59:07 2017
@@ -147,9 +147,6 @@ static inline CXTranslationUnit GetTU(CX
 static Optional
 GetTemplateArguments(QualType Type) {
   assert(!Type.isNull());
-  if (const auto *Specialization = Type->getAs())
-return Specialization->template_arguments();
-
   if (const auto *RecordDecl = Type->getAsCXXRecordDecl()) {
 

[PATCH] D32348: [libclang] Check for a record declaration before a template specialization.

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL301328: [libclang] Check for a record declaration before a 
template specialization (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D32348?vs=96130=96590#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32348

Files:
  cfe/trunk/test/Index/print-type.cpp
  cfe/trunk/tools/libclang/CXType.cpp


Index: cfe/trunk/tools/libclang/CXType.cpp
===
--- cfe/trunk/tools/libclang/CXType.cpp
+++ cfe/trunk/tools/libclang/CXType.cpp
@@ -147,16 +147,16 @@
 static Optional
 GetTemplateArguments(QualType Type) {
   assert(!Type.isNull());
-  if (const auto *Specialization = Type->getAs())
-return Specialization->template_arguments();
-
   if (const auto *RecordDecl = Type->getAsCXXRecordDecl()) {
 const auto *TemplateDecl =
   dyn_cast(RecordDecl);
 if (TemplateDecl)
   return TemplateDecl->getTemplateArgs().asArray();
   }
 
+  if (const auto *Specialization = Type->getAs())
+return Specialization->template_arguments();
+
   return None;
 }
 
Index: cfe/trunk/test/Index/print-type.cpp
===
--- cfe/trunk/test/Index/print-type.cpp
+++ cfe/trunk/test/Index/print-type.cpp
@@ -71,6 +71,11 @@
 Specialization templRefParam;
 auto autoTemplRefParam = templRefParam;
 
+template
+struct DefaultedTypeExample {};
+
+typedef DefaultedTypeExample DefaultedTypeAlias;
+
 // RUN: c-index-test -test-print-type %s -std=c++14 | FileCheck %s
 // CHECK: Namespace=outer:1:11 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
 // CHECK: ClassTemplate=Foo:4:8 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
@@ -115,7 +120,7 @@
 // CHECK: TemplateRef=Baz:9:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: IntegerLiteral= [type=int] [typekind=Int] [isPOD=1]
 // CHECK: TemplateRef=Foo:4:8 [type=] [typekind=Invalid] [isPOD=0]
-// CHECK: FieldDecl=qux:29:38 (Definition) [type=Qux] [typekind=Unexposed] [templateargs/4= [type=int] 
[typekind=Int] [type=char *] [typekind=Pointer] [type=Foo] 
[typekind=Unexposed] [type=outer::inner::Bar::FooType] [typekind=Typedef]] 
[canonicaltype=outer::Qux] 
[canonicaltypekind=Record] [canonicaltemplateargs/4= [type=int] [typekind=Int] 
[type=char *] [typekind=Pointer] [type=outer::Foo] [typekind=Record] 
[type=int] [typekind=Int]] [isPOD=1]
+// CHECK: FieldDecl=qux:29:38 (Definition) [type=Qux] [typekind=Unexposed] [templateargs/4= [type=int] 
[typekind=Int] [type=char *] [typekind=Pointer] [type=outer::Foo] 
[typekind=Record] [type=int] [typekind=Int]] [canonicaltype=outer::Qux] [canonicaltypekind=Record] 
[canonicaltemplateargs/4= [type=int] [typekind=Int] [type=char *] 
[typekind=Pointer] [type=outer::Foo] [typekind=Record] [type=int] 
[typekind=Int]] [isPOD=1]
 // CHECK: TemplateRef=Qux:12:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: TemplateRef=Foo:4:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: FunctionTemplate=tbar:36:3 [type=T (int)] [typekind=FunctionProto] 
[canonicaltype=type-parameter-0-0 (int)] [canonicaltypekind=FunctionProto] 
[resulttype=T] [resulttypekind=Unexposed] [isPOD=0]
@@ -177,3 +182,4 @@
 // CHECK: VarDecl=autoTemplRefParam:72:6 (Definition) 
[type=Specialization] [typekind=Auto] [templateargs/1= 
[type=Specialization &] [typekind=LValueReference]] 
[canonicaltype=Specialization] 
[canonicaltypekind=Record] [canonicaltemplateargs/1= [type=Specialization 
&] [typekind=LValueReference]] [isPOD=1]
 // CHECK: UnexposedExpr=templRefParam:71:40 [type=const 
Specialization] [typekind=Unexposed] const 
[templateargs/1= [type=Specialization &] [typekind=LValueReference]] 
[canonicaltype=const Specialization] 
[canonicaltypekind=Record] [canonicaltemplateargs/1= [type=Specialization 
&] [typekind=LValueReference]] [isPOD=1]
 // CHECK: DeclRefExpr=templRefParam:71:40 
[type=Specialization] [typekind=Unexposed] 
[templateargs/1= [type=Specialization &] [typekind=LValueReference]] 
[canonicaltype=Specialization] 
[canonicaltypekind=Record] [canonicaltemplateargs/1= [type=Specialization 
&] [typekind=LValueReference]] [isPOD=1]
+// CHECK: TypedefDecl=DefaultedTypeAlias:77:35 (Definition) 
[type=DefaultedTypeAlias] [typekind=Typedef] [templateargs/2= [type=int] 
[typekind=Int] [type=int] [typekind=Int]] 
[canonicaltype=DefaultedTypeExample] [canonicaltypekind=Record] 
[canonicaltemplateargs/2= [type=int] [typekind=Int] [type=int] [typekind=Int]] 
[isPOD=0]


Index: cfe/trunk/tools/libclang/CXType.cpp
===
--- cfe/trunk/tools/libclang/CXType.cpp
+++ 

[PATCH] D32348: [libclang] Check for a record declaration before a template specialization.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Sure. You can ask Chris Lattner for commit access for future patches :)


https://reviews.llvm.org/D32348



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

[PATCH] D32389: [libclang] Expose some target information via the C API.

[Emilio Cobos Álvarez via Phabricator via cfe-commits]

emilio added a comment.

Sounds good to me, will do :)


Repository:
  rL LLVM

https://reviews.llvm.org/D32389



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

[PATCH] D32348: [libclang] Check for a record declaration before a template specialization.

[Emilio Cobos Álvarez via Phabricator via cfe-commits]

emilio added a comment.

Yes, please! I've submitted a few patches, but still no commit access.

Thanks!


https://reviews.llvm.org/D32348



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

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

It's highly recommended to put some kind of error message in the assertion 
statement, as per the LLVM coding standards.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:130
+  SourceRange CallParensRange;
+  if (MakePairCall)
+CallParensRange = SourceRange(MakePairCall->getCallee()->getLocEnd(),

Can't we use the ternary operator here?



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:140
 
+  SourceLocation ExprBegin;
+  if (MakePairCall)

Same here?



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:147
   // Range for constructor name and opening brace.
-  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
-  InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
+  auto ParamCallSourceRange = CharSourceRange::getTokenRange(
+  ExprBegin, CallParensRange.getBegin());

I think this can be const.


https://reviews.llvm.org/D32395



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

Re: [PATCH] D32435: clang-cl: Add support for /permissive-

[Nico Weber via cfe-commits]

On Mon, Apr 24, 2017 at 10:00 PM, David Majnemer 
wrote:

>
>
> On Mon, Apr 24, 2017 at 11:56 AM, Nico Weber  wrote:
>
>> "Opting into the conforming mode, /permissive-, during the series of VS
>> 2017 update is a commitment to keeping your code base clean and to fixing
>> non-conforming constructs we fix conformance issues in Visual C++." [...]
>> "By contrast /permissive- offers a useful conformance mode where input C++
>> code is interpreted according to ISO C++ rules but also allows conforming
>> extensions necessary to compile C++ on targets supported by Visual C++."
>>
>> I guess the second quote agrees with your interpretation.
>>
>> We already diag most of the things they already mention. The one thing we
>> don't diag by default is Wmicrosoft-enum-forward-reference since that's
>> only an Extension and not an ExtWarn. We don't expose -pedantic from
>> clang-cl, so this seemed like a somewhat natural mapping to me.
>>
>> Should /permissive- map to -Wmicrosoft instead and turn on the parts of
>> -Wmicrosoft that are Extensions?
>>
>
> Did you mean on or off?
>

On.


> I think that their intent is that things like __declspec remain OK.
>

Nothing in -Wmicrosoft warns on __declspec.


> They want to diagnose non-conforming extensions like crazy template stuff,
> bogus typedef syntax, bad main function definitions, etc.
>

Right. The only thing it currently makes cl warn on that clang-cl doesn't
warn on by default is Wmicrosoft-enum-forward-reference, which is an
Extension warning, not an ExtWarn. So mapping /permissive- to -Wmicrosoft
would make clang-cl diagnose forward-declared enums like it does with 2017
cl.


>
>
>> Should we just ignore /permissive- and possibly make some of our
>> -Wmicrosoft Extensions ExtWarns instead?
>>
>> On Mon, Apr 24, 2017 at 2:10 PM, David Majnemer > > wrote:
>>
>>> -pedantic means "Issue all the warnings demanded by strict ISO C and ISO
>>> C++; reject all programs that use forbidden extensions, and some other
>>> programs that do not follow ISO C and ISO C++."
>>> I believe it is more akin to -fno-ms-compatibility as it disables
>>> compatibility hacks.
>>>
>>> On Mon, Apr 24, 2017 at 11:02 AM, Nico Weber 
>>> wrote:
>>>
 It does sound pretty similar to me from the blog post. I think this is
 a decent place to start from.

 On Apr 24, 2017 11:55 AM, "David Majnemer via Phabricator via
 cfe-commits"  wrote:

> majnemer requested changes to this revision.
> majnemer added a comment.
> This revision now requires changes to proceed.
>
> I don't think this is correct. GDR (of Microsoft) says the behavior is
> different: https://www.reddit.com/r/cpp/comm
> 
>   LOG(INFO) << "n_window_index: " << n_window_index;
> ents/5dh7j5/visual_c_introduces_permissive_for_conformance/da5fxjj/
> 
>
>
> https://reviews.llvm.org/D32435
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>

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

r301315 - [index] Index type source info for class specializations

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Apr 25 10:13:42 2017
New Revision: 301315

URL: http://llvm.org/viewvc/llvm-project?rev=301315=rev
Log:
[index] Index type source info for class specializations

rdar://31758344

Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
cfe/trunk/test/Index/index-refs.cpp

Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=301315=301314=301315=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Tue Apr 25 10:13:42 2017
@@ -564,6 +564,9 @@ public:
   D, 
SymbolRelation(SymbolRoleSet(SymbolRole::RelationSpecializationOf),
 SpecializationOf));
 }
+if (TypeSourceInfo *TSI = D->getTypeAsWritten())
+  IndexCtx.indexTypeSourceInfo(TSI, /*Parent=*/nullptr,
+   D->getLexicalDeclContext());
 return true;
   }
 

Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=301315=301314=301315=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Tue Apr 25 10:13:42 2017
@@ -255,3 +255,36 @@ void ContainsSpecializedMemberFunction::
 // CHECK-NEXT: RelChild
 // CHECK-NEXT: RelSpecialization | memberSpecialization | 
c:@S@ContainsSpecializedMemberFunction@FT@>1#TmemberSpecialization#v#
 }
+
+template
+class SpecializationDecl;
+// CHECK: [[@LINE-1]]:7 | class(Gen)/C++ | SpecializationDecl | 
c:@ST>1#T@SpecializationDecl |  | Ref | rel: 0
+
+template
+class SpecializationDecl { };
+// CHECK: [[@LINE-1]]:7 | class(Gen)/C++ | SpecializationDecl | 
c:@ST>1#T@SpecializationDecl |  | Def | rel: 0
+
+template<>
+class SpecializationDecl;
+// CHECK: [[@LINE-1]]:7 | class(Gen)/C++ | SpecializationDecl | 
c:@ST>1#T@SpecializationDecl |  | Ref | rel: 0
+
+template<>
+class SpecializationDecl { };
+// CHECK: [[@LINE-1]]:7 | class(Gen,TS)/C++ | SpecializationDecl | 
c:@S@SpecializationDecl>#I |  | Def,RelSpecialization | rel: 1
+// CHECK-NEXT: RelSpecialization | SpecializationDecl | 
c:@ST>1#T@SpecializationDecl
+// CHECK-NEXT: [[@LINE-3]]:7 | class(Gen)/C++ | SpecializationDecl | 
c:@ST>1#T@SpecializationDecl |  | Ref | rel: 0
+
+template
+class PartialSpecilizationClass;
+// CHECK: [[@LINE-1]]:7 | class(Gen)/C++ | PartialSpecilizationClass | 
c:@ST>2#T#T@PartialSpecilizationClass |  | Ref | rel: 0
+// CHECK-NEXT: [[@LINE-2]]:33 | class/C++ | Cls | c:@S@Cls |  | Ref 
| rel: 0
+
+template<>
+class PartialSpecilizationClass : Cls { };
+// CHECK: [[@LINE-1]]:7 | class(Gen,TS)/C++ | PartialSpecilizationClass | 
c:@S@PartialSpecilizationClass>#$@S@Cls#S0_ |  | 
Def,RelSpecialization | rel: 1
+// CHECK-NEXT: RelSpecialization | PartialSpecilizationClass | 
c:@ST>2#T#T@PartialSpecilizationClass
+// CHECK-NEXT: [[@LINE-3]]:45 | class/C++ | Cls | c:@S@Cls |  | 
Ref,RelBase,RelCont | rel: 1
+// CHECK-NEXT: RelBase,RelCont | PartialSpecilizationClass | 
c:@S@PartialSpecilizationClass>#$@S@Cls#S0_
+// CHECK-NEXT: [[@LINE-5]]:7 | class(Gen)/C++ | PartialSpecilizationClass | 
c:@ST>2#T#T@PartialSpecilizationClass |  | Ref | rel: 0
+// CHECK-NEXT: [[@LINE-6]]:33 | class/C++ | Cls | c:@S@Cls |  | Ref 
| rel: 0
+// CHECK-NEXT: [[@LINE-7]]:38 | class/C++ | Cls | c:@S@Cls |  | Ref 
| rel: 0

Modified: cfe/trunk/test/Index/index-refs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/index-refs.cpp?rev=301315=301314=301315=diff
==
--- cfe/trunk/test/Index/index-refs.cpp (original)
+++ cfe/trunk/test/Index/index-refs.cpp Tue Apr 25 10:13:42 2017
@@ -105,6 +105,7 @@ int ginitlist[] = {EnumVal};
 // CHECK:  [indexDeclaration]: kind: c++-class-template | name: TS | 
{{.*}} | loc: 47:8
 // CHECK-NEXT: [indexDeclaration]: kind: struct-template-partial-spec | name: 
TS | USR: c:@SP>1#T@TS>#t0.0#I | {{.*}} | loc: 50:8
 // CHECK-NEXT: [indexDeclaration]: kind: typedef | name: MyInt | USR: 
c:index-refs.cpp@SP>1#T@TS>#t0.0#I@T@MyInt | {{.*}} | loc: 51:15 | 
semantic-container: [TS:50:8] | lexical-container: [TS:50:8]
+// CHECK-NEXT: [indexEntityReference]: kind: c++-class-template | name: TS | 
USR: c:@ST>2#T#T@TS | lang: C++ | cursor: TemplateRef=TS:47:8 | loc: 50:8 | 
:: <> | container: [TU] | refkind: direct
 /* when indexing implicit instantiations
   [indexDeclaration]: kind: struct-template-spec | name: TS | USR: c:@S@TS>#I 
| {{.*}} | loc: 50:8
   [indexDeclaration]: kind: typedef | name: MyInt | USR: 
c:index-refs.cpp@593@S@TS>#I@T@MyInt | {{.*}} | loc: 51:15 | 
semantic-container: [TS:50:8] | lexical-container: [TS:50:8]


___
cfe-commits 

[PATCH] D32489: [compiler-rt] [test] Build sanitizer/xray tests only if COMPILER_RT_BUILD_* is on

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.

Cover the sanitizer tests with COMPILER_RT_BUILD_SANITIZERS
conditional, and add COMPILER_RT_BUILD_XRAY conditional to the xray
tests. This makes it possible to do a pure-builtins build with tests
enabled.


Repository:
  rL LLVM

https://reviews.llvm.org/D32489

Files:
  test/CMakeLists.txt


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -41,47 +41,49 @@
   if(COMPILER_RT_BUILD_BUILTINS)
 add_subdirectory(builtins)
   endif()
-  if(COMPILER_RT_HAS_ASAN)
-add_subdirectory(asan)
-  endif()
-  if(COMPILER_RT_HAS_DFSAN)
-add_subdirectory(dfsan)
-  endif()
-  if (COMPILER_RT_HAS_INTERCEPTION)
-add_subdirectory(interception)
-  endif()
-  if(COMPILER_RT_HAS_LSAN)
-add_subdirectory(lsan)
-  endif()
-  if(COMPILER_RT_HAS_MSAN)
-add_subdirectory(msan)
-  endif()
-  if(COMPILER_RT_HAS_PROFILE)
-add_subdirectory(profile)
-  endif()
-  if(COMPILER_RT_HAS_SANITIZER_COMMON)
-add_subdirectory(sanitizer_common)
-  endif()
-  if(COMPILER_RT_HAS_TSAN)
-add_subdirectory(tsan)
-  endif()
-  if(COMPILER_RT_HAS_UBSAN)
-add_subdirectory(ubsan)
-  endif()
-  # CFI tests require diagnostic mode, which is implemented in UBSan.
-  if(COMPILER_RT_HAS_UBSAN)
-add_subdirectory(cfi)
-  endif()
-  if(COMPILER_RT_HAS_SAFESTACK)
-add_subdirectory(safestack)
-  endif()
-  if(COMPILER_RT_HAS_ESAN)
-add_subdirectory(esan)
-  endif()
-  if(COMPILER_RT_HAS_SCUDO)
-add_subdirectory(scudo)
+  if(COMPILER_RT_BUILD_SANITIZERS)
+if(COMPILER_RT_HAS_ASAN)
+  add_subdirectory(asan)
+endif()
+if(COMPILER_RT_HAS_DFSAN)
+  add_subdirectory(dfsan)
+endif()
+if (COMPILER_RT_HAS_INTERCEPTION)
+  add_subdirectory(interception)
+endif()
+if(COMPILER_RT_HAS_LSAN)
+  add_subdirectory(lsan)
+endif()
+if(COMPILER_RT_HAS_MSAN)
+  add_subdirectory(msan)
+endif()
+if(COMPILER_RT_HAS_PROFILE)
+  add_subdirectory(profile)
+endif()
+if(COMPILER_RT_HAS_SANITIZER_COMMON)
+  add_subdirectory(sanitizer_common)
+endif()
+if(COMPILER_RT_HAS_TSAN)
+  add_subdirectory(tsan)
+endif()
+if(COMPILER_RT_HAS_UBSAN)
+  add_subdirectory(ubsan)
+endif()
+# CFI tests require diagnostic mode, which is implemented in UBSan.
+if(COMPILER_RT_HAS_UBSAN)
+  add_subdirectory(cfi)
+endif()
+if(COMPILER_RT_HAS_SAFESTACK)
+  add_subdirectory(safestack)
+endif()
+if(COMPILER_RT_HAS_ESAN)
+  add_subdirectory(esan)
+endif()
+if(COMPILER_RT_HAS_SCUDO)
+  add_subdirectory(scudo)
+endif()
   endif()
-  if(COMPILER_RT_HAS_XRAY)
+  if(COMPILER_RT_BUILD_XRAY AND COMPILER_RT_HAS_XRAY)
 add_subdirectory(xray)
   endif()
 endif()


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -41,47 +41,49 @@
   if(COMPILER_RT_BUILD_BUILTINS)
 add_subdirectory(builtins)
   endif()
-  if(COMPILER_RT_HAS_ASAN)
-add_subdirectory(asan)
-  endif()
-  if(COMPILER_RT_HAS_DFSAN)
-add_subdirectory(dfsan)
-  endif()
-  if (COMPILER_RT_HAS_INTERCEPTION)
-add_subdirectory(interception)
-  endif()
-  if(COMPILER_RT_HAS_LSAN)
-add_subdirectory(lsan)
-  endif()
-  if(COMPILER_RT_HAS_MSAN)
-add_subdirectory(msan)
-  endif()
-  if(COMPILER_RT_HAS_PROFILE)
-add_subdirectory(profile)
-  endif()
-  if(COMPILER_RT_HAS_SANITIZER_COMMON)
-add_subdirectory(sanitizer_common)
-  endif()
-  if(COMPILER_RT_HAS_TSAN)
-add_subdirectory(tsan)
-  endif()
-  if(COMPILER_RT_HAS_UBSAN)
-add_subdirectory(ubsan)
-  endif()
-  # CFI tests require diagnostic mode, which is implemented in UBSan.
-  if(COMPILER_RT_HAS_UBSAN)
-add_subdirectory(cfi)
-  endif()
-  if(COMPILER_RT_HAS_SAFESTACK)
-add_subdirectory(safestack)
-  endif()
-  if(COMPILER_RT_HAS_ESAN)
-add_subdirectory(esan)
-  endif()
-  if(COMPILER_RT_HAS_SCUDO)
-add_subdirectory(scudo)
+  if(COMPILER_RT_BUILD_SANITIZERS)
+if(COMPILER_RT_HAS_ASAN)
+  add_subdirectory(asan)
+endif()
+if(COMPILER_RT_HAS_DFSAN)
+  add_subdirectory(dfsan)
+endif()
+if (COMPILER_RT_HAS_INTERCEPTION)
+  add_subdirectory(interception)
+endif()
+if(COMPILER_RT_HAS_LSAN)
+  add_subdirectory(lsan)
+endif()
+if(COMPILER_RT_HAS_MSAN)
+  add_subdirectory(msan)
+endif()
+if(COMPILER_RT_HAS_PROFILE)
+  add_subdirectory(profile)
+endif()
+if(COMPILER_RT_HAS_SANITIZER_COMMON)
+  add_subdirectory(sanitizer_common)
+endif()
+if(COMPILER_RT_HAS_TSAN)
+  add_subdirectory(tsan)
+endif()
+if(COMPILER_RT_HAS_UBSAN)
+  add_subdirectory(ubsan)
+endif()
+# CFI tests require diagnostic mode, which is implemented in UBSan.
+if(COMPILER_RT_HAS_UBSAN)
+  add_subdirectory(cfi)
+endif()
+if(COMPILER_RT_HAS_SAFESTACK)
+  

[PATCH] D32486: Cleanup pragma handlers after DoPrintPreprocessedInput

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren accepted this revision.
yaron.keren added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D32486



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

[PATCH] D32486: Cleanup pragma handlers after DoPrintPreprocessedInput

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 96561.
teemperor added a comment.

- Now using unique_ptr instead of raw pointers.


https://reviews.llvm.org/D32486

Files:
  lib/Frontend/PrintPreprocessedOutput.cpp


Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -773,26 +773,33 @@
 
   // Expand macros in pragmas with -fms-extensions.  The assumption is that
   // the majority of pragmas in such a file will be Microsoft pragmas.
-  PP.AddPragmaHandler(new UnknownPragmaHandler(
-  "#pragma", Callbacks,
+  // Remember the handlers we will add so that we can remove them later.
+  std::unique_ptr MicrosoftExtHandler(
+  new UnknownPragmaHandler(
+  "#pragma", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+
+  std::unique_ptr GCCHandler(new UnknownPragmaHandler(
+  "#pragma GCC", Callbacks,
   /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
-  PP.AddPragmaHandler(
-  "GCC", new UnknownPragmaHandler(
- "#pragma GCC", Callbacks,
- /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
-  PP.AddPragmaHandler(
-  "clang", new UnknownPragmaHandler(
-   "#pragma clang", Callbacks,
-   /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+
+  std::unique_ptr ClangHandler(new UnknownPragmaHandler(
+  "#pragma clang", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+
+  PP.AddPragmaHandler(MicrosoftExtHandler.get());
+  PP.AddPragmaHandler("GCC", GCCHandler.get());
+  PP.AddPragmaHandler("clang", ClangHandler.get());
 
   // The tokens after pragma omp need to be expanded.
   //
   //  OpenMP [2.1, Directive format]
   //  Preprocessing tokens following the #pragma omp are subject to macro
   //  replacement.
-  PP.AddPragmaHandler("omp",
-  new UnknownPragmaHandler("#pragma omp", Callbacks,
-   
/*RequireTokenExpansion=*/true));
+  std::unique_ptr OpenMPHandler(
+  new UnknownPragmaHandler("#pragma omp", Callbacks,
+   /*RequireTokenExpansion=*/true));
+  PP.AddPragmaHandler("omp", OpenMPHandler.get());
 
   PP.addPPCallbacks(std::unique_ptr(Callbacks));
 
@@ -820,4 +827,11 @@
   // Read all the preprocessed tokens, printing them out to the stream.
   PrintPreprocessedTokens(PP, Tok, Callbacks, *OS);
   *OS << '\n';
+
+  // Remove the handlers we just added to leave the preprocessor in a sane 
state
+  // so that it can be reused (for example by a clang::Parser instance).
+  PP.RemovePragmaHandler(MicrosoftExtHandler.get());
+  PP.RemovePragmaHandler("GCC", GCCHandler.get());
+  PP.RemovePragmaHandler("clang", ClangHandler.get());
+  PP.RemovePragmaHandler("omp", OpenMPHandler.get());
 }


Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -773,26 +773,33 @@
 
   // Expand macros in pragmas with -fms-extensions.  The assumption is that
   // the majority of pragmas in such a file will be Microsoft pragmas.
-  PP.AddPragmaHandler(new UnknownPragmaHandler(
-  "#pragma", Callbacks,
+  // Remember the handlers we will add so that we can remove them later.
+  std::unique_ptr MicrosoftExtHandler(
+  new UnknownPragmaHandler(
+  "#pragma", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+
+  std::unique_ptr GCCHandler(new UnknownPragmaHandler(
+  "#pragma GCC", Callbacks,
   /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
-  PP.AddPragmaHandler(
-  "GCC", new UnknownPragmaHandler(
- "#pragma GCC", Callbacks,
- /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
-  PP.AddPragmaHandler(
-  "clang", new UnknownPragmaHandler(
-   "#pragma clang", Callbacks,
-   /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+
+  std::unique_ptr ClangHandler(new UnknownPragmaHandler(
+  "#pragma clang", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+
+  PP.AddPragmaHandler(MicrosoftExtHandler.get());
+  PP.AddPragmaHandler("GCC", GCCHandler.get());
+  PP.AddPragmaHandler("clang", ClangHandler.get());
 
   // The tokens after pragma omp need to be expanded.
   //
   //  OpenMP [2.1, Directive format]
   //  Preprocessing tokens following the #pragma omp are subject to macro
   //  replacement.
-  PP.AddPragmaHandler("omp",
-  new UnknownPragmaHandler("#pragma omp", Callbacks,
-   /*RequireTokenExpansion=*/true));
+  std::unique_ptr OpenMPHandler(
+  new UnknownPragmaHandler("#pragma omp", Callbacks,
+   

[PATCH] D32248: CodeGen: Cast alloca to expected address space

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1105-1119
+  // Alloca always returns a pointer in alloca address space, which may
+  // be different from the type defined by the language. For example,
+  // in C++ the auto variables are in the default address space. Therefore
+  // cast alloca to the expected address space when necessary.
+  auto Addr = address.getPointer();
+  auto AddrTy = cast(Addr->getType());
+  auto ExpectedAddrSpace = 
CGM.getTypes().getVariableType(D)->getAddressSpace();

rjmccall wrote:
> yaxunl wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > Anastasia wrote:
> > > > > yaxunl wrote:
> > > > > > yaxunl wrote:
> > > > > > > t-tye wrote:
> > > > > > > > Is any assert done to ensure that it is legal to address space 
> > > > > > > > cast from variable address space to expected address space? 
> > > > > > > > Presumably the language, by definition, will only be causing 
> > > > > > > > legal casts. For example from alloca address space to generic 
> > > > > > > > (which includes the alloca address space).
> > > > > > > > 
> > > > > > > > For OpenCL, can you explain how the local variable can have the 
> > > > > > > > constant address space and use an alloca for allocation? 
> > > > > > > > Wouldn't a constant address space mean it was static and so 
> > > > > > > > should not be using alloca? And if it is using an alloca, how 
> > > > > > > > can it then be accessed as if it was in constant address space?
> > > > > > > If the auto var has address space qualifier specified through 
> > > > > > > `__attribute__((address_space(n)))`, there is not much we can 
> > > > > > > check in clang since it is target dependent. We will just emit 
> > > > > > > address space cast when necessary and let the backend check the 
> > > > > > > validity of the address space cast.
> > > > > > > 
> > > > > > > Otherwise, for OpenCL, we can assert the expected address space 
> > > > > > > is default (for OpenCL default address space in AST represents 
> > > > > > > private address space in source language) or constant. For other 
> > > > > > > languages we can assert the expected address space qualifier is 
> > > > > > > default (no address space qualifier). It is not convenient to 
> > > > > > > further check whether the emitted LLVM address space cast 
> > > > > > > instruction is valid since it requires target specific 
> > > > > > > information, therefore such check is better deferred to the 
> > > > > > > backend.
> > > > > > > 
> > > > > > > For OpenCL, currently automatic variable in constant address 
> > > > > > > space is emitted in private address space. For example, currently 
> > > > > > > Clang does not diagnose the following code
> > > > > > > 
> > > > > > > ```
> > > > > > > void f(global int* a) {
> > > > > > >   global int* constant p = a;
> > > > > > > }
> > > > > > > 
> > > > > > > ```
> > > > > > > Instead, it emits alloca for p, essentially treats it as `global 
> > > > > > > int* const p`. This seems to be a bug to me (or maybe we can call 
> > > > > > > it a feature? since there seems no better way to translate this 
> > > > > > > to LLVM IR, or simply diagnose this as an error). However, this 
> > > > > > > is better addressed by another patch.
> > > > > > 
> > > > > > Hi Anastasia,
> > > > > > 
> > > > > > Any comments about the automatic variable in constant address 
> > > > > > space? Thanks.
> > > > > From the spec s6.5.3 it feels like we should follow the same 
> > > > > implementation path in Clang for constant AS inside kernel function 
> > > > > as local AS. Because constant AS objects are essentially global 
> > > > > objects.
> > > > > 
> > > > >  Although, we didn't have any issues up to now because const just 
> > > > > does the trick of optimising the objects out eventually. I am not 
> > > > > clear if this creates any issue now with your allocation change. It 
> > > > > feels though that it should probably work fine just as is?
> > > > If these __constant locals are required to be const (or are implicitly 
> > > > const?) and have constant initializers, it seems to me the 
> > > > implementation obviously intended by the spec is that you allocate them 
> > > > statically in the constant address space.  It's likely that just 
> > > > implicitly making the variable static in Sema, the same way you make it 
> > > > implicitly const, will make IRGen and various other parts of the 
> > > > compiler just do the right thing.
> > > My patch does not change the current behaviour of Clang regarding 
> > > function-scope variable in constant address space. Basically there is no 
> > > issue if there is no address taken. However, if there is address taken, 
> > > there will be assertion due to casting private pointer to constant 
> > > address space. I think probably it is better to emit function-scope 
> > > variable in constant address space as global variable in constant address 
> > > space instead of alloca, as 

r301310 - [index] Record the 'SpecializationOf' relation for function specializations

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Apr 25 09:22:29 2017
New Revision: 301310

URL: http://llvm.org/viewvc/llvm-project?rev=301310=rev
Log:
[index] Record the 'SpecializationOf' relation for function specializations

rdar://31603531

Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp

Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=301310=301309=301310=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Tue Apr 25 09:22:29 2017
@@ -206,6 +206,10 @@ public:
   }
 }
 gatherTemplatePseudoOverrides(D, Relations);
+if (const auto *Base = D->getPrimaryTemplate())
+  Relations.push_back(
+  SymbolRelation(SymbolRoleSet(SymbolRole::RelationSpecializationOf),
+ Base->getTemplatedDecl()));
 
 TRY_DECL(D, IndexCtx.handleDecl(D, Roles, Relations));
 handleDeclarator(D);

Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=301310=301309=301310=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Tue Apr 25 09:22:29 2017
@@ -220,3 +220,38 @@ class ConflictingPseudoOverridesInSpecia
 // CHECK-NEXT: RelOver,RelSpecialization | foo | 
c:@ST>2#T#T@ConflictingPseudoOverridesInSpecialization@F@foo#t0.0#
 // CHECK-NEXT: RelOver,RelSpecialization | foo | 
c:@ST>2#T#T@ConflictingPseudoOverridesInSpecialization@F@foo#t0.1#
 };
+
+template
+void functionSpecialization();
+
+template
+void functionSpecialization() { }
+// CHECK: [[@LINE-1]]:6 | function/C | functionSpecialization | 
c:@FT@>3#T#T#NIfunctionSpecialization#v# |  | Def | rel: 0
+
+template<>
+void functionSpecialization();
+// CHECK: [[@LINE-1]]:6 | function(Gen,TS)/C++ | functionSpecialization | 
c:@F@functionSpecialization<#I#I#VI0># | __Z22functionSpecializationIiiLi0EEvv 
| Decl,RelSpecialization | rel: 1
+// CHECK-NEXT: RelSpecialization | functionSpecialization | 
c:@FT@>3#T#T#NIfunctionSpecialization#v#
+
+template<>
+void functionSpecialization() { }
+// CHECK: [[@LINE-1]]:6 | function(Gen,TS)/C++ | functionSpecialization | 
c:@F@functionSpecialization<#I#I#VI0># | __Z22functionSpecializationIiiLi0EEvv 
| Def,RelSpecialization | rel: 1
+// CHECK-NEXT: RelSpecialization | functionSpecialization | 
c:@FT@>3#T#T#NIfunctionSpecialization#v#
+
+struct ContainsSpecializedMemberFunction {
+  template
+  void memberSpecialization();
+};
+
+template
+void ContainsSpecializedMemberFunction::memberSpecialization() {
+// CHECK: [[@LINE-1]]:41 | instance-method/C++ | memberSpecialization | 
c:@S@ContainsSpecializedMemberFunction@FT@>1#TmemberSpecialization#v# | 
 | Def,RelChild | rel: 1
+// CHECK-NEXT: RelChild
+}
+
+template<>
+void ContainsSpecializedMemberFunction::memberSpecialization() {
+// CHECK: [[@LINE-1]]:41 | instance-method(Gen,TS)/C++ | memberSpecialization 
| c:@S@ContainsSpecializedMemberFunction@F@memberSpecialization<#I># | 
__ZN33ContainsSpecializedMemberFunction20memberSpecializationIiEEvv | 
Def,RelChild,RelSpecialization | rel: 2
+// CHECK-NEXT: RelChild
+// CHECK-NEXT: RelSpecialization | memberSpecialization | 
c:@S@ContainsSpecializedMemberFunction@FT@>1#TmemberSpecialization#v#
+}


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

[PATCH] D32486: Cleanup pragma handlers after DoPrintPreprocessedInput

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren added a comment.

Please move the =new out of the PP.AddPragmaHandler calls.
While at it, this code still (as the original) leaks the PragmaHandlers. These 
should be deleted after RemovePragmaHandler or instead, simply use 
std::unique_ptr.


https://reviews.llvm.org/D32486



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

[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D30489#728945, @danielmarjamaki wrote:

> I would propose that I rename and cleanup RangeConstraintManager::uglyEval() 
> and add it. When I tested it, the Z3 does not seem to handle this.


What do you mean by Z3 not handling this? I mean, if the evalEq returns an 
unknown SVal, Z3 can not do anything with that. Does Z3 fails after the evalEq 
returns the correct SVal?

I wonder that is the reason that we do not produce the correct SVal right now. 
Maybe the reason was that, we do not want to produce SVals that the constraint 
manager can not solve anyways, to improve performance. In general the current 
constraint manager does not reason about SymSymExprs. In order to preserve the 
performance we might not want to start produce all kinds of SymSymExprs, only 
the cases we already handle in the constraint managers.

E.g.: it might make sense to produce something like ` $a - $b =0`, so 
https://reviews.llvm.org/rL300178 can kick in, and in some cases we might be 
able to find the bug (e.g. the function in the example was called with a 
concrete int).


Repository:
  rL LLVM

https://reviews.llvm.org/D30489



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

[PATCH] D32486: Cleanup pragma handlers after DoPrintPreprocessedInput

[Raphael Isemann via Phabricator via cfe-commits]

teemperor created this revision.

The UnknownPragmaHandlers added by t`DoPrintPreprocessedInput` conflict with 
the real PragmaHandlers from clang::Parser because they try to handle the same 
`#pragma` directives. This makes it impossible to use a Preprocessor (that was 
previously passed to DoPrintPreprocessedInput), as an Preprocessor for a 
`clang::Parser` instance which is what we currently do in cling. This patch 
removes the added UnknownPragmaHandler to avoid conflicts these conflicts and 
leave the PragmaHandlers of the Preprocessors in a the same state as before 
calling `DoPrintPreprocessedInput`.


https://reviews.llvm.org/D32486

Files:
  lib/Frontend/PrintPreprocessedOutput.cpp


Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -771,27 +771,37 @@
   PP, *OS, !Opts.ShowLineMarkers, Opts.ShowMacros,
   Opts.ShowIncludeDirectives, Opts.UseLineDirectives);
 
+  // Remember the handlers we will add so that we can remove them later.
+  UnknownPragmaHandler *MicrosoftExtHandler;
+  UnknownPragmaHandler *GCCHandler;
+  UnknownPragmaHandler *ClangHandler;
+  UnknownPragmaHandler *OpenMPHandler;
+
   // Expand macros in pragmas with -fms-extensions.  The assumption is that
   // the majority of pragmas in such a file will be Microsoft pragmas.
-  PP.AddPragmaHandler(new UnknownPragmaHandler(
-  "#pragma", Callbacks,
-  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
   PP.AddPragmaHandler(
-  "GCC", new UnknownPragmaHandler(
- "#pragma GCC", Callbacks,
- /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+  MicrosoftExtHandler = new UnknownPragmaHandler(
+  "#pragma", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+  PP.AddPragmaHandler(
+  "GCC",
+  GCCHandler = new UnknownPragmaHandler(
+  "#pragma GCC", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
   PP.AddPragmaHandler(
-  "clang", new UnknownPragmaHandler(
-   "#pragma clang", Callbacks,
-   /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+  "clang",
+  ClangHandler = new UnknownPragmaHandler(
+  "#pragma clang", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
 
   // The tokens after pragma omp need to be expanded.
   //
   //  OpenMP [2.1, Directive format]
   //  Preprocessing tokens following the #pragma omp are subject to macro
   //  replacement.
-  PP.AddPragmaHandler("omp",
-  new UnknownPragmaHandler("#pragma omp", Callbacks,
+  PP.AddPragmaHandler(
+  "omp",
+  OpenMPHandler = new UnknownPragmaHandler("#pragma omp", Callbacks,

/*RequireTokenExpansion=*/true));
 
   PP.addPPCallbacks(std::unique_ptr(Callbacks));
@@ -820,4 +830,11 @@
   // Read all the preprocessed tokens, printing them out to the stream.
   PrintPreprocessedTokens(PP, Tok, Callbacks, *OS);
   *OS << '\n';
+
+  // Remove the handlers we just added to leave the preprocessor in a sane 
state
+  // so that it can be reused (for example by a clang::Parser instance).
+  PP.RemovePragmaHandler(MicrosoftExtHandler);
+  PP.RemovePragmaHandler("GCC", GCCHandler);
+  PP.RemovePragmaHandler("clang", ClangHandler);
+  PP.RemovePragmaHandler("omp", OpenMPHandler);
 }


Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -771,27 +771,37 @@
   PP, *OS, !Opts.ShowLineMarkers, Opts.ShowMacros,
   Opts.ShowIncludeDirectives, Opts.UseLineDirectives);
 
+  // Remember the handlers we will add so that we can remove them later.
+  UnknownPragmaHandler *MicrosoftExtHandler;
+  UnknownPragmaHandler *GCCHandler;
+  UnknownPragmaHandler *ClangHandler;
+  UnknownPragmaHandler *OpenMPHandler;
+
   // Expand macros in pragmas with -fms-extensions.  The assumption is that
   // the majority of pragmas in such a file will be Microsoft pragmas.
-  PP.AddPragmaHandler(new UnknownPragmaHandler(
-  "#pragma", Callbacks,
-  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
   PP.AddPragmaHandler(
-  "GCC", new UnknownPragmaHandler(
- "#pragma GCC", Callbacks,
- /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+  MicrosoftExtHandler = new UnknownPragmaHandler(
+  "#pragma", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
+  PP.AddPragmaHandler(
+  "GCC",
+  GCCHandler = new UnknownPragmaHandler(
+  "#pragma GCC", Callbacks,
+  /*RequireTokenExpansion=*/PP.getLangOpts().MicrosoftExt));
   PP.AddPragmaHandler(
-  "clang", 

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D32439#736631, @Serge_Preis wrote:

> What do you think?


Seems reasonable.


https://reviews.llvm.org/D32439



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

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.

Ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D30295



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

[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.
This revision now requires changes to proceed.

Ping. Any comments?


Repository:
  rL LLVM

https://reviews.llvm.org/D30489



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

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.

Ping


Repository:
  rL LLVM

https://reviews.llvm.org/D31029



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

Re: r289018 - [Headers] Enable #include_next on Darwin

[Aaron Ballman via cfe-commits]

On Wed, Dec 7, 2016 at 9:13 PM, Bruno Cardoso Lopes via cfe-commits
 wrote:
> Author: bruno
> Date: Wed Dec  7 20:13:56 2016
> New Revision: 289018
>
> URL: http://llvm.org/viewvc/llvm-project?rev=289018=rev
> Log:
> [Headers] Enable #include_next on Darwin
>
> Allows darwin targets to provide additional definitions and
> implementation specifc values for float.h
>
> rdar://problem/21961491
>
> Added:
> cfe/trunk/test/Headers/Inputs/usr/
> cfe/trunk/test/Headers/Inputs/usr/include/
> cfe/trunk/test/Headers/Inputs/usr/include/float.h
> cfe/trunk/test/Headers/float-darwin.c
> Modified:
> cfe/trunk/lib/Headers/float.h

This commit appears to have caused a regression:
https://bugs.llvm.org//show_bug.cgi?id=31504

I am running into this on a Snow Leopard system as well, where the
Integration tests are now failing because float.h cannot be found by
the system #include_next. I was thinking of modifying
lib/Headers/float.h to use the SDK version as mentioned in the bug
report, but I share the reporter's question: why was this change
needed in the first place? I couldn't find a review for the commit,
and don't have access to the rdar link.

Thanks!

~Aaron

>
> Modified: cfe/trunk/lib/Headers/float.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/float.h?rev=289018=289017=289018=diff
> ==
> --- cfe/trunk/lib/Headers/float.h (original)
> +++ cfe/trunk/lib/Headers/float.h Wed Dec  7 20:13:56 2016
> @@ -27,9 +27,12 @@
>  /* If we're on MinGW, fall back to the system's float.h, which might have
>   * additional definitions provided for Windows.
>   * For more details see http://msdn.microsoft.com/en-us/library/y0ybw9fy.aspx
> + *
> + * Also fall back on Darwin to allow additional definitions and
> + * implementation-defined values.
>   */
> -#if (defined(__MINGW32__) || defined(_MSC_VER)) && __STDC_HOSTED__ && \
> -__has_include_next()
> +#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
> +__STDC_HOSTED__ && __has_include_next()
>  #  include_next 
>
>  /* Undefine anything that we'll be redefining below. */
>
> Added: cfe/trunk/test/Headers/Inputs/usr/include/float.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/Inputs/usr/include/float.h?rev=289018=auto
> ==
> --- cfe/trunk/test/Headers/Inputs/usr/include/float.h (added)
> +++ cfe/trunk/test/Headers/Inputs/usr/include/float.h Wed Dec  7 20:13:56 2016
> @@ -0,0 +1,6 @@
> +#ifndef SYSFLOAT_H
> +#define SYSFLOAT_H
> +
> +#define FLT_HAS_SUBNORM 1
> +
> +#endif /* SYSFLOAT_H */
>
> Added: cfe/trunk/test/Headers/float-darwin.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/float-darwin.c?rev=289018=auto
> ==
> --- cfe/trunk/test/Headers/float-darwin.c (added)
> +++ cfe/trunk/test/Headers/float-darwin.c Wed Dec  7 20:13:56 2016
> @@ -0,0 +1,13 @@
> +// REQUIRES: system-darwin
> +// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only -std=c11 
> -isysroot %S/Inputs %s
> +#include 
> +
> +// Test the #include_next on float.h works on Darwin.
> +#ifndef FLT_HAS_SUBNORM
> +  #error "FLT_HAS_SUBNORM not defined"
> +#endif
> +
> +// Test that definition from builtin are also present.
> +#ifndef FLT_MAX
> +  #error "FLT_MAX not defined"
> +#endif
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: lib/AST/ASTContext.cpp:1497
+  auto FnUnitCacheEntry = FunctionAstUnitMap.find(MangledFnName);
+  if (FnUnitCacheEntry == FunctionAstUnitMap.end()) {
+if (FunctionFileMap.empty()) {

danielmarjamaki wrote:
> How about replacing FunctionAstUnitMap.find() with FunctionAstUnitMap.count()?
> 
`FnUnitCacheEntry` is used in line `1525`.


https://reviews.llvm.org/D30691



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

[PATCH] D32424: Add a fix-it for -Wunguarded-availability

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 96538.
arphaman added a comment.

Now the patch takes the following situations into account:

- Enclose only the statement in a `case`.
- If the fixit has to enclose a declaration statement, then the fixit will try 
to enclose the appropriate uses as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D32424

Files:
  include/clang/Lex/Lexer.h
  lib/Lex/Lexer.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/FixIt/fixit-availability.c
  test/FixIt/fixit-availability.mm

Index: test/FixIt/fixit-availability.mm
===
--- /dev/null
+++ test/FixIt/fixit-availability.mm
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunguarded-availability -fdiagnostics-parseable-fixits -triple x86_64-apple-darwin9 %s 2>&1 | FileCheck %s
+
+__attribute__((availability(macos, introduced=10.12)))
+int function(void);
+
+void anotherFunction(int function);
+
+int use() {
+  function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:14-[[@LINE-2]]:14}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+  int y = function(), x = 0;
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:29-[[@LINE-2]]:29}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+  x += function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:19-[[@LINE-2]]:19}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+  if (1) {
+x = function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+  }
+  anotherFunction(function());
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:31-[[@LINE-2]]:31}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+  if (function()) {
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE+1]]:4-[[@LINE+1]]:4}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+  }
+  while (function())
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE+1]]:6-[[@LINE+1]]:6}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+;
+  do
+function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n"
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n} else {\n// Fallback on earlier versions\n}"
+  while (1);
+  for (int i = 0; i < 10; ++i)
+function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n"
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n} else {\n// Fallback on earlier versions\n}"
+  switch (x) {
+  case 0:
+function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n"
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n} else {\n// Fallback on earlier versions\n}"
+  case 2:
+anotherFunction(1);
+function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n"
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n} else {\n// Fallback on earlier versions\n}"
+break;
+  default:
+function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n"
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n} else {\n// Fallback on earlier versions\n}"
+break;
+  }
+  return function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:21-[[@LINE-2]]:21}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+}
+
+#define MYFUNCTION function
+
+#define MACRO_ARGUMENT(X) X
+#define MACRO_ARGUMENT_SEMI(X) X;
+#define MACRO_ARGUMENT_2(X) if (1) X;
+
+#define INNER_MACRO if (1) MACRO_ARGUMENT(function()); else ;
+
+void useInMacros() {
+  MYFUNCTION();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n  } else {\n  // Fallback on earlier versions\n  }"
+
+  MACRO_ARGUMENT_SEMI(function())
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n  "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:34-[[@LINE-2]]:34}:"\n  } else {\n 

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:8
+
+In the example code below the developer probably wanted to make room for an 
extra char in the allocation but misplaced the addition.
+

danielmarjamaki wrote:
> JonasToth wrote:
> > when the intend was to allocate one more char, he would need to do 
> > `strlen(s) + 1`, why is it changed to subtraction then?
> If I change it to strlen(s) + 1 then the logic of the program is changed.
> 
> If I change it to subtraction then the logic is the same and the program is 
> still wrong, but imho it is easier to see.
it should be made clear in the docs, that the code is bad, especially if its 
UB. the dangers of copy & paste ;) (even its unlikely that this will be 
copy).



Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20
+char *p = new char[(strlen(s) - 1)]
+strcpy(p, s);
+

danielmarjamaki wrote:
> JonasToth wrote:
> > isnt that an overflow?
> > an example:
> > `strlen(s) == 10` -> `p` will be 9 characters long, since its substracted 
> > with `1`.
> > 
> > the copy operation will then copy the content of `s` into `p`, therefore 
> > copying 10 characters into a buffer of length 9.
> > 
> > as i understand it `strcpy(p, s + 1)` would be correct with the sizes.
> yes it is overflow. My intention was to show that strlen(s+1) syntax is 
> dangerous.
ok. please state that the overflow in a comment, its better to make that 
explicit.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346



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

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Simon Dardis via Phabricator via cfe-commits]

sdardis updated this revision to Diff 96533.
sdardis added a comment.

Rebase + ping.


https://reviews.llvm.org/D25866

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Sema/vector-cast.c
  test/Sema/vector-gcc-compat.c
  test/Sema/vector-gcc-compat.cpp
  test/Sema/vector-ops.c
  test/Sema/zvector.c
  test/SemaCXX/vector-no-lax.cpp

Index: test/SemaCXX/vector-no-lax.cpp
===
--- test/SemaCXX/vector-no-lax.cpp
+++ test/SemaCXX/vector-no-lax.cpp
@@ -4,6 +4,6 @@
 
 vSInt32 foo (vUInt32 a) {
   vSInt32 b = { 0, 0, 0, 0 };
-  b += a; // expected-error{{cannot convert between vector values}}
+  b += a; // expected-error{{cannot convert between vector type 'vUInt32' (vector of 4 'unsigned int' values) and vector type 'vSInt32' (vector of 4 'int' values) as implicit conversion would cause truncation}}
   return b;
 }
Index: test/Sema/zvector.c
===
--- test/Sema/zvector.c
+++ test/Sema/zvector.c
@@ -326,14 +326,14 @@
   bc = bc + sc2; // expected-error {{incompatible type}}
   bc = sc + bc2; // expected-error {{incompatible type}}
 
-  sc = sc + sc_scalar; // expected-error {{cannot convert}}
-  sc = sc + uc_scalar; // expected-error {{cannot convert}}
-  sc = sc_scalar + sc; // expected-error {{cannot convert}}
-  sc = uc_scalar + sc; // expected-error {{cannot convert}}
-  uc = uc + sc_scalar; // expected-error {{cannot convert}}
-  uc = uc + uc_scalar; // expected-error {{cannot convert}}
-  uc = sc_scalar + uc; // expected-error {{cannot convert}}
-  uc = uc_scalar + uc; // expected-error {{cannot convert}}
+  sc = sc + sc_scalar;
+  sc = sc + uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  sc = sc_scalar + sc;
+  sc = uc_scalar + sc; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc = uc + sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc + uc_scalar;
+  uc = sc_scalar + uc; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc = uc_scalar + uc;
 
   ss = ss + ss2;
   us = us + us2;
@@ -368,10 +368,10 @@
   sc += sl2; // expected-error {{cannot convert}}
   sc += fd2; // expected-error {{cannot convert}}
 
-  sc += sc_scalar; // expected-error {{cannot convert}}
-  sc += uc_scalar; // expected-error {{cannot convert}}
-  uc += sc_scalar; // expected-error {{cannot convert}}
-  uc += uc_scalar; // expected-error {{cannot convert}}
+  sc += sc_scalar;
+  sc += uc_scalar; // expected-error {{cannot convert between scalar type 'unsigned char' and vector type '__vector signed char' (vector of 16 'signed char' values) as implicit conversion would cause truncation}}
+  uc += sc_scalar; // expected-error {{implicit conversion changes signedness: 'signed char' to '__vector unsigned char' (vector of 16 'unsigned char' values)}}
+  uc += uc_scalar;
 
   ss += ss2;
   us += us2;
Index: test/Sema/vector-ops.c
===
--- test/Sema/vector-ops.c
+++ test/Sema/vector-ops.c
@@ -13,11 +13,11 @@
   (void)(~v2fa); // expected-error{{invalid argument type 'v2f' (vector of 2 'float' values) to unary}}
 
   // Comparison operators
-  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
+  v2ua = (v2ua==v2sa); // expected-warning{{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values}}
   v2sa = (v2ua==v2sa);
 
   // Arrays
-  int array1[v2ua]; // expected-error{{size of array has non-integer type 'v2u' (vector of 2 'unsigned int' values)}}
+  int array1[v2ua]; // expected-error{{size of array has non-integer type 'v2u' (vector of 2 'unsigned int' values}}
   int array2[17];
   // FIXME: error message below needs type!
   (void)(array2[v2ua]); // expected-error{{array subscript is not an integer}}
@@ -28,108 +28,108 @@
 }
 
 void testLogicalVecVec(v2u v2ua, v2s v2sa, v2f v2fa) {
-
   // Logical operators
-  v2ua = v2ua && v2ua; // expected-warning {{incompatible vector types assigning to 'v2u' (vector of 2 'unsigned int' values) from '__attribute__((__vector_size__(2 * sizeof(int int' (vector of 2 'int' values)}}
-  v2ua = v2ua || v2ua; // expected-warning {{incompatible vector 

[PATCH] D31496: Make -defsym a driver option

[Salman Arif via Phabricator via cfe-commits]

salari01 abandoned this revision.
salari01 added a comment.

In https://reviews.llvm.org/D31496#721262, @rnk wrote:

> `-Wl,--defsym` is also a pretty common linker option. I'm hesitant to add 
> this to the driver interface, since it seems like they could easily be 
> confused.


I see. Was not aware of that linker option. Having looked it up now, I agree 
that the risk of confusion with the linker option outweighs the convenience of 
having this as a Driver option. I'll adandon this revision.


https://reviews.llvm.org/D31496



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

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

[Serge Preis via Phabricator via cfe-commits]

Serge_Preis added a comment.

In https://reviews.llvm.org/D32439#736503, @malcolm.parsons wrote:

> Is it possible to add a test for this change?
>  There are some source range checks in test/Misc/ast-dump-decl.cpp


I looked into test/Misc/ast-dump-decl.cpp and other tests around, but they all 
use -ast-dump which doesn't dump source locations of affected entities (at 
least for problematic codes I've got). The only tool that exposed the 
difference is c-index-test -test-load-source all bug.cpp
bug.cpp:

  template
  struct Derived:  MyBase::InnerIterator
  {
  Derived() : MyBase::InnerIterator() {}
  };

Bad (trunk) has:

  // CHECK: bug.cpp:4:25: TypeRef=MyBase:1:19 Extent=[4:25 - 4:38]

Good (patched) has:

  // CHECK: bug.cpp:4:17: TypeRef=MyBase:1:19 Extent=[4:17 - 4:23]

I am new to clang development, but think that this difference can be exploited 
to craft proper test.

Something like:

  // RUN: c-index-test -test-load-source all %s | FileCheck %s
  template
  struct Derived:  MyBase::InnerIterator
  {
  Derived() : MyBase::InnerIterator() {}
  // CHECK:  TypeRef=MyBase:1:19 Extent=[4:17 - 4:23]
  };

What do you think? 
I may add some other cases similar to this to the test including other variants 
of dependent and independent names.


https://reviews.llvm.org/D32439



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

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added inline comments.



Comment at: lib/AST/ASTContext.cpp:1495
+  ASTUnit *Unit = nullptr;
+  StringRef ASTFileName;
+  auto FnUnitCacheEntry = FunctionAstUnitMap.find(MangledFnName);

As far as I see you can move this ASTFileName declaration down.



Comment at: lib/AST/ASTContext.cpp:1497
+  auto FnUnitCacheEntry = FunctionAstUnitMap.find(MangledFnName);
+  if (FnUnitCacheEntry == FunctionAstUnitMap.end()) {
+if (FunctionFileMap.empty()) {

How about replacing FunctionAstUnitMap.find() with FunctionAstUnitMap.count()?




Comment at: lib/AST/ASTContext.cpp:1511
+auto It = FunctionFileMap.find(MangledFnName);
+if (It != FunctionFileMap.end())
+  ASTFileName = It->second;

As far as I see this can be changed to:
```
  if (It == FunctionFileMap.end())
return nullptr;
  const StringRef ASTFileName = It->second;
```



Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:48
+  : Ctx(Context), ItaniumCtx(MangleCtx) {}
+  std::string CurrentFileName;
+

As far I see CurrentFileName is only used in 1 method and should therefore be 
private.



Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:86
+  SM.getFileEntryForID(SM.getMainFileID())->getName();
+  char *Path = realpath(SMgrName.str().c_str(), nullptr);
+  CurrentFileName = Path;

I believe realpath is a posix function.


https://reviews.llvm.org/D30691



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

r301302 - Fix fuzzer.c test on platforms where CLANG_DEFAULT_CXX_STDLIB is libc++

[Ismail Donmez via cfe-commits]

Author: ismail
Date: Tue Apr 25 06:24:14 2017
New Revision: 301302

URL: http://llvm.org/viewvc/llvm-project?rev=301302=rev
Log:
Fix fuzzer.c test on platforms where CLANG_DEFAULT_CXX_STDLIB is libc++

Modified:
cfe/trunk/test/Driver/fuzzer.c

Modified: cfe/trunk/test/Driver/fuzzer.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fuzzer.c?rev=301302=301301=301302=diff
==
--- cfe/trunk/test/Driver/fuzzer.c (original)
+++ cfe/trunk/test/Driver/fuzzer.c Tue Apr 25 06:24:14 2017
@@ -7,7 +7,7 @@
 // CHECK-COVERAGE-SAME: -fsanitize-coverage-indirect-calls
 // CHECK-COVERAGE-SAME: -fsanitize-coverage-trace-cmp
 
-// RUN: %clang -fsanitize=fuzzer -target i386-unknown-linux %s -### 2>&1 | 
FileCheck --check-prefixes=CHECK-LIBCXX-LINUX %s
+// RUN: %clang -fsanitize=fuzzer -target i386-unknown-linux -stdlib=platform 
%s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBCXX-LINUX %s
 //
 // CHECK-LIBCXX-LINUX: -lstdc++
 


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

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added inline comments.



Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:8
+
+In the example code below the developer probably wanted to make room for an 
extra char in the allocation but misplaced the addition.
+

JonasToth wrote:
> when the intend was to allocate one more char, he would need to do `strlen(s) 
> + 1`, why is it changed to subtraction then?
If I change it to strlen(s) + 1 then the logic of the program is changed.

If I change it to subtraction then the logic is the same and the program is 
still wrong, but imho it is easier to see.



Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20
+char *p = new char[(strlen(s) - 1)]
+strcpy(p, s);
+

JonasToth wrote:
> isnt that an overflow?
> an example:
> `strlen(s) == 10` -> `p` will be 9 characters long, since its substracted 
> with `1`.
> 
> the copy operation will then copy the content of `s` into `p`, therefore 
> copying 10 characters into a buffer of length 9.
> 
> as i understand it `strcpy(p, s + 1)` would be correct with the sizes.
yes it is overflow. My intention was to show that strlen(s+1) syntax is 
dangerous.



Comment at: test/clang-tidy/readability-strlen-argument.cpp:11
+  X = strlen(Str + 10);
+  // CHECK-FIXES: {{^}}  X = (strlen(Str) - 10);{{$}}
+

JonasToth wrote:
> sberg wrote:
> > but if any of the first ten chars in Str can be NUL, then the change is 
> > bogus?  (I guess I fail to see the reason for this check)
> intersting point. If the Array holds a long list of string concatenated and 
> seperated by `0`, the offsetting would be a valid usecase.
> (are strings in programm executables stored like this?)
> 
> But i have no idea if that is actually a scenario. usually i use 
> `std::string` :)
I think that in theory, you have a point. It would be best that such users 
don't use this check. I doubt that is a big problem in practice. We don't need 
to speculate much, I will test this on all debian source code..

It's possible that strings in program executables are stored like that, but I'd 
say it's ub to calculate the address Str+10 and then dereference that if Str is 
a string that is 10 bytes long.



Repository:
  rL LLVM

https://reviews.llvm.org/D32346



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

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> #include 
>  void dostuff(const char *P) {
> 
>   if (strncmp(P+2,"xyz",3)==0) {}
> 
> }
> 
>   

Iam not super familiar with C, but the intend of the function is to check the 
following:

  P = "foxyz", P2 = "abxyz", P3 = "opxyz", ...

And if P matches this kind of string pattern.

> /home/danielm/strlen.c:3:16: warning: strlen() argument has pointer addition, 
> it is recommended to subtract the result instead. 
> [readability-strlen-argument]
> 
>   if (strncmp(P+2,"xyz",3)==0) {}
>^
> 
>   

Why is it matched? This is the code transformation from -O2? I dont know a way 
to surround that, and i think you should not call clang-tidy with optimization.

> I should probably avoid these, I guess skipping all warnings in macro code 
> sounds ok to me.

You can check if the code is a result of macro expansion, i think that would be 
enough.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346



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

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.

I am thinking about making my check more strict so it only warns in 
allocations. I believe the example code is much more motivating when there is 
allocation.

In https://reviews.llvm.org/D32346#733430, @JonasToth wrote:

> My thoughts on the check added.
>  Have you run it over a big codebase? What is the turnout?
>
> And please add the check to the ReleaseNotes.


I have a script that runs clang/clang-tidy on all debian source code. It 
basically grabs all packages and if it has a configure script it runs:  
./configure && bear make && clang-tidy  .. Running that right now.

It goes slowly I have run clang-tidy on 22 packages (735 files) so far and got 
13 warnings.

Unfortunately most warnings so far are false positives as in this example code:

  #include 
  void dostuff(const char *P) {
if (strncmp(P+2,"xyz",3)==0) {}
  }

Without -O2 I get no warning. With -O2 I get a false positive:

  danielm@debian:~$ ~/llvm/build/bin/clang-tidy 
-checks=-*,readability-strlen-argument strlen.c -- -O2
  1 warning generated.
  /home/danielm/strlen.c:3:16: warning: strlen() argument has pointer addition, 
it is recommended to subtract the result instead. [readability-strlen-argument]
if (strncmp(P+2,"xyz",3)==0) {}
 ^

When the -O2 flag is used then on my machine the strncmp() function call is 
expanded to lots of code. and in that code, there are calls to strlen().

I should probably avoid these, I guess skipping all warnings in macro code 
sounds ok to me.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346



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

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:8
+
+In the example code below the developer probably wanted to make room for an 
extra char in the allocation but misplaced the addition.
+

when the intend was to allocate one more char, he would need to do `strlen(s) + 
1`, why is it changed to subtraction then?



Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20
+char *p = new char[(strlen(s) - 1)]
+strcpy(p, s);
+

isnt that an overflow?
an example:
`strlen(s) == 10` -> `p` will be 9 characters long, since its substracted with 
`1`.

the copy operation will then copy the content of `s` into `p`, therefore 
copying 10 characters into a buffer of length 9.

as i understand it `strcpy(p, s + 1)` would be correct with the sizes.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346



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

[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki updated this revision to Diff 96526.
danielmarjamaki added a comment.

Fixed review comments. Made code examples and documentation more motivational.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/StrlenArgumentCheck.cpp
  clang-tidy/readability/StrlenArgumentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-strlen-argument.rst
  test/clang-tidy/readability-strlen-argument.cpp

Index: test/clang-tidy/readability-strlen-argument.cpp
===
--- test/clang-tidy/readability-strlen-argument.cpp
+++ test/clang-tidy/readability-strlen-argument.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s readability-strlen-argument %t
+
+namespace std {
+typedef __typeof(sizeof(int)) size_t;
+size_t strlen(const char *s);
+}; // namespace std
+using namespace std;
+
+void warn1(const char *Str) {
+  char *P;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:27: warning: strlen() argument has pointer addition, it is recommended to subtract the result instead. [readability-strlen-argument]
+  P = new char[strlen(Str + 1)];
+  // CHECK-FIXES: {{^}}  P = new char[(strlen(Str) - 1)];{{$}}
+  delete[] P;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:30: warning: strlen() argument has pointer addition, it is recommended to subtract the result instead. [readability-strlen-argument]
+  P = new char[std::strlen(1 + Str)];
+  delete[] P;
+}
+
+struct S {
+  char *P;
+};
+void warn2(const struct S ) {
+  char *P;
+  // CHECK-MESSAGES: :[[@LINE+1]]:34: warning: strlen() argument has pointer addition, it is recommended to subtract the result instead. [readability-strlen-argument]
+  P = new char[std::strlen(Par.P + 1)];
+  // CHECK-FIXES: {{^}}  P = new char[(std::strlen(Par.P) - 1)];{{$}}
+  delete[] P;
+}
Index: docs/clang-tidy/checks/readability-strlen-argument.rst
===
--- docs/clang-tidy/checks/readability-strlen-argument.rst
+++ docs/clang-tidy/checks/readability-strlen-argument.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - readability-strlen-argument
+
+readability-strlen-argument
+===
+
+This check will detect addition in `strlen()` argument.
+
+In the example code below the developer probably wanted to make room for an extra char in the allocation but misplaced the addition.
+
+.. code-block:: c++
+
+char *p = new char[strlen(s + 1)];
+strcpy(p, s);
+
+With -fix that becomes:
+
+.. code-block:: c++
+
+char *p = new char[(strlen(s) - 1)]
+strcpy(p, s);
+
+For readability it is considered better to subtract the result outside the `strlen()`.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -169,4 +169,5 @@
readability-redundant-string-init
readability-simplify-boolean-expr
readability-static-definition-in-anonymous-namespace
+   readability-strlen-argument
readability-uniqueptr-delete-release
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -76,6 +76,12 @@
 
   Finds functions that have more then `ParameterThreshold` parameters and emits a warning.
 
+- New `readability-strlen-argument
+  `_ check
+
+  Finds misleading `strlen()` argument. If addition is used in the argument then the effect is that
+  the strlen() result is subtracted. 
+
 - New `hicpp` module
 
   Adds checks that implement the `High Integrity C++ Coding Standard `_ and other safety
Index: clang-tidy/readability/StrlenArgumentCheck.h
===
--- clang-tidy/readability/StrlenArgumentCheck.h
+++ clang-tidy/readability/StrlenArgumentCheck.h
@@ -0,0 +1,35 @@
+//===--- StrlenArgumentCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLEN_ARGUMENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRLEN_ARGUMENT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Detect pointer addition in strlen() argument.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-strlen-argument.html
+class StrlenArgumentCheck : public ClangTidyCheck {
+public:
+  

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

[Serge Preis via Phabricator via cfe-commits]

Serge_Preis added a comment.



In https://reviews.llvm.org/D32439#736503, @malcolm.parsons wrote:

> Is it possible to add a test for this change?
>  There are some source range checks in test/Misc/ast-dump-decl.cpp


Yes, I think testing is quite possible. I will take a look into 
test/Misc/ast-dump-decl.cpp and try to come up with something meaningful.

The code exposed this issue for me looked like this:

  template
  bool checkNameAtPos(const EntityT entity) {
   std::string name = GetName(entity);  /// EntityT-dependent way to 
discover name 
   SourceLoc start = GetStart(entity);/// entity.getBeginLoc() by 
default with rare exceptions
  
  const char* buf = SourceMgr.getCharacterData(start, );
  assert(!err && "getCharacterData() failed");
  
  if (buf[0] != name[0]) {
  llvm::errs() << "Anchor text doesn't match the symbol:\n"
  << "  Name = " << nameStr << "\n"
  << "  @Anchor = " << bufStr << "\n"
  start.print(llvm::errs(), SourceMgr);
  llvm::errs() << '\n';
  return false;
  }
  return true;
  }

I wanted to be sure that there will be a name of an enity in the entity's 
location (at least prefix of entity's name). So tesing for something like this 
may be added for entities being TypeLoc, NestedNameSpecifier and 
CXXCtorInitializer.


https://reviews.llvm.org/D32439



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

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

Is it possible to add a test for this change?
There are some source range checks in test/Misc/ast-dump-decl.cpp


https://reviews.llvm.org/D32439



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

[PATCH] D32427: Fix float abi for SUSE ARM triples

[İsmail Dönmez via Phabricator via cfe-commits]

ismail abandoned this revision.
ismail added a comment.

compnerd has a better fix.


https://reviews.llvm.org/D32427



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

[PATCH] D32436: [clang-tidy] Support detecting for-range loop in inefficient-vector-operation check.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 96516.
hokein marked an inline comment as done.
hokein added a comment.

Fix doc.


https://reviews.llvm.org/D32436

Files:
  clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tidy/performance/InefficientVectorOperationCheck.h
  docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  test/clang-tidy/performance-inefficient-vector-operation.cpp

Index: test/clang-tidy/performance-inefficient-vector-operation.cpp
===
--- test/clang-tidy/performance-inefficient-vector-operation.cpp
+++ test/clang-tidy/performance-inefficient-vector-operation.cpp
@@ -1,9 +1,27 @@
-// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- -format-style=llvm --
+// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- -format-style=llvm -- --std=c++11
 
 namespace std {
 
 typedef int size_t;
 
+template class initializer_list {
+public:
+  using value_type = E;
+  using reference = E&;
+  using const_reference = const E&;
+  using size_type = size_t;
+  using iterator = const E*;
+  using const_iterator = const E*;
+  initializer_list();
+  size_t size() const; // number of elements
+  const E* begin() const; // first element
+  const E* end() const; // one past the last element
+};
+
+// initializer list range access
+template const E* begin(initializer_list il);
+template const E* end(initializer_list il);
+
 template 
 class vector {
  public:
@@ -23,9 +41,24 @@
   size_t size();
   const_reference operator[] (size_type) const;
   reference operator[] (size_type);
+
+  const_iterator begin() const;
+  const_iterator end() const;
 };
 } // namespace std
 
+class Foo {
+ public:
+  explicit Foo(int);
+};
+
+class Bar {
+ public:
+  Bar(int);
+};
+
+int Op(int);
+
 void f(std::vector& t) {
   {
 std::vector v;
@@ -85,7 +118,38 @@
   v.push_back(t[i]);
 } // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
   }
-
+  {
+std::vector v;
+// CHECK-FIXES: v.reserve(t.size());
+for (const auto  : t) {
+  v.push_back(e);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+}
+  }
+  {
+std::vector v;
+// CHECK-FIXES: v.reserve(t.size());
+for (const auto  : t) {
+  v.push_back(Op(e));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+}
+  }
+  {
+std::vector v;
+// CHECK-FIXES: v.reserve(t.size());
+for (const auto  : t) {
+  v.push_back(Foo(e));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+}
+  }
+  {
+std::vector v;
+// CHECK-FIXES: v.reserve(t.size());
+for (const auto  : t) {
+  v.push_back(e);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+}
+  }
   //  Non-fixed Cases 
   {
 std::vector v;
@@ -181,4 +245,21 @@
   v.push_back(t[i]);
 }
   }
+  {
+std::vector v;
+// initializer_list should not trigger the check.
+for (int e : {1, 2, 3, 4, 5}) {
+  v.push_back(e);
+}
+  }
+  {
+std::vector v;
+std::vector* v2 = 
+// We only support detecting the range init expression which references
+// container directly.
+// Complex range init expressions like `*v2` is not supported.
+for (const auto  : *v2) {
+  v.push_back(e);
+}
+  }
 }
Index: docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
===
--- docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
+++ docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
@@ -3,11 +3,13 @@
 performance-inefficient-vector-operation
 
 
-Finds possible inefficient `std::vector` operations (e.g. `push_back`) that may
-cause unnecessary memory reallocations.
+Finds possible inefficient ``std::vector`` operations (e.g. ``push_back``) that
+may cause unnecessary memory reallocations.
 
-Currently, the check only detects a typical counter-based for loop with a single
-statement in it, see below:
+Currently, the check only detects following kinds of loops with a single
+statement body:
+
+* Counter-based for loops start with 0:
 
 .. code-block:: c++
 
@@ -18,3 +20,30 @@
 // memory reallocations in v. This can be avoid by inserting a 'reserve(n)'
 // statment before the for statment.
   }
+
+
+* For-range loops like ``for (range-declaration : range_expression)``, the type
+  of ``range_expression`` can be ``std::vector``, ``std::array``,
+  ``std::dequeue``, ``std::set``, ``std::unordered_set``, ``std::map``,
+  ``std::unordered_set``:
+
+.. code-block:: c++
+
+  std::vector data;
+  std::vector v;
+
+  for (auto element : data) {
+v.push_back(element);
+// This will trigger the warning since the 'push_back' may cause multiple
+// memory reallocations in v. This can be avoid by inserting a
+// 

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

[Jacob Bandes-Storch via Phabricator via cfe-commits]

jtbandes created this revision.
Herald added a subscriber: klimek.

This is an attempt to fix the issue described in my recent email: 
http://lists.llvm.org/pipermail/cfe-dev/2017-April/053632.html

After digging in for a while, I learned that:

- the spurious line breaks were occurring inside the condition 
`Current.is(TT_BinaryOperator) && Current.CanBreakBefore && 
State.Stack.back().BreakBeforeParameter`.
- `BreakBeforeParameter` was being set to true because `AvoidBinPacking` was 
true.
- `AvoidBinPacking` was true because it propagated all the way along the stack 
starting from the `(` "scope opener".

Given the above, it seemed to make sense to reset `AvoidBinPacking` to `false` 
when propagating it into a subexpression.

 hello @djasper — you seem to be the most prolific clang-formatter — please 
feel free to tear this apart 


https://reviews.llvm.org/D32475

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2591,6 +2591,30 @@
Style);
 }
 
+TEST_F(FormatTest, AllowBinPackingInsideArguments) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+  verifyFormat(StringRef(R"(
+someFunction(
+arg1,
+arg2,
+arg3 + is + quite + long + so + it
++ f(and_even,
+the,
+arguments << of << its << subexpressions << lengthy << as << they
+  << may << or__ << may,
+not__,
+be)
++ a + r + e / l + o + v + i + n + g + l + y / b + i + n - p + a + c + k
++ e + d,
+arg4,
+arg5);
+   )").trim(), Style);
+}
+
 TEST_F(FormatTest, ConstructorInitializers) {
   verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
   verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -919,6 +919,12 @@
 NewParenState.NoLineBreak =
 NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
+// Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
+if (Current.FakeLParens.size() > 0 &&
+Current.FakeLParens.back() > prec::Comma) {
+  NewParenState.AvoidBinPacking = false;
+}
+
 // Indent from 'LastSpace' unless these are fake parentheses encapsulating
 // a builder type call after 'return' or, if the alignment after opening
 // brackets is disabled.


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2591,6 +2591,30 @@
Style);
 }
 
+TEST_F(FormatTest, AllowBinPackingInsideArguments) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+  verifyFormat(StringRef(R"(
+someFunction(
+arg1,
+arg2,
+arg3 + is + quite + long + so + it
++ f(and_even,
+the,
+arguments << of << its << subexpressions << lengthy << as << they
+  << may << or__ << may,
+not__,
+be)
++ a + r + e / l + o + v + i + n + g + l + y / b + i + n - p + a + c + k
++ e + d,
+arg4,
+arg5);
+   )").trim(), Style);
+}
+
 TEST_F(FormatTest, ConstructorInitializers) {
   verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
   verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -919,6 +919,12 @@
 NewParenState.NoLineBreak =
 NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
+// Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
+if (Current.FakeLParens.size() > 0 &&
+Current.FakeLParens.back() > prec::Comma) {
+  NewParenState.AvoidBinPacking = false;
+}
+
 // Indent from 'LastSpace' unless these are fake parentheses encapsulating
 // a builder type call after 'return' or, if the alignment after opening
 // brackets is disabled.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org