date:20180508

[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode

2018-05-08 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

In https://reviews.llvm.org/D46520#1092233, @rnk wrote:

> Please don't do this, this is actually really problematic, since `#line` 
> directives lose information about what's a system header. That means the 
> result of -E usually won't compile, since Windows headers are typically full 
> of warnings and default-error warnings.

Oh, ok - I can revert it then; I didn't have any specific need for this, I just 
noticed that clang in msvc mode and cl.exe differed in this aspect.

Repository:
  rL LLVM

https://reviews.llvm.org/D46520

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

[PATCH] D46613: _Atomic of empty struct shouldn't assert

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
jfb marked an inline comment as done.
Closed by commit rC331845: _Atomic of empty struct shouldnt assert 
(authored by jfb, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46613?vs=145858=145859#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46613

Files:
  lib/AST/ASTContext.cpp
  test/CodeGen/c11atomics-ios.c
  test/CodeGen/c11atomics.c


Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
 Width = Info.Width;
 Align = Info.Align;
 
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+  // An otherwise zero-sized type should still generate an
+  // atomic operation.
+  Width = Target->getCharWidth();
+  assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+  // If the size of the type doesn't exceed the platform's max
+  // atomic promotion width, make the size and alignment more
+  // favorable to atomic operations:
+
   // Round the size up to a power of 2.
   if (!llvm::isPowerOf2_64(Width))
 Width = llvm::NextPowerOf2(Width);
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) 
{
+  // CHECK-LABEL: @test_empty_struct_store(
+  // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext 
%{{.*}}, i32 5)
+  __c11_atomic_store(empty, value, 5);
+}


Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
 Width = Info.Width;
 Align = Info.Align;
 
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+  // An otherwise zero-sized type should still generate an
+  // atomic operation.
+  Width = Target->getCharWidth();
+  assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+  // If the size of the type doesn't exceed the platform's max
+  // atomic promotion width, make the size and alignment more
+  // favorable to atomic operations:
+
   // Round the size up to a power of 2.
   if (!llvm::isPowerOf2_64(Width))
 Width = llvm::NextPowerOf2(Width);
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
   // CHECK:

r331845 - _Atomic of empty struct shouldn't assert

2018-05-08 Thread JF Bastien via cfe-commits

Author: jfb
Date: Tue May  8 20:51:12 2018
New Revision: 331845

URL: http://llvm.org/viewvc/llvm-project?rev=331845=rev
Log:
_Atomic of empty struct shouldn't assert

Summary:

An _Atomic of an empty struct is pretty silly. In general we just widen empty
structs to hold a byte's worth of storage, and we represent size and alignment
as 0 internally and let LLVM figure out what to do. For _Atomic it's a bit
different: the memory model mandates concrete effects occur when atomic
operations occur, so in most cases actual instructions need to get emitted. It's
really not worth trying to optimize empty struct atomics by figuring out e.g.
that a fence would do, even though sane compilers should do optimize atomics.
Further, wg21.link/p0528 will fix C++20 atomics with padding bits so that
cmpxchg on them works, which means that we'll likely need to do the zero-init
song and dance for empty atomic structs anyways (and I think we shouldn't
special-case this behavior to C++20 because prior standards are just broken).

This patch therefore makes a minor change to r176658 "Promote atomic type sizes
up to a power of two": if the width of the atomic's value type is 0, just use 1
byte for width and leave alignment as-is (since it should never be zero, and
over-aligned zero-width structs are weird but fine).

This fixes an assertion:
   (NumBits >= MIN_INT_BITS && "bitwidth too small"), function get, file 
../lib/IR/Type.cpp, line 241.

It seems like this has run into other assertions before (namely the unreachable
Kind check in ImpCastExprToType), but I haven't reproduced that issue with
tip-of-tree.



Reviewers: arphaman, rjmccall

Subscribers: aheejin, cfe-commits

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

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/test/CodeGen/c11atomics-ios.c
cfe/trunk/test/CodeGen/c11atomics.c

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=331845=331844=331845=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue May  8 20:51:12 2018
@@ -1958,10 +1958,16 @@ TypeInfo ASTContext::getTypeInfoImpl(con
 Width = Info.Width;
 Align = Info.Align;
 
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+  // An otherwise zero-sized type should still generate an
+  // atomic operation.
+  Width = Target->getCharWidth();
+  assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+  // If the size of the type doesn't exceed the platform's max
+  // atomic promotion width, make the size and alignment more
+  // favorable to atomic operations:
+
   // Round the size up to a power of 2.
   if (!llvm::isPowerOf2_64(Width))
 Width = llvm::NextPowerOf2(Width);

Modified: cfe/trunk/test/CodeGen/c11atomics-ios.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/c11atomics-ios.c?rev=331845=331844=331845=diff
==
--- cfe/trunk/test/CodeGen/c11atomics-ios.c (original)
+++ cfe/trunk/test/CodeGen/c11atomics-ios.c Tue May  8 20:51:12 2018
@@ -319,3 +319,19 @@ _Bool test_promoted_cmpxchg(_Atomic(PS)
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}

Modified: cfe/trunk/test/CodeGen/c11atomics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/c11atomics.c?rev=331845=331844=331845=diff
==
--- cfe/trunk/test/CodeGen/c11atomics.c (original)
+++ cfe/trunk/test/CodeGen/c11atomics.c Tue May  8 20:51:12 2018
@@ -474,3 +474,17 @@ _Bool test_promoted_cmpxchg(_Atomic(PS)
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) 
{
+  // CHECK-LABEL:

[PATCH] D46613: _Atomic of empty struct shouldn't assert

2018-05-08 Thread John McCall via Phabricator via cfe-commits

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

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D46613



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

[PATCH] D46613: _Atomic of empty struct shouldn't assert

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: lib/AST/ASTContext.cpp:1965
+  Width = Target->getCharWidth();
+  Align = Target->getCharWidth();
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {

rjmccall wrote:
> Alignment, unlike size, is definitely never 0.  I think you should leave the 
> original alignment in place; it's a weird case, but we honor over-aligned 
> empty types in a bunch of other places.
Yeah that makes total sense. I turned it to an assert.


Repository:
  rC Clang

https://reviews.llvm.org/D46613



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

[PATCH] D46613: _Atomic of empty struct shouldn't assert

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb updated this revision to Diff 145858.
jfb added a comment.

- Assert on zero alignment, instead of making it always byte-aligned.


Repository:
  rC Clang

https://reviews.llvm.org/D46613

Files:
  lib/AST/ASTContext.cpp
  test/CodeGen/c11atomics-ios.c
  test/CodeGen/c11atomics.c


Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) 
{
+  // CHECK-LABEL: @test_empty_struct_store(
+  // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext 
%{{.*}}, i32 5)
+  __c11_atomic_store(empty, value, 5);
+}
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
 Width = Info.Width;
 Align = Info.Align;
 
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+  // An otherwise zero-sized type should still generate an
+  // atomic operation.
+  Width = Target->getCharWidth();
+  assert(Align);
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+  // If the size of the type doesn't exceed the platform's max
+  // atomic promotion width, make the size and alignment more
+  // favorable to atomic operations:
+
   // Round the size up to a power of 2.
   if (!llvm::isPowerOf2_64(Width))
 Width = llvm::NextPowerOf2(Width);


Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @test_empty_struct_store(
+  // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %{{.*}}, i32 5)
+  __c11_atomic_store(empty, value, 5);
+}
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
 Width = Info.Width;
 Align = Info.Align;
 
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+  // An otherwise zero-sized type should still generate an
+

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

2018-05-08 Thread Phabricator via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL331843: [DebugInfo] Generate debug information for labels. 
(authored by shiva, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45045?vs=143243=145849#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45045

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/CodeGen/CGDebugInfo.h
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/test/CodeGen/backend-unsupported-error.ll
  cfe/trunk/test/CodeGen/debug-label-inline.c
  cfe/trunk/test/CodeGen/debug-label.c

Index: cfe/trunk/lib/CodeGen/CGDebugInfo.h
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h
@@ -396,6 +396,9 @@
llvm::Value *AI,
CGBuilderTy );
 
+  /// Emit call to \c llvm.dbg.label for an label.
+  void EmitLabel(const LabelDecl *D, CGBuilderTy );
+
   /// Emit call to \c llvm.dbg.declare for an imported variable
   /// declaration in a block.
   void EmitDeclareOfBlockDeclRefVariable(const VarDecl *variable,
Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -531,6 +531,16 @@
   }
 
   EmitBlock(Dest.getBlock());
+
+  // Emit debug info for labels.
+  if (CGDebugInfo *DI = getDebugInfo()) {
+if (CGM.getCodeGenOpts().getDebugInfo() >=
+codegenoptions::LimitedDebugInfo) {
+  DI->setLocation(D->getLocation());
+  DI->EmitLabel(D, Builder);
+}
+  }
+
   incrementProfileCounter(D->getStmt());
 }
 
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3647,6 +3647,32 @@
   return EmitDeclare(VD, Storage, llvm::None, Builder);
 }
 
+void CGDebugInfo::EmitLabel(const LabelDecl *D, CGBuilderTy ) {
+  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+
+  if (D->hasAttr())
+return;
+
+  auto *Scope = cast(LexicalBlockStack.back());
+  llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
+
+  // Get location information.
+  unsigned Line = getLineNumber(D->getLocation());
+  unsigned Column = getColumnNumber(D->getLocation());
+
+  StringRef Name = D->getName();
+
+  // Create the descriptor for the label.
+  auto *L =
+  DBuilder.createLabel(Scope, Name, Unit, Line, CGM.getLangOpts().Optimize);
+
+  // Insert an llvm.dbg.label into the current block.
+  DBuilder.insertLabel(L,
+   llvm::DebugLoc::get(Line, Column, Scope, CurInlinedAt),
+   Builder.GetInsertBlock());
+}
+
 llvm::DIType *CGDebugInfo::CreateSelfType(const QualType ,
   llvm::DIType *Ty) {
   llvm::DIType *CachedTy = getTypeOrNull(QualTy);
Index: cfe/trunk/test/CodeGen/debug-label.c
===
--- cfe/trunk/test/CodeGen/debug-label.c
+++ cfe/trunk/test/CodeGen/debug-label.c
@@ -0,0 +1,16 @@
+// This test will test the correstness of generating DILabel and
+// llvm.dbg.label for labels.
+//
+// RUN: %clang_cc1 -emit-llvm %s -o - -emit-llvm -debug-info-kind=limited | FileCheck %s
+
+int f1(int a, int b) {
+  int sum;
+
+top:
+  // CHECK: call void @llvm.dbg.label(metadata [[LABEL_METADATA:!.*]]), !dbg [[LABEL_LOCATION:!.*]]
+  sum = a + b;
+  return sum;
+}
+
+// CHECK: [[LABEL_METADATA]] = !DILabel({{.*}}, name: "top", {{.*}}, line: 9)
+// CHECK: [[LABEL_LOCATION]] = !DILocation(line: 9,
Index: cfe/trunk/test/CodeGen/backend-unsupported-error.ll
===
--- cfe/trunk/test/CodeGen/backend-unsupported-error.ll
+++ cfe/trunk/test/CodeGen/backend-unsupported-error.ll
@@ -30,11 +30,11 @@
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.9.0", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2)
 !1 = !DIFile(filename: "test.c", directory: "")
 !2 = !{}
-!4 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, variables: !2)
+!4 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, retainedNodes: !2)
 !5 = !DISubroutineType(types: !6)
 !6 = !{!7}
 !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-!8 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3,

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-05-08 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added a comment.

> Can you post the diagnostic exactly as it appears in the compiler output? I 
> am surprised that it would appear here. It should appear here only if the 
> standard library's implicit conversion from std::map<...>::iterator to 
> std::map<...>::const_iterator were implemented as a conversion operator 
> instead of as a converting constructor. I have verified that both libstdc++ 
> trunk and libc++ trunk implement it "correctly" as a converting constructor, 
> and I have verified on Wandbox/Godbolt that no warning is emitted on your 
> sample code when using either of those standard libraries.
> 
> Is it possible that you are using a standard library that is neither libc++ 
> or libstdc++?
> 
> Is it possible that that standard library implements the 
> iterator-to-const_iterator conversion as a conversion operator instead of a 
> converting constructor?

@thakis ping — did you ever resolve this issue to your satisfaction?


Repository:
  rC Clang

https://reviews.llvm.org/D43322



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

[PATCH] D46613: _Atomic of empty struct shouldn't assert

2018-05-08 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: lib/AST/ASTContext.cpp:1965
+  Width = Target->getCharWidth();
+  Align = Target->getCharWidth();
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {

Alignment, unlike size, is definitely never 0.  I think you should leave the 
original alignment in place; it's a weird case, but we honor over-aligned empty 
types in a bunch of other places.


Repository:
  rC Clang

https://reviews.llvm.org/D46613



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In https://reviews.llvm.org/D42933#1092077, @smeenai wrote:

> Apple's current recommendation for using printf with the NSInteger types is 
> casting to a long, per 
> https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html.
>  Are you suggesting that recommendation would change to using `%zd` instead?


I'm not saying that the recommendation would change, but `long` being 
pointer-sized is a pretty universal assumption on Darwin, I'm afraid.

In https://reviews.llvm.org/D42933#1092154, @jfb wrote:

> I don't disagree with the original intent, but AFAICT that's exactly the 
> intent of the warning I'd like to get rid of. I'm making a very narrow point: 
> there is no portability problem with the warning I'm specifically trying to 
> silence (using `%z` with `NS[U]Integer` on Darwin). If we decide that 
> `-Wformat` shouldn't emit portability warnings then my point is moot, so 
> let's see if people agree to that. If not, then my point still stands.


Agreed.

>>> - There are always established idioms for solving portability problems.  In 
>>> this case, a certain set of important typedefs from the C standard have 
>>> been blessed with dedicated format modifiers like "z", but every other 
>>> typedef in the world has to find some other solution, either by asserting 
>>> that some existing format is good enough (as NSInteger does) or by 
>>> introducing a macro that expands to an appropriate format (as some things 
>>> in POSIX do).  A proper format-portability warning would have to know about 
>>> all of these conventions (in order to check that e.g. part of the format 
>>> string came from the right macro), which just seems wildly out-of-scope for 
>>> a compiler warning.
> 
> We could provide a macro for `NS[U]Integer`, but it's been long enough and 
> there's enough existing code with `%z`. Not sure the code churn on user is 
> worth it, if we can instead say "`%z` works".

Right.  I'm not trying to argue that we should add a macro for NSInteger, I'm 
saying that that's the sort of thing that a portability warning would have to 
consider.  Portability warnings are... always tempting, but they're very, very 
difficult in C.  Since we're not going to do that sort of work, we should back 
off from doing a portability warning.

John.


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D46320: Remove \brief commands from doxygen comments.

2018-05-08 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331834: Remove \brief commands from doxygen comments. 
(authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46320?vs=144736=145832#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46320

Files:
  cfe/trunk/docs/LibFormat.rst
  cfe/trunk/docs/doxygen.cfg.in
  cfe/trunk/include/clang-c/BuildSystem.h
  cfe/trunk/include/clang-c/CXCompilationDatabase.h
  cfe/trunk/include/clang-c/CXErrorCode.h
  cfe/trunk/include/clang-c/CXString.h
  cfe/trunk/include/clang-c/Documentation.h
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/include/clang/ARCMigrate/ARCMT.h
  cfe/trunk/include/clang/ARCMigrate/ARCMTActions.h
  cfe/trunk/include/clang/AST/APValue.h
  cfe/trunk/include/clang/AST/ASTConsumer.h
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/include/clang/AST/ASTDiagnostic.h
  cfe/trunk/include/clang/AST/ASTFwd.h
  cfe/trunk/include/clang/AST/ASTImporter.h
  cfe/trunk/include/clang/AST/ASTLambda.h
  cfe/trunk/include/clang/AST/ASTMutationListener.h
  cfe/trunk/include/clang/AST/ASTTypeTraits.h
  cfe/trunk/include/clang/AST/ASTUnresolvedSet.h
  cfe/trunk/include/clang/AST/Attr.h
  cfe/trunk/include/clang/AST/Availability.h
  cfe/trunk/include/clang/AST/CXXInheritance.h
  cfe/trunk/include/clang/AST/CanonicalType.h
  cfe/trunk/include/clang/AST/CommentBriefParser.h
  cfe/trunk/include/clang/AST/CommentCommandTraits.h
  cfe/trunk/include/clang/AST/CommentLexer.h
  cfe/trunk/include/clang/AST/CommentSema.h
  cfe/trunk/include/clang/AST/ComparisonCategories.h
  cfe/trunk/include/clang/AST/DataCollection.h
  cfe/trunk/include/clang/AST/Decl.h
  cfe/trunk/include/clang/AST/DeclBase.h
  cfe/trunk/include/clang/AST/DeclCXX.h
  cfe/trunk/include/clang/AST/DeclContextInternals.h
  cfe/trunk/include/clang/AST/DeclObjC.h
  cfe/trunk/include/clang/AST/DeclOpenMP.h
  cfe/trunk/include/clang/AST/DeclTemplate.h
  cfe/trunk/include/clang/AST/DeclVisitor.h
  cfe/trunk/include/clang/AST/DeclarationName.h
  cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/include/clang/AST/ExprObjC.h
  cfe/trunk/include/clang/AST/ExprOpenMP.h
  cfe/trunk/include/clang/AST/ExternalASTSource.h
  cfe/trunk/include/clang/AST/LambdaCapture.h
  cfe/trunk/include/clang/AST/LocInfoType.h
  cfe/trunk/include/clang/AST/Mangle.h
  cfe/trunk/include/clang/AST/MangleNumberingContext.h
  cfe/trunk/include/clang/AST/NSAPI.h
  cfe/trunk/include/clang/AST/NestedNameSpecifier.h
  cfe/trunk/include/clang/AST/OpenMPClause.h
  cfe/trunk/include/clang/AST/OperationKinds.def
  cfe/trunk/include/clang/AST/OperationKinds.h
  cfe/trunk/include/clang/AST/ParentMap.h
  cfe/trunk/include/clang/AST/PrettyPrinter.h
  cfe/trunk/include/clang/AST/QualTypeNames.h
  cfe/trunk/include/clang/AST/RawCommentList.h
  cfe/trunk/include/clang/AST/RecordLayout.h
  cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
  cfe/trunk/include/clang/AST/Redeclarable.h
  cfe/trunk/include/clang/AST/SelectorLocationsKind.h
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/include/clang/AST/StmtCXX.h
  cfe/trunk/include/clang/AST/StmtObjC.h
  cfe/trunk/include/clang/AST/StmtOpenMP.h
  cfe/trunk/include/clang/AST/StmtVisitor.h
  cfe/trunk/include/clang/AST/TemplateBase.h
  cfe/trunk/include/clang/AST/TemplateName.h
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/include/clang/AST/TypeLoc.h
  cfe/trunk/include/clang/AST/TypeOrdering.h
  cfe/trunk/include/clang/AST/TypeVisitor.h
  cfe/trunk/include/clang/AST/UnresolvedSet.h
  cfe/trunk/include/clang/AST/VTTBuilder.h
  cfe/trunk/include/clang/AST/VTableBuilder.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchFinder.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchersMacros.h
  cfe/trunk/include/clang/ASTMatchers/Dynamic/Diagnostics.h
  cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h
  cfe/trunk/include/clang/ASTMatchers/Dynamic/Registry.h
  cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
  cfe/trunk/include/clang/Analysis/Analyses/Consumed.h
  cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
  cfe/trunk/include/clang/Analysis/Analyses/FormatString.h
  cfe/trunk/include/clang/Analysis/Analyses/PostOrderCFGView.h
  cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
  cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyLogical.h
  cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
  cfe/trunk/include/clang/Analysis/CFG.h
  cfe/trunk/include/clang/Analysis/CallGraph.h
  cfe/trunk/include/clang/Analysis/CodeInjector.h
  cfe/trunk/include/clang/Analysis/ProgramPoint.h
  cfe/trunk/include/clang/Basic/ABI.h
  cfe/trunk/include/clang/Basic/AddressSpaces.h

[PATCH] D46320: Remove \brief commands from doxygen comments.

2018-05-08 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331834: Remove \brief commands from doxygen comments. 
(authored by adrian, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46320?vs=144736=145831#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46320

Files:
  docs/LibFormat.rst
  docs/doxygen.cfg.in
  include/clang-c/BuildSystem.h
  include/clang-c/CXCompilationDatabase.h
  include/clang-c/CXErrorCode.h
  include/clang-c/CXString.h
  include/clang-c/Documentation.h
  include/clang-c/Index.h
  include/clang/ARCMigrate/ARCMT.h
  include/clang/ARCMigrate/ARCMTActions.h
  include/clang/AST/APValue.h
  include/clang/AST/ASTConsumer.h
  include/clang/AST/ASTContext.h
  include/clang/AST/ASTDiagnostic.h
  include/clang/AST/ASTFwd.h
  include/clang/AST/ASTImporter.h
  include/clang/AST/ASTLambda.h
  include/clang/AST/ASTMutationListener.h
  include/clang/AST/ASTTypeTraits.h
  include/clang/AST/ASTUnresolvedSet.h
  include/clang/AST/Attr.h
  include/clang/AST/Availability.h
  include/clang/AST/CXXInheritance.h
  include/clang/AST/CanonicalType.h
  include/clang/AST/CommentBriefParser.h
  include/clang/AST/CommentCommandTraits.h
  include/clang/AST/CommentLexer.h
  include/clang/AST/CommentSema.h
  include/clang/AST/ComparisonCategories.h
  include/clang/AST/DataCollection.h
  include/clang/AST/Decl.h
  include/clang/AST/DeclBase.h
  include/clang/AST/DeclCXX.h
  include/clang/AST/DeclContextInternals.h
  include/clang/AST/DeclObjC.h
  include/clang/AST/DeclOpenMP.h
  include/clang/AST/DeclTemplate.h
  include/clang/AST/DeclVisitor.h
  include/clang/AST/DeclarationName.h
  include/clang/AST/EvaluatedExprVisitor.h
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/ExprObjC.h
  include/clang/AST/ExprOpenMP.h
  include/clang/AST/ExternalASTSource.h
  include/clang/AST/LambdaCapture.h
  include/clang/AST/LocInfoType.h
  include/clang/AST/Mangle.h
  include/clang/AST/MangleNumberingContext.h
  include/clang/AST/NSAPI.h
  include/clang/AST/NestedNameSpecifier.h
  include/clang/AST/OpenMPClause.h
  include/clang/AST/OperationKinds.def
  include/clang/AST/OperationKinds.h
  include/clang/AST/ParentMap.h
  include/clang/AST/PrettyPrinter.h
  include/clang/AST/QualTypeNames.h
  include/clang/AST/RawCommentList.h
  include/clang/AST/RecordLayout.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Redeclarable.h
  include/clang/AST/SelectorLocationsKind.h
  include/clang/AST/Stmt.h
  include/clang/AST/StmtCXX.h
  include/clang/AST/StmtObjC.h
  include/clang/AST/StmtOpenMP.h
  include/clang/AST/StmtVisitor.h
  include/clang/AST/TemplateBase.h
  include/clang/AST/TemplateName.h
  include/clang/AST/Type.h
  include/clang/AST/TypeLoc.h
  include/clang/AST/TypeOrdering.h
  include/clang/AST/TypeVisitor.h
  include/clang/AST/UnresolvedSet.h
  include/clang/AST/VTTBuilder.h
  include/clang/AST/VTableBuilder.h
  include/clang/ASTMatchers/ASTMatchFinder.h
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  include/clang/ASTMatchers/ASTMatchersMacros.h
  include/clang/ASTMatchers/Dynamic/Diagnostics.h
  include/clang/ASTMatchers/Dynamic/Parser.h
  include/clang/ASTMatchers/Dynamic/Registry.h
  include/clang/ASTMatchers/Dynamic/VariantValue.h
  include/clang/Analysis/Analyses/Consumed.h
  include/clang/Analysis/Analyses/Dominators.h
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Analysis/Analyses/PostOrderCFGView.h
  include/clang/Analysis/Analyses/ThreadSafety.h
  include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  include/clang/Analysis/Analyses/ThreadSafetyLogical.h
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/Analysis/CallGraph.h
  include/clang/Analysis/CodeInjector.h
  include/clang/Analysis/ProgramPoint.h
  include/clang/Basic/ABI.h
  include/clang/Basic/AddressSpaces.h
  include/clang/Basic/AlignedAllocation.h
  include/clang/Basic/AllDiagnostics.h
  include/clang/Basic/AttrKinds.h
  include/clang/Basic/AttrSubjectMatchRules.h
  include/clang/Basic/Attributes.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/BuiltinsWebAssembly.def
  include/clang/Basic/CapturedStmt.h
  include/clang/Basic/CommentOptions.h
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/DiagnosticError.h
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticOptions.h
  include/clang/Basic/ExceptionSpecificationType.h
  include/clang/Basic/ExpressionTraits.h
  include/clang/Basic/FileManager.h
  include/clang/Basic/FileSystemOptions.h
  include/clang/Basic/FileSystemStatCache.h
  include/clang/Basic/IdentifierTable.h
  include/clang/Basic/LLVM.h
  include/clang/Basic/Lambda.h
  include/clang/Basic/LangOptions.def
  include/clang/Basic/LangOptions.h
  include/clang/Basic/Linkage.h
  include/clang/Basic/MacroBuilder.h
  include/clang/Basic/Module.h
  include/clang/Basic/ObjCRuntime.h

r331833 - Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes

2018-05-08 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Tue May  8 17:58:12 2018
New Revision: 331833

URL: http://llvm.org/viewvc/llvm-project?rev=331833=rev
Log:
Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes

This doesn't make any difference since we don't use RPATH/RUNPATH
on Fuchsia but it avoids the CMake error when re-linking libraries
while building with Ninja.

Differntial Revision: https://reviews.llvm.org/D46610

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=331833=331832=331833=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Tue May  8 17:58:12 2018
@@ -67,6 +67,7 @@ endif()
 foreach(target x86_64;aarch64)
   set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_TYPE 
${FUCHSIA_RUNTIMES_BUILD_TYPE} CACHE STRING "")
+  set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE 
STRING "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_C_FLAGS} 
CACHE PATH "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_C_FLAGS ${FUCHSIA_${target}_C_FLAGS} 
CACHE PATH "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_CXX_FLAGS 
${FUCHSIA_${target}_CXX_FLAGS} CACHE PATH "")


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

[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes

2018-05-08 Thread Jake Ehrlich via Phabricator via cfe-commits

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

Spoke offline, LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46610



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

[PATCH] D45470: Emit an error when include after

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb added a comment.

In https://reviews.llvm.org/D45470#1092212, @vsapsai wrote:

> Here is another approach that should emit an error only when mixing headers
>  causes compilation problems.
>
> Have no ideas how to test the change. `-verify` doesn't work with fatal errors
>  and libcxx doesn't use FileCheck. Performed only manual testing.

This worked with `` before `` as well as with the order 
reversed?

LGTM pending testing answer. This seems simpler than implementing  
http://wg21.link/p0943 outright, though that would still be the best solution 
IMO.

Comment at: libcxx/include/atomic:558
 #endif
+#ifdef kill_dependency
+#error C++ standard library is incompatible with 

I checked and C guarantees that this is a macro :)

https://reviews.llvm.org/D45470

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

[PATCH] D46615: [tools] Updating PPCallbacks::InclusionDirective calls

2018-05-08 Thread Julie Hockett via Phabricator via cfe-commits

juliehockett created this revision.
juliehockett added a reviewer: aaron.ballman.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: jkorous, kbarton, ioeric, nemanjai.

[[ https://reviews.llvm.org/D46614 | [https://reviews.llvm.org/D46614]  ]] adds 
SrcMgr::CharacteristicKind to the InclusionDirective callback, this patch 
updates instances of it in clang-tools-extra.


https://reviews.llvm.org/D46615

Files:
  clang-move/ClangMove.cpp
  clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  clang-tidy/utils/IncludeInserter.cpp
  clangd/ClangdUnit.cpp
  clangd/Headers.cpp
  modularize/CoverageChecker.cpp
  modularize/PreprocessorTracker.cpp
  pp-trace/PPCallbacksTracker.cpp
  pp-trace/PPCallbacksTracker.h

Index: pp-trace/PPCallbacksTracker.h
===
--- pp-trace/PPCallbacksTracker.h
+++ pp-trace/PPCallbacksTracker.h
@@ -102,7 +102,8 @@
   const clang::FileEntry *File,
   llvm::StringRef SearchPath,
   llvm::StringRef RelativePath,
-  const clang::Module *Imported) override;
+  const clang::Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override;
   void moduleImport(clang::SourceLocation ImportLoc, clang::ModuleIdPath Path,
 const clang::Module *Imported) override;
   void EndOfMainFile() override;
Index: pp-trace/PPCallbacksTracker.cpp
===
--- pp-trace/PPCallbacksTracker.cpp
+++ pp-trace/PPCallbacksTracker.cpp
@@ -139,7 +139,7 @@
 llvm::StringRef FileName, bool IsAngled,
 clang::CharSourceRange FilenameRange, const clang::FileEntry *File,
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
-const clang::Module *Imported) {
+const clang::Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
@@ -149,6 +149,7 @@
   appendFilePathArgument("SearchPath", SearchPath);
   appendFilePathArgument("RelativePath", RelativePath);
   appendArgument("Imported", Imported);
+  appendArgument("FileType", FileType);
 }
 
 // Callback invoked whenever there was an explicit module-import
Index: modularize/PreprocessorTracker.cpp
===
--- modularize/PreprocessorTracker.cpp
+++ modularize/PreprocessorTracker.cpp
@@ -750,7 +750,8 @@
   const clang::FileEntry *File,
   llvm::StringRef SearchPath,
   llvm::StringRef RelativePath,
-  const clang::Module *Imported) override;
+  const clang::Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override;
   void FileChanged(clang::SourceLocation Loc,
clang::PPCallbacks::FileChangeReason Reason,
clang::SrcMgr::CharacteristicKind FileType,
@@ -1289,7 +1290,7 @@
 llvm::StringRef FileName, bool IsAngled,
 clang::CharSourceRange FilenameRange, const clang::FileEntry *File,
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
-const clang::Module *Imported) {
+const clang::Module *Imported, SrcMgr::CharacteristicKind FileType) {
   int DirectiveLine, DirectiveColumn;
   std::string HeaderPath = getSourceLocationFile(PP, HashLoc);
   getSourceLocationLineAndColumn(PP, HashLoc, DirectiveLine, DirectiveColumn);
Index: modularize/CoverageChecker.cpp
===
--- modularize/CoverageChecker.cpp
+++ modularize/CoverageChecker.cpp
@@ -90,7 +90,8 @@
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
   StringRef SearchPath, StringRef RelativePath,
-  const Module *Imported) override {
+  const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override {
 Checker.collectUmbrellaHeaderHeader(File->getName());
   }
 
Index: clangd/Headers.cpp
===
--- clangd/Headers.cpp
+++ clangd/Headers.cpp
@@ -34,7 +34,8 @@
   CharSourceRange /*FilenameRange*/,
   const FileEntry *File, llvm::StringRef /*SearchPath*/,
   llvm::StringRef /*RelativePath*/,
-  const Module * /*Imported*/) override {
+  const Module * /*Imported*/,
+  SrcMgr::CharacteristicKind FileType) override {
 WrittenHeaders.insert(
 (IsAngled ? "<" + FileName + ">" : "\"" + FileName +

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

2018-05-08 Thread Julie Hockett via Phabricator via cfe-commits

juliehockett created this revision.
juliehockett added a reviewer: aaron.ballman.
juliehockett added a project: clang.
Herald added subscribers: kbarton, nemanjai.

Adding a SrcMgr::CharacteristicKind parameter to the InclusionDirective in 
PPCallbacks, and updating calls to that function. This will be useful in 
https://reviews.llvm.org/D43778 to determine which includes are system headers.


https://reviews.llvm.org/D46614

Files:
  include/clang/Lex/PPCallbacks.h
  include/clang/Lex/PreprocessingRecord.h
  lib/CodeGen/MacroPPCallbacks.cpp
  lib/CodeGen/MacroPPCallbacks.h
  lib/Frontend/DependencyFile.cpp
  lib/Frontend/DependencyGraph.cpp
  lib/Frontend/ModuleDependencyCollector.cpp
  lib/Frontend/PrintPreprocessedOutput.cpp
  lib/Frontend/Rewrite/InclusionRewriter.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/PreprocessingRecord.cpp
  tools/libclang/Indexing.cpp
  unittests/Lex/PPCallbacksTest.cpp

Index: unittests/Lex/PPCallbacksTest.cpp
===
--- unittests/Lex/PPCallbacksTest.cpp
+++ unittests/Lex/PPCallbacksTest.cpp
@@ -39,16 +39,18 @@
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
   StringRef SearchPath, StringRef RelativePath,
-  const Module *Imported) override {
-  this->HashLoc = HashLoc;
-  this->IncludeTok = IncludeTok;
-  this->FileName = FileName.str();
-  this->IsAngled = IsAngled;
-  this->FilenameRange = FilenameRange;
-  this->File = File;
-  this->SearchPath = SearchPath.str();
-  this->RelativePath = RelativePath.str();
-  this->Imported = Imported;
+  const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override {
+this->HashLoc = HashLoc;
+this->IncludeTok = IncludeTok;
+this->FileName = FileName.str();
+this->IsAngled = IsAngled;
+this->FilenameRange = FilenameRange;
+this->File = File;
+this->SearchPath = SearchPath.str();
+this->RelativePath = RelativePath.str();
+this->Imported = Imported;
+this->FileType = FileType;
   }
 
   SourceLocation HashLoc;
@@ -60,6 +62,7 @@
   SmallString<16> SearchPath;
   SmallString<16> RelativePath;
   const Module* Imported;
+  SrcMgr::CharacteristicKind FileType;
 };
 
 // Stub to collect data from PragmaOpenCLExtension callbacks.
Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -249,7 +249,8 @@
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
   StringRef SearchPath, StringRef RelativePath,
-  const Module *Imported) override {
+  const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override {
 bool isImport = (IncludeTok.is(tok::identifier) &&
 IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import);
 DataConsumer.ppIncludedFile(HashLoc, FileName, File, isImport, IsAngled,
Index: lib/Lex/PreprocessingRecord.cpp
===
--- lib/Lex/PreprocessingRecord.cpp
+++ lib/Lex/PreprocessingRecord.cpp
@@ -471,7 +471,8 @@
 const FileEntry *File,
 StringRef SearchPath,
 StringRef RelativePath,
-const Module *Imported) {
+const Module *Imported, 
+SrcMgr::CharacteristicKind FileType) {
   InclusionDirective::InclusionKind Kind = InclusionDirective::Include;
   
   switch (IncludeTok.getIdentifierInfo()->getPPKeywordID()) {
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1968,7 +1968,7 @@
 HashLoc, IncludeTok,
 LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename, isAngled,
 FilenameRange, File, SearchPath, RelativePath,
-ShouldEnter ? nullptr : SuggestedModule.getModule());
+ShouldEnter ? nullptr : SuggestedModule.getModule(), FileCharacter);
 if (SkipHeader && !SuggestedModule.getModule())
   Callbacks->FileSkipped(*File, FilenameTok, FileCharacter);
   }
Index: lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -77,7 +77,8 @@
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange, const FileEntry *File,
   StringRef SearchPath, StringRef RelativePath,
-  const Module *Imported) override;
+  const Module *Imported,
+

[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode

2018-05-08 Thread Reid Kleckner via Phabricator via cfe-commits

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

Please don't do this, this is actually really problematic, since `#line` 
directives lose information about what's a system header. That means the result 
of -E usually won't compile, since Windows headers are typically full of 
warnings and default-error warnings.


Repository:
  rL LLVM

https://reviews.llvm.org/D46520



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

[PATCH] D46613: _Atomic of empty struct shouldn't assert

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb created this revision.
jfb added reviewers: arphaman, rjmccall.
Herald added subscribers: cfe-commits, aheejin.

An _Atomic of an empty struct is pretty silly. In general we just widen empty 
structs to hold a byte's worth of storage, and we represent size and alignment 
as 0 internally and let LLVM figure out what to do. For _Atomic it's a bit 
different: the memory model mandates concrete effects occur when atomic 
operations occur, so in most cases actual instructions need to get emitted. 
It's really not worth trying to optimize empty struct atomics by figuring out 
e.g. that a fence would do, even though sane compilers should do optimize 
atomics. Further, wg21.link/p0528 will fix C++20 atomics with padding bits so 
that cmpxchg on them works, which means that we'll likely need to do the 
zero-init song and dance for empty atomic structs anyways (and I think we 
shouldn't special-case this behavior to C++20 because prior standards are just 
broken).

This patch therefore makes a minor change to r176658 "Promote atomic type sizes 
up to a power of two": if the width of the atomic's value type is 0, just use 1 
byte for width and alignment.

This fixes an assertion:

  (NumBits >= MIN_INT_BITS && "bitwidth too small"), function get, file 
../lib/IR/Type.cpp, line 241.

It seems like this has run into other assertions before (namely the unreachable 
Kind check in ImpCastExprToType), but I haven't reproduced that issue with 
tip-of-tree.

rdar://problem/39678063


Repository:
  rC Clang

https://reviews.llvm.org/D46613

Files:
  lib/AST/ASTContext.cpp
  test/CodeGen/c11atomics-ios.c
  test/CodeGen/c11atomics.c


Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) 
{
+  // CHECK-LABEL: @test_empty_struct_store(
+  // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext 
%{{.*}}, i32 5)
+  __c11_atomic_store(empty, value, 5);
+}
Index: test/CodeGen/c11atomics-ios.c
===
--- test/CodeGen/c11atomics-ios.c
+++ test/CodeGen/c11atomics-ios.c
@@ -319,3 +319,19 @@
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1958,10 +1958,16 @@
 Width = Info.Width;
 Align = Info.Align;
 
-// If the size of the type doesn't exceed the platform's max
-// atomic promotion width, make the size and alignment more
-// favorable to atomic operations:
-if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+if (!Width) {
+  // An otherwise zero-sized type should still generate an
+  // atomic operation.
+  Width = Target->getCharWidth();
+  Align = Target->getCharWidth();
+} else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+  // If the size of the type doesn't exceed the platform's max
+  // atomic promotion width, make the size and alignment more
+  // favorable to atomic operations:
+
   // Round the size up to a power of 2.
   if (!llvm::isPowerOf2_64(Width))
 Width = llvm::NextPowerOf2(Width);


Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,17 @@
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @test_empty_struct_store(
+  // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext

[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.

2018-05-08 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: include/clang/Basic/Attr.td:1494
+def NoStackProtector : InheritableAttr {
+  let Spellings = [GCC<"no_stack_protector">];
+  let Subjects = SubjectList<[Function]>;

aaron.ballman wrote:
> manojgupta wrote:
> > aaron.ballman wrote:
> > > This is not a GCC attribute, so this should use the Clang spelling.
> > > 
> > > However, why spell the attribute this way rather than use the GCC 
> > > spelling (`optimize("no-stack-protector")`?
> > Thanks, I have changed it to use Clang spelling.
> > 
> > Regarding __attribute__((optimize("..."))), it is a generic facility in GCC 
> > that works for many optimizer flags.
> > Clang currently does not support this syntax currently instead preferring 
> > its own version for some options e.g. -O0. 
> > e.g.  
> > ```
> > __attribute__((optimize("O0")))  // clang version is 
> > __attribute__((optnone)) 
> > ```
> > If we want to support the GCC syntax, future expectation would be support 
> > more flags under this syntax. Is that the path we want to take (I do not 
> > know the history related to previous syntax decisions but better GCC 
> > compatibility will be a nice thing to have) 
> The history of `optnone` predates my involvement with Clang and I've not been 
> able to find the original review thread (I did find the one where I gave my 
> LGTM on the original patch, but that was a resubmission after the original 
> design was signed off).
> 
> I'm not keen on attributes that have the same semantics but differ in 
> spelling from attributes supported by GCC unless there's a good reason to 
> deviate. Given that we've already deviated, I'd like to understand why better 
> -- I don't want to push you to implement something we've already decided was 
> a poor design, but I also don't want to accept code if we can instead use 
> syntax that is compatible with GCC.
I do not think we want to pursue supporting generic optimization flags with 
`__attribute__((optimize("...")))`.

Part of the reason we only support `optnone` is that LLVM's pass pipeline is 
global to the entire module. It's not trivial to enable optimizations for a 
single function, but it is much easier to have optimization passes skip 
functions marked `optnone`.


Repository:
  rC Clang

https://reviews.llvm.org/D46300



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

[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes

2018-05-08 Thread Jake Ehrlich via Phabricator via cfe-commits

jakehehrlich added a comment.

Need some clarification on this option

1. The rpath is the path from which libs mentioned in .dynamic are relative to?
2. Normally cmake first links binaries without setting the RPATH and then sets 
it later.
3. This makes RPATH always equal to the empty string and avoids the "relinking" 
step that resets RPATH

If so this LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46610



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

r331826 - [CMake] Include llvm-strip in Fuchsia toolchain distribution

2018-05-08 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Tue May  8 17:05:28 2018
New Revision: 331826

URL: http://llvm.org/viewvc/llvm-project?rev=331826=rev
Log:
[CMake] Include llvm-strip in Fuchsia toolchain distribution

Now that llvm-strip is available, include it in the Fuchsia toolchain.

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

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=331826=331825=331826=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Tue May  8 17:05:28 2018
@@ -108,6 +108,7 @@ set(LLVM_TOOLCHAIN_TOOLS
   llvm-readelf
   llvm-readobj
   llvm-size
+  llvm-strip
   llvm-symbolizer
   opt
   sancov


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

[PATCH] D46612: [CMake] Include llvm-strip in Fuchsia toolchain distribution

2018-05-08 Thread Petr Hosek via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331826: [CMake] Include llvm-strip in Fuchsia toolchain 
distribution (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46612?vs=145815=145818#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46612

Files:
  cmake/caches/Fuchsia-stage2.cmake


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -108,6 +108,7 @@
   llvm-readelf
   llvm-readobj
   llvm-size
+  llvm-strip
   llvm-symbolizer
   opt
   sancov


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -108,6 +108,7 @@
   llvm-readelf
   llvm-readobj
   llvm-size
+  llvm-strip
   llvm-symbolizer
   opt
   sancov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45470: Emit an error when mixing and

2018-05-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai updated this revision to Diff 145817.
vsapsai added a comment.

Here is another approach that should emit an error only when mixing headers
causes compilation problems.

Have no ideas how to test the change. `-verify` doesn't work with fatal errors
and libcxx doesn't use FileCheck. Performed only manual testing.


https://reviews.llvm.org/D45470

Files:
  libcxx/include/atomic


Index: libcxx/include/atomic
===
--- libcxx/include/atomic
+++ libcxx/include/atomic
@@ -555,6 +555,9 @@
 #if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP)
 #error  is not implemented
 #endif
+#ifdef kill_dependency
+#error C++ standard library is incompatible with 
+#endif
 
 #if _LIBCPP_STD_VER > 14
 # define __cpp_lib_atomic_is_always_lock_free 201603L


Index: libcxx/include/atomic
===
--- libcxx/include/atomic
+++ libcxx/include/atomic
@@ -555,6 +555,9 @@
 #if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP)
 #error  is not implemented
 #endif
+#ifdef kill_dependency
+#error C++ standard library is incompatible with 
+#endif
 
 #if _LIBCPP_STD_VER > 14
 # define __cpp_lib_atomic_is_always_lock_free 201603L
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45470: Emit an error when mixing and

2018-05-08 Thread Volodymyr Sapsai via Phabricator via cfe-commits

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

`__ALLOW_STDC_ATOMICS_IN_CXX__` approach didn't work in practice, I've reverted 
all changes.


Repository:
  rC Clang

https://reviews.llvm.org/D45470



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

[PATCH] D46612: [CMake] Include llvm-strip in Fuchsia toolchain distribution

2018-05-08 Thread Petr Hosek via Phabricator via cfe-commits

phosek created this revision.
phosek added a reviewer: jakehehrlich.
Herald added subscribers: cfe-commits, mgorny.

Now that llvm-strip is available, include it in the Fuchsia toolchain.


Repository:
  rC Clang

https://reviews.llvm.org/D46612

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -108,6 +108,7 @@
   llvm-readelf
   llvm-readobj
   llvm-size
+  llvm-strip
   llvm-symbolizer
   opt
   sancov


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -108,6 +108,7 @@
   llvm-readelf
   llvm-readobj
   llvm-size
+  llvm-strip
   llvm-symbolizer
   opt
   sancov
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes

2018-05-08 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

Actually after testing this I believe this is really what we want, without this 
option CMake sets RPATH for build libraries and then removes it when installing 
these. With this option it doesn't even set the RPATH for build libraries.


Repository:
  rC Clang

https://reviews.llvm.org/D46610



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

[PATCH] D46610: [CMake] Set CMAKE_BUILD_WITH_INSTALL_RPATH for Fuchsia runtimes

2018-05-08 Thread Petr Hosek via Phabricator via cfe-commits

phosek created this revision.
phosek added a reviewer: mcgrathr.
Herald added subscribers: cfe-commits, mgorny.

This doesn't make any difference since we don't use RPATH/RUNPATH
on Fuchsia but it avoids the CMake error when re-linking libraries
while building with Ninja.


Repository:
  rC Clang

https://reviews.llvm.org/D46610

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -67,6 +67,7 @@
 foreach(target x86_64;aarch64)
   set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_TYPE 
${FUCHSIA_RUNTIMES_BUILD_TYPE} CACHE STRING "")
+  set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE 
STRING "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_C_FLAGS} 
CACHE PATH "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_C_FLAGS ${FUCHSIA_${target}_C_FLAGS} 
CACHE PATH "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_CXX_FLAGS 
${FUCHSIA_${target}_CXX_FLAGS} CACHE PATH "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -67,6 +67,7 @@
 foreach(target x86_64;aarch64)
   set(RUNTIMES_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_TYPE ${FUCHSIA_RUNTIMES_BUILD_TYPE} CACHE STRING "")
+  set(RUNTIMES_${target}-fuchsia_CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE STRING "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_C_FLAGS} CACHE PATH "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_C_FLAGS ${FUCHSIA_${target}_C_FLAGS} CACHE PATH "")
   set(RUNTIMES_${target}-fuchsia_CMAKE_CXX_FLAGS ${FUCHSIA_${target}_CXX_FLAGS} CACHE PATH "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb added a comment.

In https://reviews.llvm.org/D42933#1092077, @smeenai wrote:

> In https://reviews.llvm.org/D42933#1092048, @rjmccall wrote:
>
> > I agree that the format-specifier checker is not intended to be a 
> > portability checker.
>

I don't disagree with the original intent, but AFAICT that's exactly the intent 
of the warning I'd like to get rid of. I'm making a very narrow point: there is 
no portability problem with the warning I'm specifically trying to silence 
(using `%z` with `NS[U]Integer` on Darwin). If we decide that `-Wformat` 
shouldn't emit portability warnings then my point is moot, so let's see if 
people agree to that. If not, then my point still stands.

>> Any attempt to check portability problems has to account for two things:
>> 
>> - Not all code is intended to be portable.  If you're going to warn about 
>> portability problems, you need some way to not annoy programmers who might 
>> have good reason to say that they only care about their code running on 
>> Linux / macOS / 64-bit / 32-bit / whatever.  Generally this means splitting 
>> the portability warning out as a separate warning group.

Right, it seems like recently `-Wformat` went against this, and users aren't 
thrilled.

>> - There are always established idioms for solving portability problems.  In 
>> this case, a certain set of important typedefs from the C standard have been 
>> blessed with dedicated format modifiers like "z", but every other typedef in 
>> the world has to find some other solution, either by asserting that some 
>> existing format is good enough (as NSInteger does) or by introducing a macro 
>> that expands to an appropriate format (as some things in POSIX do).  A 
>> proper format-portability warning would have to know about all of these 
>> conventions (in order to check that e.g. part of the format string came from 
>> the right macro), which just seems wildly out-of-scope for a compiler 
>> warning.

We could provide a macro for `NS[U]Integer`, but it's been long enough and 
there's enough existing code with `%z`. Not sure the code churn on user is 
worth it, if we can instead say "`%z` works".

> Apple's current recommendation for using printf with the NSInteger types is 
> casting to a long, per 
> https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html.
>  Are you suggesting that recommendation would change to using `%zd` instead?

Indeed, I believe that came from 
https://github.com/llvm-mirror/clang/commit/ec08735e1b6a51c11533311b774bf6336c0a5d63
 and I intend to update documentation once the compiler is updated. It's not 
*wrong*, it's kinda more portable in that it's what you probably want to do if 
it's not `NS[U]Integer`, but it's more typing and users clearly haven't been 
following the recommendation (because let's be honest, `%z` is fine).

Repository:
  rC Clang

https://reviews.llvm.org/D42933

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

[PATCH] D46603: [Support] Print TimeRecord as CSV

2018-05-08 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In https://reviews.llvm.org/D46603#1092135, @george.karpenkov wrote:

> @lebedev.ri LLVM already has facilities for outputting statistics in JSON, it 
> seems that would be sufficient for your purposes?
>  I did something similar for the static analyzer in 
> https://reviews.llvm.org/D43131

YAML != JSON.
I did see all that `TimerGroup` stuff.
I will take a second look, but the `dumpVal()` change will still be needed 
either way.

Repository:
  rL LLVM

https://reviews.llvm.org/D46603

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

[clang-tools-extra] r331822 - Partially revert r331456: [clang-tidy] Remove AnalyzeTemporaryDtors option.

2018-05-08 Thread Roman Lebedev via cfe-commits

Author: lebedevri
Date: Tue May  8 16:15:58 2018
New Revision: 331822

URL: http://llvm.org/viewvc/llvm-project?rev=331822=rev
Log:
Partially revert r331456: [clang-tidy] Remove AnalyzeTemporaryDtors option.

That broke every single .clang-tidy config out there
which happened to specify AnalyzeTemporaryDtors option:

YAML:5:24: error: unknown key 'AnalyzeTemporaryDtors'
AnalyzeTemporaryDtors: false
   ^
Error parsing <...>/.clang-tidy: Invalid argument

More so, that error isn't actually a error, the
clang-tidy does not exit with $? != 0, it continues
with the default config.

Surely this breakage isn't the intended behavior.
But if it is, feel free to revert this commit.

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp?rev=331822=331821=331822=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp Tue May  8 16:15:58 
2018
@@ -83,9 +83,11 @@ template <> struct MappingTraits NOpts(
 IO, Options.CheckOptions);
+bool Ignored = false;
 IO.mapOptional("Checks", Options.Checks);
 IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
 IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
+IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // legacy compatibility
 IO.mapOptional("FormatStyle", Options.FormatStyle);
 IO.mapOptional("User", Options.User);
 IO.mapOptional("CheckOptions", NOpts->Options);

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp?rev=331822=331821=331822=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp Tue 
May  8 16:15:58 2018
@@ -58,6 +58,7 @@ TEST(ParseConfiguration, ValidConfigurat
   llvm::ErrorOr Options =
   parseConfiguration("Checks: \"-*,misc-*\"\n"
  "HeaderFilterRegex: \".*\"\n"
+ "AnalyzeTemporaryDtors: true\n"
  "User: some.user");
   EXPECT_TRUE(!!Options);
   EXPECT_EQ("-*,misc-*", *Options->Checks);
@@ -69,6 +70,7 @@ TEST(ParseConfiguration, MergeConfigurat
   llvm::ErrorOr Options1 = parseConfiguration(R"(
   Checks: "check1,check2"
   HeaderFilterRegex: "filter1"
+  AnalyzeTemporaryDtors: true
   User: user1
   ExtraArgs: ['arg1', 'arg2']
   ExtraArgsBefore: ['arg-before1', 'arg-before2']
@@ -77,6 +79,7 @@ TEST(ParseConfiguration, MergeConfigurat
   llvm::ErrorOr Options2 = parseConfiguration(R"(
   Checks: "check3,check4"
   HeaderFilterRegex: "filter2"
+  AnalyzeTemporaryDtors: false
   User: user2
   ExtraArgs: ['arg3', 'arg4']
   ExtraArgsBefore: ['arg-before3', 'arg-before4']


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

[PATCH] D46603: [Support] Print TimeRecord as CSV

2018-05-08 Thread George Karpenkov via Phabricator via cfe-commits

george.karpenkov added a comment.

@lebedev.ri LLVM already has facilities for outputting statistics in JSON, it 
seems that would be sufficient for your purposes?
I did something similar for the static analyzer in 
https://reviews.llvm.org/D43131


Repository:
  rL LLVM

https://reviews.llvm.org/D46603



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

[PATCH] D46332: [X86] Only enable the ud2 and int2c builtins if intrin.h has been included.

2018-05-08 Thread Reid Kleckner via Phabricator via cfe-commits

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

lgtm


https://reviews.llvm.org/D46332



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

[libcxx] r331818 - Revert "Emit an error when mixing and "

2018-05-08 Thread Volodymyr Sapsai via cfe-commits

Author: vsapsai
Date: Tue May  8 15:50:35 2018
New Revision: 331818

URL: http://llvm.org/viewvc/llvm-project?rev=331818=rev
Log:
Revert "Emit an error when mixing  and "

It reverts commit r331379 because turned out `__ALLOW_STDC_ATOMICS_IN_CXX__`
doesn't work well in practice.

Removed:
libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp
Modified:
libcxx/trunk/include/atomic

Modified: libcxx/trunk/include/atomic
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/atomic?rev=331818=331817=331818=diff
==
--- libcxx/trunk/include/atomic (original)
+++ libcxx/trunk/include/atomic Tue May  8 15:50:35 2018
@@ -555,9 +555,6 @@ void atomic_signal_fence(memory_order m)
 #if !defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_GCC_ATOMIC_IMP)
 #error  is not implemented
 #endif
-#ifdef __ALLOW_STDC_ATOMICS_IN_CXX__
-#error  is incompatible with the C++ standard library
-#endif
 
 #if _LIBCPP_STD_VER > 14
 # define __cpp_lib_atomic_is_always_lock_free 201603L

Removed: libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp?rev=331817=auto
==
--- libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp (original)
+++ libcxx/trunk/test/libcxx/atomics/c_compatibility.fail.cpp (removed)
@@ -1,28 +0,0 @@
-//===--===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===//
-//
-// UNSUPPORTED: libcpp-has-no-threads
-//
-// 
-
-// Test that including  fails to compile when we want to use C atomics
-// in C++ and have corresponding macro defined.
-
-// MODULES_DEFINES: __ALLOW_STDC_ATOMICS_IN_CXX__
-#ifndef __ALLOW_STDC_ATOMICS_IN_CXX__
-#define __ALLOW_STDC_ATOMICS_IN_CXX__
-#endif
-
-#include 
-// expected-error@atomic:* {{ is incompatible with the C++ 
standard library}}
-
-int main()
-{
-}
-


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

[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files

2018-05-08 Thread Joe via Phabricator via cfe-commits

rja added a comment.

+1 for JSON


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46602



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

[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files

2018-05-08 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

In https://reviews.llvm.org/D46602#1092084, @Eugene.Zelenko wrote:

> I think will be good idea to store data in JSON format too.


Yeah, i have thought about it, and i'm not sure.
The output is so dumb so there isn't even much point in using anything more 
advanced than CSV.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46602



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

[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files

2018-05-08 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

I think will be good idea to store data in JSON format too.




Comment at: docs/ReleaseNotes.rst:60
 
+- clang-tidy learned to store checks profiling info as CSV files.
+

May be //Profile information could be stored in SSV format.// sounds better?



Comment at: docs/clang-tidy/index.rst:762
+
+To enable profiling info collection, use ``-enable-check-profile`` argument.
+The timings will be outputted to the ``stderr`` as a table. Example output:

Please use ` for command line options and other non-C/C++ constructs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46602



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

r331802 - Add missing newlines to cl::extrahelp uses

2018-05-08 Thread Stephane Sezer via cfe-commits

Author: sas
Date: Tue May  8 12:46:29 2018
New Revision: 331802

URL: http://llvm.org/viewvc/llvm-project?rev=331802=rev
Log:
Add missing newlines to cl::extrahelp uses

Modified:
cfe/trunk/docs/LibASTMatchersTutorial.rst
cfe/trunk/docs/LibTooling.rst
cfe/trunk/include/clang/Tooling/CommonOptionsParser.h

Modified: cfe/trunk/docs/LibASTMatchersTutorial.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersTutorial.rst?rev=331802=331801=331802=diff
==
--- cfe/trunk/docs/LibASTMatchersTutorial.rst (original)
+++ cfe/trunk/docs/LibASTMatchersTutorial.rst Tue May  8 12:46:29 2018
@@ -146,7 +146,7 @@ documentation `_.
   static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
   // A help message for this specific tool can be added afterwards.
-  static cl::extrahelp MoreHelp("\nMore help text...");
+  static cl::extrahelp MoreHelp("\nMore help text...\n");
 
   int main(int argc, const char **argv) {
 CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);

Modified: cfe/trunk/docs/LibTooling.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibTooling.rst?rev=331802=331801=331802=diff
==
--- cfe/trunk/docs/LibTooling.rst (original)
+++ cfe/trunk/docs/LibTooling.rst Tue May  8 12:46:29 2018
@@ -130,7 +130,7 @@ version of this example tool is also che
   static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
 
   // A help message for this specific tool can be added afterwards.
-  static cl::extrahelp MoreHelp("\nMore help text...");
+  static cl::extrahelp MoreHelp("\nMore help text...\n");
 
   int main(int argc, const char **argv) {
 CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);

Modified: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CommonOptionsParser.h?rev=331802=331801=331802=diff
==
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h (original)
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h Tue May  8 12:46:29 
2018
@@ -52,7 +52,7 @@ namespace tooling {
 ///
 /// static cl::OptionCategory MyToolCategory("My tool options");
 /// static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
-/// static cl::extrahelp MoreHelp("\nMore help text...");
+/// static cl::extrahelp MoreHelp("\nMore help text...\n");
 /// static cl::opt YourOwnOption(...);
 /// ...
 ///


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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

In https://reviews.llvm.org/D42933#1092048, @rjmccall wrote:

> I agree that the format-specifier checker is not intended to be a portability 
> checker.
>
> Any attempt to check portability problems has to account for two things:
>
> - Not all code is intended to be portable.  If you're going to warn about 
> portability problems, you need some way to not annoy programmers who might 
> have good reason to say that they only care about their code running on Linux 
> / macOS / 64-bit / 32-bit / whatever.  Generally this means splitting the 
> portability warning out as a separate warning group.
> - There are always established idioms for solving portability problems.  In 
> this case, a certain set of important typedefs from the C standard have been 
> blessed with dedicated format modifiers like "z", but every other typedef in 
> the world has to find some other solution, either by asserting that some 
> existing format is good enough (as NSInteger does) or by introducing a macro 
> that expands to an appropriate format (as some things in POSIX do).  A proper 
> format-portability warning would have to know about all of these conventions 
> (in order to check that e.g. part of the format string came from the right 
> macro), which just seems wildly out-of-scope for a compiler warning.


Apple's current recommendation for using printf with the NSInteger types is 
casting to a long, per 
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html.
 Are you suggesting that recommendation would change to using `%zd` instead?


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

2018-05-08 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp:67
+  llvm::SmallDenseMap IncludeDirectives;
+  llvm::StringMap IsSystem;
+

Rather than use this `StringMap`, can you change `InclusionDirective()` to 
filter out includes you don't care about? Something along the lines of:
```
FileID FID = SM.translateFile(File);
assert(FID != FileID() && "Source Manager does not know about this header 
file");

bool Invalid;
const SLocEntry  = SM.getSLocEntry(FID, );
if (!Invalid && SrcMgr::isSystem(Entry.getFile().getFileCharacteristic())) {
  // It's a system file.
}
```
This way you don't have to store all the path information and test against 
paths in `EndOfMainFile()`, unless I'm missing something. Note, this is 
untested code and perhaps that assert will fire (maybe SourceManager is unaware 
of this file by the time InclusionDirective() is called).

Alternatively, you could alter InclusionDirective() to also pass in the 
`CharacteristicKind` calculated by `Preprocessor::HandleIncludeDirective()` as 
that seems like generally useful information for the callback to have on hand.



Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.h:1-2
+//===--- RestrictSystemIncludesCheck.h - clang-tidy-*-
+//C++-*-===//
+//

This should only be on a single line --  you can remove some `-` characters.



Comment at: docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst:32
+   A string containing a semi-colon separated list of allowed include 
filenames.
+   The default is an empty string, which allows all includes.

This default seems a bit odd to me, but perhaps it's fine. What's novel is that 
the check is a no-op by default, so how do Fuchsia developers get the correct 
list? Or is there no canonical list and developers are expected to populate 
their own manually?


https://reviews.llvm.org/D43778



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

[Diffusion] rL331456: [clang-tidy] Remove AnalyzeTemporaryDtors option.

2018-05-08 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added subscribers: cfe-commits, alexfh, lebedev.ri.
lebedev.ri raised a concern with this commit.
lebedev.ri added a comment.

Every single `.clang-tidy` config file that still happens to contain
`AnalyzeTemporaryDtors: true/false` param specified is now **silently** (!) 
ignored,
and a default config is used. I have found that out the hard way.


Users:
  alexfh (Author)
  lebedev.ri (Auditor)

https://reviews.llvm.org/rL331456



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

I agree that the format-specifier checker is not intended to be a portability 
checker.

Any attempt to check portability problems has to account for two things:

- Not all code is intended to be portable.  If you're going to warn about 
portability problems, you need some way to not annoy programmers who might have 
good reason to say that they only care about their code running on Linux / 
macOS / 64-bit / 32-bit / whatever.  Generally this means splitting the 
portability warning out as a separate warning group.
- There are always established idioms for solving portability problems.  In 
this case, a certain set of important typedefs from the C standard have been 
blessed with dedicated format modifiers like "z", but every other typedef in 
the world has to find some other solution, either by asserting that some 
existing format is good enough (as NSInteger does) or by introducing a macro 
that expands to an appropriate format (as some things in POSIX do).  A proper 
format-portability warning would have to know about all of these conventions 
(in order to check that e.g. part of the format string came from the right 
macro), which just seems wildly out-of-scope for a compiler warning.


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D46603: [Support] Print TimeRecord as CSV

2018-05-08 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza, bkramer, george.karpenkov.

This is needed for the continuation of https://reviews.llvm.org/D46504,
to be able to store the timings as CSV.

The floating-point values are dumped with no precision loss.

See dependent differential https://reviews.llvm.org/D46602 for details/use-case.


Repository:
  rL LLVM

https://reviews.llvm.org/D46603

Files:
  include/llvm/Support/Timer.h
  lib/Support/Timer.cpp


Index: lib/Support/Timer.cpp
===
--- lib/Support/Timer.cpp
+++ lib/Support/Timer.cpp
@@ -22,6 +22,8 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+
 using namespace llvm;
 
 // This ugly hack is brought to you courtesy of constructor/destructor ordering
@@ -169,6 +171,35 @@
 OS << format("%9" PRId64 "  ", (int64_t)getMemUsed());
 }
 
+static void dumpVal(double Val, raw_ostream ) {
+  constexpr auto max_digits10 = std::numeric_limits::max_digits10;
+  // FIXME: can ',' be a decimal separator?
+  OS << format("%.*e", max_digits10 - 1, Val);
+}
+
+void TimeRecord::printCSV(const TimeRecord , raw_ostream ) const {
+  int Column = 0;
+  auto comma = [, ]() {
+if (Column)
+  OS << ','; // FIXME: can ',' be a decimal separator?
+++Column;
+  };
+  auto printColumn = [comma, ](bool Print, double Val) {
+if (!Print)
+  return;
+comma();
+dumpVal(Val, OS);
+  };
+
+  printColumn(Total.getUserTime(), getUserTime());
+  printColumn(Total.getSystemTime(), getSystemTime());
+  printColumn(Total.getProcessTime(), getProcessTime());
+  printColumn(true, getWallTime());
+  if (Total.getMemUsed()) {
+comma();
+OS << format("%9" PRId64, (int64_t)getMemUsed());
+  }
+}
 
 
//===--===//
 //   NamedRegionTimer Implementation
Index: include/llvm/Support/Timer.h
===
--- include/llvm/Support/Timer.h
+++ include/llvm/Support/Timer.h
@@ -64,6 +64,10 @@
   /// Print the current time record to \p OS, with a breakdown showing
   /// contributions to the \p Total time record.
   void print(const TimeRecord , raw_ostream ) const;
+
+  /// Print the current time record to \p OS as CSV, with full precision.
+  /// Only the values in this timer are printed, no percentages.
+  void printCSV(const TimeRecord , raw_ostream ) const;
 };
 
 /// This class is used to track the amount of time spent between invocations of


Index: lib/Support/Timer.cpp
===
--- lib/Support/Timer.cpp
+++ lib/Support/Timer.cpp
@@ -22,6 +22,8 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+
 using namespace llvm;
 
 // This ugly hack is brought to you courtesy of constructor/destructor ordering
@@ -169,6 +171,35 @@
 OS << format("%9" PRId64 "  ", (int64_t)getMemUsed());
 }
 
+static void dumpVal(double Val, raw_ostream ) {
+  constexpr auto max_digits10 = std::numeric_limits::max_digits10;
+  // FIXME: can ',' be a decimal separator?
+  OS << format("%.*e", max_digits10 - 1, Val);
+}
+
+void TimeRecord::printCSV(const TimeRecord , raw_ostream ) const {
+  int Column = 0;
+  auto comma = [, ]() {
+if (Column)
+  OS << ','; // FIXME: can ',' be a decimal separator?
+++Column;
+  };
+  auto printColumn = [comma, ](bool Print, double Val) {
+if (!Print)
+  return;
+comma();
+dumpVal(Val, OS);
+  };
+
+  printColumn(Total.getUserTime(), getUserTime());
+  printColumn(Total.getSystemTime(), getSystemTime());
+  printColumn(Total.getProcessTime(), getProcessTime());
+  printColumn(true, getWallTime());
+  if (Total.getMemUsed()) {
+comma();
+OS << format("%9" PRId64, (int64_t)getMemUsed());
+  }
+}
 
 //===--===//
 //   NamedRegionTimer Implementation
Index: include/llvm/Support/Timer.h
===
--- include/llvm/Support/Timer.h
+++ include/llvm/Support/Timer.h
@@ -64,6 +64,10 @@
   /// Print the current time record to \p OS, with a breakdown showing
   /// contributions to the \p Total time record.
   void print(const TimeRecord , raw_ostream ) const;
+
+  /// Print the current time record to \p OS as CSV, with full precision.
+  /// Only the values in this timer are printed, no percentages.
+  void printCSV(const TimeRecord , raw_ostream ) const;
 };
 
 /// This class is used to track the amount of time spent between invocations of
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46602: [clang-tidy] Store checks profiling info as CSV files

2018-05-08 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza.
Herald added subscribers: mgrang, xazax.hun.

Continuation of https://reviews.llvm.org/D46504.

Example output:

  $ clang-tidy -enable-check-profile -store-check-profile=. 
-store-check-profile-elide-prefix=. -checks=-*,readability-function-size 
source.cpp 2>&1 >/dev/null
  $ cat .csv
  "User Time","System Time","User+System","Wall Time","Name"
  
8.842049973945e-01,1.20267757e-01,1.004472997370e+00,1.0031676292419434e+00,"readability-function-size"
  
8.842049973945e-01,1.20267757e-01,1.004472997370e+00,1.0031676292419434e+00,"Total"

There are two arguments that control profile storage:

- `-store-check-profile=` This option controls the prefix where these 
per-TU profiles are stored as CSV. If the prefix is not an absolute path, it is 
considered to be relative to the directory from where you have run 
`clang-tidy`. All `.` and `..` patterns in the path are collapsed, and symlinks 
are resolved.

  **Example**: Let's suppose you have a source file named `example.cpp`, 
located in `/source` directory.
  - If you specify `-store-check-profile=/tmp`, then the profile will be saved 
to `/tmp/source/example.cpp.csv`
  - If you run `clang-tidy` from within `/foo` directory, and specify 
`-store-check-profile=.`, then the profile will still be saved to 
`/foo/source/example.cpp.csv`
- `-store-check-profile-elide-prefix=` When specified, this prefix will 
be elided from the source file name, before prepending it with the prefix 
specified by `-store-check-profile`. If the prefix is not an absolute path, it 
is considered to be relative to the directory from where you have run 
`clang-tidy`. All `.` and `..` patterns in the path are collapsed, and symlinks 
are resolved.

  **Example**: Let's suppose you have a source file named `example.cpp`, 
located in `/source` directory.
  - If you specify `-store-check-profile=/tmp 
-store-check-profile-elide-prefix=/source` , then the profile will be saved to 
`/tmp/example.cpp.csv`
  - If you run `clang-tidy` from within `/source` directory, and specify 
`-store-check-profile=/foo -store-check-profile-elide-prefix=.`, then the 
profile will be saved to `/foo/example.cpp.csv`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46602

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidy.h
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/ClangTidyProfiling.cpp
  clang-tidy/ClangTidyProfiling.h
  clang-tidy/tool/ClangTidyMain.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp

Index: test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp
===
--- /dev/null
+++ test/clang-tidy/clang-tidy-store-check-profile-one-tu.cpp
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' -store-check-profile=%T -store-check-profile-elide-prefix=%s %s 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK-CONSOLE %s
+// RUN: FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' -input-file=%T/.csv -check-prefix=CHECK-FILE %s
+
+// CHECK-CONSOLE: ===-===
+// CHECK-CONSOLE-NEXT: {{.*}}  --- Name ---
+// CHECK-CONSOLE-NEXT: {{.*}}  readability-function-size
+// CHECK-CONSOLE-NEXT: {{.*}}  Total
+// CHECK-CONSOLE-NEXT: ===-===
+
+// CHECK-FILE: {{.*}}"Wall Time","Name"
+// CHECK-FILE-NEXT: {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}},"readability-function-size"
+// CHECK-FILE-NEXT: {{.*}}{{[0-9]}}.{{[0-9]+}}e{{[-+]}}{{[0-9]}}{{[0-9]}},"Total"
+
+class A {
+  A() {}
+  ~A() {}
+};
Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -99,114 +99,127 @@
 
 .. code-block:: console
 
-  $ clang-tidy -help
+  $ clang-tidy --help
   USAGE: clang-tidy [options]  [... ]
 
   OPTIONS:
 
   Generic Options:
 
--help- Display available options (-help-hidden for more)
--help-list   - Display list of available options (-help-list-hidden for more)
--version - Display the version of this program
+-help  - Display available options (-help-hidden for more)
+-help-list - Display list of available options (-help-list-hidden for more)
+-version   - Display the version of this program
 
   clang-tidy options:
 
--checks= -
-   Comma-separated list of globs with optional '-'
-   prefix. Globs

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

2018-05-08 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:116
+
+  Checks for allowed system includes and suggests removal of any others. If no
+  includes are specified, the check will exit without issuing any warnings.

Is it necessary to highlight that warnings will not be emitted in case of 
disallowed headers are not found? Same in documentation.


https://reviews.llvm.org/D43778



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

r331812 - Fix float->int conversion warnings when near barriers.

2018-05-08 Thread Erich Keane via cfe-commits

Author: erichkeane
Date: Tue May  8 14:26:21 2018
New Revision: 331812

URL: http://llvm.org/viewvc/llvm-project?rev=331812=rev
Log:
Fix float->int conversion warnings when near barriers.

As Eli brought up here: https://reviews.llvm.org/D46535
I'd previously messed up this fix by missing conversions
that are just slightly outside the range.  This patch fixes
this by no longer ignoring the return value of 
convertToInteger.  Additionally, one of the error messages
wasn't very sensical (mentioning out of range value, when it 
really was not), so it was cleaned up as well.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
cfe/trunk/test/SemaCXX/warn-literal-conversion.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=331812=331811=331812=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May  8 14:26:21 
2018
@@ -3148,8 +3148,7 @@ def warn_impcast_float_integer : Warning
   InGroup, DefaultIgnore;
 
 def warn_impcast_float_to_integer : Warning<
-  "implicit conversion of out of range value from %0 to %1 changes value "
-  "from %2 to %3">,
+  "implicit conversion from %0 to %1 changes value from %2 to %3">,
   InGroup, DefaultIgnore;
 def warn_impcast_float_to_integer_out_of_range : Warning<
   "implicit conversion of out of range value from %0 to %1 is undefined">,

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=331812=331811=331812=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue May  8 14:26:21 2018
@@ -9419,26 +9419,25 @@ static void DiagnoseFloatingImpCast(Sema
 
   llvm::APSInt IntegerValue(S.Context.getIntWidth(T),
 T->hasUnsignedIntegerRepresentation());
-  if (Value.convertToInteger(IntegerValue, llvm::APFloat::rmTowardZero,
- ) == llvm::APFloat::opOK &&
-  isExact) {
+  llvm::APFloat::opStatus Result = Value.convertToInteger(
+  IntegerValue, llvm::APFloat::rmTowardZero, );
+
+  if (Result == llvm::APFloat::opOK && isExact) {
 if (IsLiteral) return;
 return DiagnoseImpCast(S, E, T, CContext, diag::warn_impcast_float_integer,
PruneWarnings);
   }
 
+  // Conversion of a floating-point value to a non-bool integer where the
+  // integral part cannot be represented by the integer type is undefined.
+  if (!IsBool && Result == llvm::APFloat::opInvalidOp)
+return DiagnoseImpCast(
+S, E, T, CContext,
+IsLiteral ? diag::warn_impcast_literal_float_to_integer_out_of_range
+  : diag::warn_impcast_float_to_integer_out_of_range);
+
   unsigned DiagID = 0;
   if (IsLiteral) {
-// Conversion of a floating-point value to a non-bool integer where the
-// integral part cannot be represented by the integer type is undefined.
-if (!IsBool &&
-((IntegerValue.isSigned() && (IntegerValue.isMaxSignedValue() ||
-  IntegerValue.isMinSignedValue())) ||
- (IntegerValue.isUnsigned() &&
-  (IntegerValue.isMaxValue() || IntegerValue.isMinValue()
-  return DiagnoseImpCast(
-  S, E, T, CContext,
-  diag::warn_impcast_literal_float_to_integer_out_of_range);
 // Warn on floating point literal to integer.
 DiagID = diag::warn_impcast_literal_float_to_integer;
   } else if (IntegerValue == 0) {
@@ -9454,19 +9453,12 @@ static void DiagnoseFloatingImpCast(Sema
 return DiagnoseImpCast(S, E, T, CContext,
diag::warn_impcast_float_integer, 
PruneWarnings);
   }
-  if (!IsBool && (IntegerValue.isMaxValue() || IntegerValue.isMinValue()))
-return DiagnoseImpCast(S, E, T, CContext,
-   
diag::warn_impcast_float_to_integer_out_of_range,
-   PruneWarnings);
 } else {  // IntegerValue.isSigned()
   if (!IntegerValue.isMaxSignedValue() &&
   !IntegerValue.isMinSignedValue()) {
 return DiagnoseImpCast(S, E, T, CContext,
diag::warn_impcast_float_integer, 
PruneWarnings);
   }
-  return DiagnoseImpCast(S, E, T, CContext,
- diag::warn_impcast_float_to_integer_out_of_range,
- PruneWarnings);
 }
 // Warn on evaluatable floating point expression to integer conversion.
 DiagID = diag::warn_impcast_float_to_integer;

Modified: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
URL:

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

2018-05-08 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

I think the request was that we check that a type is trivially copyable when we 
perform an atomic operation?  I don't see the code for that anywhere.

Also needs some test coverage for atomic operations which aren't calls, like 
"typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2; };".


https://reviews.llvm.org/D46112



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

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

2018-05-08 Thread Julie Hockett via Phabricator via cfe-commits

juliehockett updated this revision to Diff 145788.
juliehockett marked 10 inline comments as done.
juliehockett added a comment.

Made the check for system headers more comprehensive & fixed newline issues


https://reviews.llvm.org/D43778

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp
  clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/a.h
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/j.h
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/s.h
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/t.h
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h
  test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h
  test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp
  test/clang-tidy/fuchsia-restrict-system-includes.cpp

Index: test/clang-tidy/fuchsia-restrict-system-includes.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-restrict-system-includes.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
+// RUN:		-- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 's.h'}]}" \
+// RUN:   -- -std=c++11 -I %S/Inputs/fuchsia-restrict-system-includes -isystem %S/Inputs/fuchsia-restrict-system-includes/system
+
+#include "a.h"
+
+#include 
+#include 
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed
+// CHECK-FIXES-NOT: #include 
+
+#include "s.h"
+#include "t.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include t.h not allowed
+// CHECK-FIXES-NOT: #include "t.h"
+
+#define foo 
+
+#include foo
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include j.h not allowed
+// CHECK-FIXES-NOT: #include foo
+
+#/* comment */ include /* comment */ foo
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system include j.h not allowed
+// CHECK-FIXES-NOT: # /* comment */ include /* comment */ foo
Index: test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-restrict-system-includes-headers.cpp
@@ -0,0 +1,20 @@
+// RUN: cp -r %S/Inputs/fuchsia-restrict-system-includes %T/Inputs
+// RUN: %check_clang_tidy %s fuchsia-restrict-system-includes %t \
+// RUN:		-- -config="{CheckOptions: [{key: fuchsia-restrict-system-includes.Includes, value: 'transitive.h;s.h'}]}" \
+// RUN:   -system-headers -header-filter=.* \
+// RUN:   -- -std=c++11 -I %T/Inputs/fuchsia-restrict-system-includes -isystem %T/Inputs/fuchsia-restrict-system-includes/system
+// RUN: FileCheck -input-file=%T/Inputs/transitive2.h %s -check-prefix=CHECK-HEADER-FIXES
+
+// transitive.h includes  and 
+#include 
+// CHECK-MESSAGES: :1:1: warning: system include r.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/system/transitive.h
+// CHECK-MESSAGES: :2:1: warning: system include t.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/system/transitive.h
+
+// transitive.h includes  and 
+#include "transitive2.h"
+// CHECK-MESSAGES: :2:1: warning: system include t.h not allowed, transitively included from {{(.*\/)*}}Inputs/fuchsia-restrict-system-includes/transitive2.h
+// CHECK-HEADER-FIXES-NOT: #include 
+
+int main() {
+  // f() is declared in r.h
+}
Index: test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/fuchsia-restrict-system-includes/transitive2.h
@@ -0,0 +1,2 @@
+#include 
+#include 
Index: test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/transitive.h
@@ -0,0 +1,3 @@
+#include 
+#include 
+#include 
Index: test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/fuchsia-restrict-system-includes/system/r.h
@@ -0,0 +1 @@
+void f() {}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -95,6 +95,7 @@
fuchsia-default-arguments
fuchsia-multiple-inheritance
fuchsia-overloaded-operator
+   fuchsia-restrict-system-includes
fuchsia-statically-constructed-objects
fuchsia-trailing-return
fuchsia-virtual-inheritance
Index:

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a subscriber: rsmith.
smeenai added a comment.

In https://reviews.llvm.org/D42933#1091943, @jyknight wrote:

> In https://reviews.llvm.org/D42933#1091817, @jfb wrote:
>
> > In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:
> >
> > > I also think that special casing these two specifiers doesn't make sense. 
> > > The problem is a general issue -- and one I've often found irritating. 
> > > This exact same situation comes up all the time in non-Darwin contexts 
> > > too.
> >
> >
> > I don't think that's true. In this very specific case the platform 
> > guarantees that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) 
> > == sizeof(NSUInteger))` for all architectures this platform supports. This 
> > exact same situation does not come up all the time in other contexts 
> > because the `int` / `long` / `long long` distinction isn't backed by a 
> > portability guarantee. The warning is there to say "this code isn't 
> > portable!", but in the very specific case of `NSInteger` and `NSUInteger` 
> > it is and users rely on it so it cannot be broken. The warning is therefore 
> > spurious, users therefore rightly complain.
>
>
> The printf format specifier warning is not primarily a cross-platform 
> portability checker. And, although in a few limited cases it can act somewhat 
> like one, in general it does not. Take my previous example -- you get no 
> warning on a platform that has int64_t as a typedef for long -- if this 
> feature is to be useful as a portability checker, it should require that you 
> used the PRId64 macro. Or, given `ssize_t x = 0; printf("%ld", x);`, it 
> doesn't tell you to use "%zd" instead if ssize_t is a typedef for long -- 
> although to be portable you ought to.
>
> No, the major usefulness of the printf warning is to tell you that your code 
> is incorrect for the _current_ platform. And //most// importantly when you 
> chose the wrong size for your argument.
>
> Those types which have matched size and alignment are still different types, 
> and so it's technically appropriate to warn about using the wrong specifier 
> there, too. But it's also entirely reasonable to not want to be bothered by 
> such useless trivia, so skipping the warning when there's only a technical 
> and not actual mismatch seems entirely sensible. (I might suggest that it 
> should even be the default behavior for the warning, and if you want the 
> stricter checks you'd ask for them...but I bet I'll get more pushback on 
> that...)


+1 to everything you said; what are other people's opinions on this? @rsmith 
perhaps?


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D46471: [HIP] Add hip offload kind

2018-05-08 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rL331811: [HIP] Add hip offload kind (authored by yaxunl, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46471?vs=145472=145780#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46471

Files:
  cfe/trunk/include/clang/Driver/Action.h
  cfe/trunk/lib/Driver/Action.cpp
  cfe/trunk/lib/Driver/Compilation.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -131,6 +131,10 @@
 Work(*C.getSingleOffloadToolChain());
   else if (JA.isDeviceOffloading(Action::OFK_Cuda))
 Work(*C.getSingleOffloadToolChain());
+  else if (JA.isHostOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
+  else if (JA.isDeviceOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
 
   if (JA.isHostOffloading(Action::OFK_OpenMP)) {
 auto TCs = C.getOffloadToolChains();
@@ -3105,13 +3109,14 @@
   // Check number of inputs for sanity. We need at least one input.
   assert(Inputs.size() >= 1 && "Must have at least one input.");
   const InputInfo  = Inputs[0];
-  // CUDA compilation may have multiple inputs (source file + results of
+  // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also take the host IR as a
   // second input. All other jobs are expected to have exactly one
   // input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
+  bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
-  assert((IsCuda || (IsOpenMPDevice && Inputs.size() == 2) ||
+  assert((IsCuda || IsHIP || (IsOpenMPDevice && Inputs.size() == 2) ||
   Inputs.size() == 1) &&
  "Unable to handle multiple inputs.");
 
@@ -3123,10 +3128,10 @@
   bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
   bool IsIAMCU = RawTriple.isOSIAMCU();
 
-  // Adjust IsWindowsXYZ for CUDA compilations.  Even when compiling in device
-  // mode (i.e., getToolchain().getTriple() is NVPTX, not Windows), we need to
-  // pass Windows-specific flags to cc1.
-  if (IsCuda) {
+  // Adjust IsWindowsXYZ for CUDA/HIP compilations.  Even when compiling in
+  // device mode (i.e., getToolchain().getTriple() is NVPTX/AMDGCN, not
+  // Windows), we need to pass Windows-specific flags to cc1.
+  if (IsCuda || IsHIP) {
 IsWindowsMSVC |= AuxTriple && AuxTriple->isWindowsMSVCEnvironment();
 IsWindowsGNU |= AuxTriple && AuxTriple->isWindowsGNUEnvironment();
 IsWindowsCygnus |= AuxTriple && AuxTriple->isWindowsCygwinEnvironment();
@@ -3150,18 +3155,21 @@
 Args.ClaimAllArgs(options::OPT_MJ);
   }
 
-  if (IsCuda) {
-// We have to pass the triple of the host if compiling for a CUDA device and
-// vice-versa.
+  if (IsCuda || IsHIP) {
+// We have to pass the triple of the host if compiling for a CUDA/HIP device
+// and vice-versa.
 std::string NormalizedTriple;
-if (JA.isDeviceOffloading(Action::OFK_Cuda))
+if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+JA.isDeviceOffloading(Action::OFK_HIP))
   NormalizedTriple = C.getSingleOffloadToolChain()
  ->getTriple()
  .normalize();
 else
-  NormalizedTriple = C.getSingleOffloadToolChain()
- ->getTriple()
- .normalize();
+  NormalizedTriple =
+  (IsCuda ? C.getSingleOffloadToolChain()
+  : C.getSingleOffloadToolChain())
+  ->getTriple()
+  .normalize();
 
 CmdArgs.push_back("-aux-triple");
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
Index: cfe/trunk/lib/Driver/Compilation.cpp
===
--- cfe/trunk/lib/Driver/Compilation.cpp
+++ cfe/trunk/lib/Driver/Compilation.cpp
@@ -196,10 +196,10 @@
   if (FailingCommands.empty())
 return false;
 
-  // CUDA can have the same input source code compiled multiple times so do not
-  // compiled again if there are already failures. It is OK to abort the CUDA
-  // pipeline on errors.
-  if (A->isOffloading(Action::OFK_Cuda))
+  // CUDA/HIP can have the same input source code compiled multiple times so do
+  // not compiled again if there are already failures. It is OK to abort the
+  // CUDA pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))
 return true;
 
   for (const auto  : FailingCommands)
Index: cfe/trunk/lib/Driver/Action.cpp
===
--- cfe/trunk/lib/Driver/Action.cpp
+++

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread James Y Knight via Phabricator via cfe-commits

jyknight added a comment.

In https://reviews.llvm.org/D42933#1091817, @jfb wrote:

> In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:
>
> > I also think that special casing these two specifiers doesn't make sense. 
> > The problem is a general issue -- and one I've often found irritating. This 
> > exact same situation comes up all the time in non-Darwin contexts too.
>
>
> I don't think that's true. In this very specific case the platform guarantees 
> that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> sizeof(NSUInteger))` for all architectures this platform supports. This exact 
> same situation does not come up all the time in other contexts because the 
> `int` / `long` / `long long` distinction isn't backed by a portability 
> guarantee. The warning is there to say "this code isn't portable!", but in 
> the very specific case of `NSInteger` and `NSUInteger` it is and users rely 
> on it so it cannot be broken. The warning is therefore spurious, users 
> therefore rightly complain.

The printf format specifier warning is not primarily a cross-platform 
portability checker. And, although in a few limited cases it can act somewhat 
like one, in general it does not. Take my previous example -- you get no 
warning on a platform that has int64_t as a typedef for long -- if this feature 
is to be useful as a portability checker, it should require that you used the 
PRId64 macro. Or, given `ssize_t x = 0; printf("%ld", x);`, it doesn't tell you 
to use "%zd" instead if ssize_t is a typedef for long -- although to be 
portable you ought to.

No, the major usefulness of the printf warning is to tell you that your code is 
incorrect for the _current_ platform. And //most// importantly when you chose 
the wrong size for your argument.

Those types which have matched size and alignment are still different types, 
and so it's technically appropriate to warn about using the wrong specifier 
there, too. But it's also entirely reasonable to not want to be bothered by 
such useless trivia, so skipping the warning when there's only a technical and 
not actual mismatch seems entirely sensible. (I might suggest that it should 
even be the default behavior for the warning, and if you want the stricter 
checks you'd ask for them...but I bet I'll get more pushback on that...)

Repository:
  rC Clang

https://reviews.llvm.org/D42933

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

r331811 - [HIP] Add hip offload kind

2018-05-08 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Tue May  8 14:02:12 2018
New Revision: 331811

URL: http://llvm.org/viewvc/llvm-project?rev=331811=rev
Log:
[HIP] Add hip offload kind

There are quite differences in HIP action builder and action job creation,
which justifies to define a separate offload kind.

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

Modified:
cfe/trunk/include/clang/Driver/Action.h
cfe/trunk/lib/Driver/Action.cpp
cfe/trunk/lib/Driver/Compilation.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/include/clang/Driver/Action.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Action.h?rev=331811=331810=331811=diff
==
--- cfe/trunk/include/clang/Driver/Action.h (original)
+++ cfe/trunk/include/clang/Driver/Action.h Tue May  8 14:02:12 2018
@@ -88,6 +88,7 @@ public:
 // The device offloading tool chains - one bit for each programming model.
 OFK_Cuda = 0x02,
 OFK_OpenMP = 0x04,
+OFK_HIP = 0x08,
   };
 
   static const char *getClassName(ActionClass AC);

Modified: cfe/trunk/lib/Driver/Action.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Action.cpp?rev=331811=331810=331811=diff
==
--- cfe/trunk/lib/Driver/Action.cpp (original)
+++ cfe/trunk/lib/Driver/Action.cpp Tue May  8 14:02:12 2018
@@ -96,6 +96,8 @@ std::string Action::getOffloadingKindPre
 return "device-cuda";
   case OFK_OpenMP:
 return "device-openmp";
+  case OFK_HIP:
+return "device-hip";
 
 // TODO: Add other programming models here.
   }
@@ -104,8 +106,13 @@ std::string Action::getOffloadingKindPre
 return {};
 
   std::string Res("host");
+  assert(!((ActiveOffloadKindMask & OFK_Cuda) &&
+   (ActiveOffloadKindMask & OFK_HIP)) &&
+ "Cannot offload CUDA and HIP at the same time");
   if (ActiveOffloadKindMask & OFK_Cuda)
 Res += "-cuda";
+  if (ActiveOffloadKindMask & OFK_HIP)
+Res += "-hip";
   if (ActiveOffloadKindMask & OFK_OpenMP)
 Res += "-openmp";
 
@@ -142,6 +149,8 @@ StringRef Action::GetOffloadKindName(Off
 return "cuda";
   case OFK_OpenMP:
 return "openmp";
+  case OFK_HIP:
+return "hip";
 
 // TODO: Add other programming models here.
   }

Modified: cfe/trunk/lib/Driver/Compilation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=331811=331810=331811=diff
==
--- cfe/trunk/lib/Driver/Compilation.cpp (original)
+++ cfe/trunk/lib/Driver/Compilation.cpp Tue May  8 14:02:12 2018
@@ -196,10 +196,10 @@ static bool ActionFailed(const Action *A
   if (FailingCommands.empty())
 return false;
 
-  // CUDA can have the same input source code compiled multiple times so do not
-  // compiled again if there are already failures. It is OK to abort the CUDA
-  // pipeline on errors.
-  if (A->isOffloading(Action::OFK_Cuda))
+  // CUDA/HIP can have the same input source code compiled multiple times so do
+  // not compiled again if there are already failures. It is OK to abort the
+  // CUDA pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))
 return true;
 
   for (const auto  : FailingCommands)

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331811=331810=331811=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May  8 14:02:12 2018
@@ -131,6 +131,10 @@ forAllAssociatedToolChains(Compilation &
 Work(*C.getSingleOffloadToolChain());
   else if (JA.isDeviceOffloading(Action::OFK_Cuda))
 Work(*C.getSingleOffloadToolChain());
+  else if (JA.isHostOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
+  else if (JA.isDeviceOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
 
   if (JA.isHostOffloading(Action::OFK_OpenMP)) {
 auto TCs = C.getOffloadToolChains();
@@ -3105,13 +3109,14 @@ void Clang::ConstructJob(Compilation ,
   // Check number of inputs for sanity. We need at least one input.
   assert(Inputs.size() >= 1 && "Must have at least one input.");
   const InputInfo  = Inputs[0];
-  // CUDA compilation may have multiple inputs (source file + results of
+  // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also take the host IR as a
   // second input. All other jobs are expected to have exactly one
   // input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
+  bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
-  assert((IsCuda || (IsOpenMPDevice &&

r331810 - Add a mno-outline flag to disable the MachineOutliner

2018-05-08 Thread Jessica Paquette via cfe-commits

Author: paquette
Date: Tue May  8 13:58:32 2018
New Revision: 331810

URL: http://llvm.org/viewvc/llvm-project?rev=331810=rev
Log:
Add a mno-outline flag to disable the MachineOutliner

Since we're working on turning the MachineOutliner by default under -Oz for
AArch64, it makes sense to have an -mno-outline flag available. This currently
doesn't do much (it basically just undoes -moutline).

When the MachineOutliner is on by default under AArch64, this flag should
set -mllvm -enable-machine-outliner=never.

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331810=331809=331810=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue May  8 13:58:32 2018
@@ -1908,6 +1908,8 @@ def mms_bitfields : Flag<["-"], "mms-bit
   HelpText<"Set the default structure layout to be compatible with the 
Microsoft compiler standard">;
 def moutline : Flag<["-"], "moutline">, Group, 
Flags<[CC1Option]>,
 HelpText<"Enable function outlining (AArch64 only)">;
+def mno_outline : Flag<["-"], "mno-outline">, Group, 
Flags<[CC1Option]>,
+HelpText<"Disable function outlining (AArch64 only)">;
 def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group,
   HelpText<"Do not set the default structure layout to be compatible with the 
Microsoft compiler standard">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331810=331809=331810=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May  8 13:58:32 2018
@@ -1485,7 +1485,8 @@ void Clang::AddAArch64TargetArgs(const A
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (Args.getLastArg(options::OPT_moutline)) {
+  if (!Args.hasArg(options::OPT_mno_outline) &&
+   Args.getLastArg(options::OPT_moutline)) {
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back("-enable-machine-outliner");
   }

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331810=331809=331810=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue May  8 13:58:32 2018
@@ -1,4 +1,9 @@
 // REQUIRES: aarch64-registered-target
 
-// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
-// CHECK: "-mllvm" "-enable-machine-outliner"
+// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s 
-check-prefix=ON
+// ON: "-mllvm" "-enable-machine-outliner"
+
+// RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF1
+// RUN: %clang -target aarch64 -mno-outline -moutline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF2
+// OFF1-NOT: "-mllvm" "-enable-machine-outliner"
+// OFF2-NOT: "-mllvm" "-enable-machine-outliner"


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

[PATCH] D46287: [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets

2018-05-08 Thread Martin Storsjö via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331807: [Driver] Dont add -dwarf-column-info when 
using -gcodeview on non-msvc targets (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46287?vs=144630=145776#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46287

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/codeview-column-info.c


Index: cfe/trunk/test/Driver/codeview-column-info.c
===
--- cfe/trunk/test/Driver/codeview-column-info.c
+++ cfe/trunk/test/Driver/codeview-column-info.c
@@ -6,6 +6,8 @@
 // RUN: FileCheck < %t1 %s
 // RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
 // RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
 // RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
 // RUN: FileCheck < %t2 %s
 
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3000,7 +3000,7 @@
   // debuggers don't handle missing end columns well, so it's better not to
   // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+   /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
 CmdArgs.push_back("-dwarf-column-info");
 


Index: cfe/trunk/test/Driver/codeview-column-info.c
===
--- cfe/trunk/test/Driver/codeview-column-info.c
+++ cfe/trunk/test/Driver/codeview-column-info.c
@@ -6,6 +6,8 @@
 // RUN: FileCheck < %t1 %s
 // RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
 // RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
 // RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
 // RUN: FileCheck < %t2 %s
 
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3000,7 +3000,7 @@
   // debuggers don't handle missing end columns well, so it's better not to
   // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+   /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
 CmdArgs.push_back("-dwarf-column-info");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331807 - [Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets

2018-05-08 Thread Martin Storsjo via cfe-commits

Author: mstorsjo
Date: Tue May  8 13:55:23 2018
New Revision: 331807

URL: http://llvm.org/viewvc/llvm-project?rev=331807=rev
Log:
[Driver] Don't add -dwarf-column-info when using -gcodeview on non-msvc targets

-dwarf-column-info is omitted if -gcodeview is specified for msvc
targets at the moment, but since -gcodeview is an option that can be
specified for any target, there's little reason to restrict this
handling to msvc targets.

This allows getting proper codeview debug info by passing -gcodeview
for e.g. MinGW targets as well.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/codeview-column-info.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331807=331806=331807=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May  8 13:55:23 2018
@@ -3000,7 +3000,7 @@ static void RenderDebugOptions(const Too
   // debuggers don't handle missing end columns well, so it's better not to
   // include any column info.
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default=*/!(IsWindowsMSVC && EmitCodeView) &&
+   /*Default=*/!EmitCodeView &&
DebuggerTuning != llvm::DebuggerKind::SCE))
 CmdArgs.push_back("-dwarf-column-info");
 

Modified: cfe/trunk/test/Driver/codeview-column-info.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/codeview-column-info.c?rev=331807=331806=331807=diff
==
--- cfe/trunk/test/Driver/codeview-column-info.c (original)
+++ cfe/trunk/test/Driver/codeview-column-info.c Tue May  8 13:55:23 2018
@@ -6,6 +6,8 @@
 // RUN: FileCheck < %t1 %s
 // RUN: %clangxx -### --target=x86_64-windows-msvc -c -g -gcodeview %s 2> %t2
 // RUN: FileCheck < %t2 %s
+// RUN: %clangxx -### --target=x86_64-windows-gnu -c -g -gcodeview %s 2> %t2
+// RUN: FileCheck < %t2 %s
 // RUN: %clang_cl -### --target=x86_64-windows-msvc /c /Z7 -- %s 2> %t2
 // RUN: FileCheck < %t2 %s
 


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

r331806 - Change -foutline to -moutline

2018-05-08 Thread Jessica Paquette via cfe-commits

Author: paquette
Date: Tue May  8 13:53:19 2018
New Revision: 331806

URL: http://llvm.org/viewvc/llvm-project?rev=331806=rev
Log:
Change -foutline to -moutline

Nitpicky, but the MachineOutliner is a machine-level pass, and so we should
reflect that by using "m" instead of "n".

Figured we should get this in before people get used to the letter f. :)

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331806=331805=331806=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue May  8 13:53:19 2018
@@ -1317,8 +1317,6 @@ def fno_exceptions : Flag<["-"], "fno-ex
 def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group, 
Flags<[CC1Option]>;
 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, 
Group, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group, 
Flags<[CC1Option]>;
-def foutline : Flag<["-"], "foutline">, Group, 
Flags<[CC1Option]>,
-HelpText<"Enable function outlining (AArch64 only)">;
 def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">, 
Group,
   HelpText<"Disables the experimental global instruction selector">;
 def fno_experimental_new_pass_manager : Flag<["-"], 
"fno-experimental-new-pass-manager">,
@@ -1908,6 +1906,8 @@ def mmacos_version_min_EQ : Joined<["-"]
   Group, Alias;
 def mms_bitfields : Flag<["-"], "mms-bitfields">, Group, 
Flags<[CC1Option]>,
   HelpText<"Set the default structure layout to be compatible with the 
Microsoft compiler standard">;
+def moutline : Flag<["-"], "moutline">, Group, 
Flags<[CC1Option]>,
+HelpText<"Enable function outlining (AArch64 only)">;
 def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group,
   HelpText<"Do not set the default structure layout to be compatible with the 
Microsoft compiler standard">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331806=331805=331806=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May  8 13:53:19 2018
@@ -1485,7 +1485,7 @@ void Clang::AddAArch64TargetArgs(const A
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (Args.getLastArg(options::OPT_foutline)) {
+  if (Args.getLastArg(options::OPT_moutline)) {
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back("-enable-machine-outliner");
   }

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331806=331805=331806=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue May  8 13:53:19 2018
@@ -1,4 +1,4 @@
 // REQUIRES: aarch64-registered-target
 
-// RUN: %clang -target aarch64 -foutline -S %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
 // CHECK: "-mllvm" "-enable-machine-outliner"


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

[PATCH] D46601: [OpenCL] Fix typos in emitted enqueue kernel function names

2018-05-08 Thread Brian Sumner via Phabricator via cfe-commits

b-sumner added a comment.

Thanks!  Looks good to me.


https://reviews.llvm.org/D46601



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

[PATCH] D46601: [OpenCL] Fix typos in emitted enqueue kernel function names

2018-05-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added reviewers: Anastasia, b-sumner.

Two typos: 
vaarg => vararg
get_kernel_preferred_work_group_multiple => 
get_kernel_preferred_work_group_size_multiple


https://reviews.llvm.org/D46601

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl

Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -88,7 +88,7 @@
   // B64: %[[TMP:.*]] = alloca [1 x i64]
   // B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
   // B64: store i64 256, i64* %[[TMP1]], align 8
-  // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs(
+  // COMMON-LABEL: call i32 @__enqueue_kernel_varargs(
   // COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK1:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG1]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -109,7 +109,7 @@
   // B64: %[[TMP:.*]] = alloca [1 x i64]
   // B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
   // B64: store i64 %{{.*}}, i64* %[[TMP1]], align 8
-  // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs(
+  // COMMON-LABEL: call i32 @__enqueue_kernel_varargs(
   // COMMON-SAME: %opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK2:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG2]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -133,7 +133,7 @@
   // B64: %[[TMP:.*]] = alloca [1 x i64]
   // B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
   // B64: store i64 256, i64* %[[TMP1]], align 8
-  // COMMON-LABEL: call i32 @__enqueue_kernel_events_vaargs
+  // COMMON-LABEL: call i32 @__enqueue_kernel_events_varargs
   // COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]],  %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.*}} [[WAIT_EVNT]], %opencl.clk_event_t{{.*}} [[EVNT]],
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK3:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG3]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -157,7 +157,7 @@
   // B64: %[[TMP:.*]] = alloca [1 x i64]
   // B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
   // B64: store i64 %{{.*}}, i64* %[[TMP1]], align 8
-  // COMMON-LABEL: call i32 @__enqueue_kernel_events_vaargs
+  // COMMON-LABEL: call i32 @__enqueue_kernel_events_varargs
   // COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]],  %struct.ndrange_t* {{.*}}, i32 2, %opencl.clk_event_t{{.*}}* addrspace(4)* [[WAIT_EVNT]], %opencl.clk_event_t{{.*}}* addrspace(4)* [[EVNT]],
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK4:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG4]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -179,7 +179,7 @@
   // B64: %[[TMP:.*]] = alloca [1 x i64]
   // B64: %[[TMP1:.*]] = getelementptr [1 x i64], [1 x i64]* %[[TMP]], i32 0, i32 0
   // B64: store i64 %{{.*}}, i64* %[[TMP1]], align 8
-  // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs
+  // COMMON-LABEL: call i32 @__enqueue_kernel_varargs
   // COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK5:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG5]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 1,
@@ -208,7 +208,7 @@
   // B64: store i64 2, i64* %[[TMP2]], align 8
   // B64: %[[TMP3:.*]] = getelementptr [3 x i64], [3 x i64]* %[[TMP]], i32 0, i32 2
   // B64: store i64 4, i64* %[[TMP3]], align 8
-  // COMMON-LABEL: call i32 @__enqueue_kernel_vaargs
+  // COMMON-LABEL: call i32 @__enqueue_kernel_varargs
   // COMMON-SAME: (%opencl.queue_t{{.*}}* [[DEF_Q]], i32 [[FLAGS]], %struct.ndrange_t* [[NDR]]{{([0-9]+)?}},
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8* bitcast ({{.*}} [[INVGK6:[^ ]+_kernel]] to i8*) to i8 addrspace(4)*),
   // COMMON-SAME: i8 addrspace(4)* addrspacecast (i8 addrspace(1)* bitcast ({ i32, i32 } addrspace(1)* [[BLG6]] to i8 addrspace(1)*) to i8 addrspace(4)*), i32 3,
@@ -229,7 +229,7 @@

[PATCH] D46349: [X86] Mark builtins 'const' where possible

2018-05-08 Thread Eli Friedman via Phabricator via cfe-commits

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

I'm pretty sure this has zero visible effect, but I guess it makes sense as 
documentation.  LGTM.


https://reviews.llvm.org/D46349



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

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

2018-05-08 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: docs/LanguageExtensions.rst:1998
+``__ATOMIC_CONSUME``, ``__ATOMIC_ACQUIRE``, ``__ATOMIC_RELEASE``,
+``__ATOMIC_ACQ_REL``, or ``__ATOMIC_SEQ_CST`` following C++11 memory model 
semantics.
+

Thank you for adding this documentation.  Please do clarify what the memory 
ordering semantics actually are when the atomic object does not need to be 
updated, though, and verify that target code generation actually obeys that 
ordering.  For example, if the memory ordering makes this a release operation, 
`__atomic_fetch_min` must always store the result back to the atomic object, 
even if the new value was actually greater than the stored value; I believe 
that would not be required with a relaxed operation.


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[clang-tools-extra] r331805 - Fix Wdocumentation warning. NFCI.

2018-05-08 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Tue May  8 13:24:45 2018
New Revision: 331805

URL: http://llvm.org/viewvc/llvm-project?rev=331805=rev
Log:
Fix Wdocumentation warning. NFCI.

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.h

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=331805=331804=331805=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Tue May  8 13:24:45 2018
@@ -222,8 +222,8 @@ ClangTidyOptions::OptionMap getCheckOpti
 
 /// \brief Run a set of clang-tidy checks on a set of files.
 ///
-/// \param Profile if provided, it enables check profile collection in
-/// MatchFinder, and will contain the result of the profile.
+/// \param EnableCheckProfile If provided, it enables check profile collection
+/// in MatchFinder, and will contain the result of the profile.
 void runClangTidy(clang::tidy::ClangTidyContext ,
   const tooling::CompilationDatabase ,
   ArrayRef InputFiles,


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

[PATCH] D46471: [HIP] Add hip offload kind

2018-05-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:133-135
 Work(*C.getSingleOffloadToolChain());
 
+  if (JA.isHostOffloading(Action::OFK_HIP))

tra wrote:
> CUDA and HIP are mutually exclusive, so this should probably be `else if`
Will do when committing.


https://reviews.llvm.org/D46471



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

[PATCH] D46475: [HIP] Set proper triple and offload kind for the toolchain

2018-05-08 Thread John McCall via Phabricator via cfe-commits

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

Thanks, LGTM.


https://reviews.llvm.org/D46475



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

[PATCH] D46535: Correct warning on Float->Integer conversions.

2018-05-08 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In https://reviews.llvm.org/D46535#1091787, @efriedma wrote:

> The check for whether an input is "out of range" doesn't handle fractions 
> correctly.  Testcase:
>
>   int a() { return 2147483647.5; }
>   unsigned b() { return -.5; }
>

Hrm... For some reaosn I had it in my head that both of those were UB...  
Interestingly, EDG/ICC thinks the second one is.  I'll look into this more to 
see if I can refine the warning.

Repository:
  rC Clang

https://reviews.llvm.org/D46535

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

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

2018-05-08 Thread Reka Kovacs via Phabricator via cfe-commits

rnkovacs added inline comments.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2342
+BugReport ) {
+  if (isInvalidated)
+return nullptr;

george.karpenkov wrote:
> Is this field actually necessary? Do we ever check the same bug report with 
> the same visitor multiple times?
I believe this function is called for each node on the bug path. I have a 
similar field to indicate the first visited node in the new version, but there 
may exist a better solution for that as well.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2351
+
+  if (!RefutationMgr.checkRangedStateConstraints(Succ->getState())) {
+const LocationContext *LC = Succ->getLocationContext();

george.karpenkov wrote:
> For the initial version I would just do all work in the visitor, but that's a 
> matter of taste.
I think that doing all the work in the visitor would need exposing even more of 
`Z3ConstraintManager`'s internals as of `RangedConstraintManager`. I tried to 
keep such changes minimal.



Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:86
+  ? CreateZ3ConstraintManager(*this, SubEng)
+  : nullptr;
 }

george.karpenkov wrote:
> Would then we crash on NPE if `getRefutationManager` is called? Getters 
> should preferably not cause crashes.
Um, currently yes, it will give a backend error if clang isn't built with Z3, 
but the option is on.


https://reviews.llvm.org/D45517



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

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

2018-05-08 Thread Reka Kovacs via Phabricator via cfe-commits

rnkovacs updated this revision to Diff 145762.
rnkovacs marked 4 inline comments as done.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.

Expression chaining is fixed. The visitor now collects constraints that are 
about to disappear along the bug path and checks them once in the end.


https://reviews.llvm.org/D45517

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "RangedConstraintManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -915,6 +916,13 @@
   void print(ProgramStateRef St, raw_ostream , const char *nl,
  const char *sep) override;
 
+  void reset() override;
+
+  bool isModelFeasible() override;
+
+  void addRangeConstraints(ConstraintRangeTy PrevCR, ConstraintRangeTy SuccCR,
+   bool OnlyPurged) override;
+
   //===--===//
   // Implementation for interface from SimpleConstraintManager.
   //===--===//
@@ -1235,6 +1243,57 @@
   return State->set(CZ);
 }
 
+void Z3ConstraintManager::reset() { Solver.reset(); }
+
+bool Z3ConstraintManager::isModelFeasible() {
+  return Solver.check() != Z3_L_FALSE;
+}
+
+void Z3ConstraintManager::addRangeConstraints(ConstraintRangeTy PrevCR,
+  ConstraintRangeTy SuccCR,
+  bool OnlyPurged) {
+  if (OnlyPurged && PrevCR.isEmpty())
+return;
+  if (!OnlyPurged && SuccCR.isEmpty())
+return;
+  ConstraintRangeTy CR = OnlyPurged ? PrevCR : SuccCR;
+
+  for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
+SymbolRef Sym = I.getKey();
+
+if (OnlyPurged && SuccCR.contains(Sym))
+  continue;
+
+Z3Expr Constraints = Z3Expr::fromBoolean(false);
+
+for (const auto  : I.getData()) {
+  const llvm::APSInt  = Range.From();
+  const llvm::APSInt  = Range.To();
+
+  assert((getAPSIntType(From) == getAPSIntType(To)) &&
+ "Range values have different types!");
+  QualType RangeTy = getAPSIntType(From);
+  // Skip ranges whose endpoints cannot be converted to APSInts with
+  // a valid APSIntType.
+  if (RangeTy.isNull())
+continue;
+
+  QualType SymTy;
+  Z3Expr Exp = getZ3Expr(Sym, );
+  bool IsSignedTy = SymTy->isSignedIntegerOrEnumerationType();
+
+  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
+  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+
+  Z3Expr LHS = getZ3BinExpr(Exp, SymTy, BO_GE, FromExp, RangeTy, nullptr);
+  Z3Expr RHS = getZ3BinExpr(Exp, SymTy, BO_LE, ToExp, RangeTy, nullptr);
+  Z3Expr SymRange = Z3Expr::fromBinOp(LHS, BO_LAnd, RHS, IsSignedTy);
+  Constraints = Z3Expr::fromBinOp(Constraints, BO_LOr, SymRange, IsSignedTy);
+}
+Solver.addConstraint(Constraints);
+  }
+}
+
 //===--===//
 // Internal implementation.
 //===--===//
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -13,6 +13,8 @@
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
@@ -78,6 +80,10 @@
 CallEventMgr(new CallEventManager(alloc)), Alloc(alloc) {
   StoreMgr = (*CreateSMgr)(*this);
   ConstraintMgr = (*CreateCMgr)(*this, SubEng);
+  AnalyzerOptions  = SubEng->getAnalysisManager().getAnalyzerOptions();
+  RefutationMgr = Opts.shouldCrosscheckWithZ3()
+  ? CreateZ3ConstraintManager(*this, SubEng)
+

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a reviewer: rjmccall.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added a comment.

In https://reviews.llvm.org/D42933#1091502, @jfb wrote:

> In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D42933#1090268, @jfb wrote:
> >
> > > I was just looking at this, and I think @arphaman's patch is pretty much 
> > > the right approach (with 2 suggested fixes below).
> > >
> > > I don't think the code we're currently warning on is broken: a user code 
> > > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all 
> > > platforms which support those types the implementor has guaranteed that 
> > > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> > > sizeof(NSUInteger))`.
> >
> >
> > Yes, but is this guaranteed to be the case or does it happen to be the 
> > case? I'm worried about the less mainstream uses where you might find ObjC 
> > code where this does not hold but -Wformat points out a portability concern.
>
>
> For Darwin platform, yes. That's why this diagnostic should only be squelched 
> for Darwin platforms.

Ah, I missed the fact that you were proposing this only for Darwin.

 I agree that, if we're playing C++ pedant and look at the typedefs, then 
 it's undefined behavior and the code is broken.
>>> 
>>> This is reason alone to diagnose the code, because the optimizer is welcome 
>>> to use that UB to perform transformations the programmer did not expect. 
>>> However, I'm not certain our optimizer is currently paying attention to 
>>> that UB directly, so this may not be an immediate issue (but could be a 
>>> pitfall for the future) but I'm not certain about other optimizers for 
>>> which portability would still be a concern.
>> 
>> I would honestly find it a bit surprising (and scary) if the optimizer 
>> actually took advantage of UB in the case where the size and alignment of 
>> the specifier and the actual type matches.
> 
> Hear, hear! I'm happy to fix any such optimization out of their misguided 
> optimism :-)

People used to think the same thing about many other optimizations that had 
non-local effects, but we've come to learn that these are not uncommon and 
incredibly hard for users to track down when it happens. Some enterprising soul 
could lower the call with an undef on the wrongly-typed argument and then who 
knows what happens. However, it's also reasonable for us to define undefined 
behavior for a given platform and that sounds like exactly this case.

If we're looking at only a specific change for Darwin, I think it's reasonable 
to make it to `-Wformat` rather than require `-Wformat-relaxed` (though we may 
still want to assert that the size and alignment of the underlying type match 
expectations).

I've added @rjmccall as a reviewer to see what his opinions are on this, but 
I'm inclined to say this is a reasonable change to make for Darwin targets.

Repository:
  rC Clang

https://reviews.llvm.org/D42933

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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb added a comment.

In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:

> I also think that special casing these two specifiers doesn't make sense. The 
> problem is a general issue -- and one I've often found irritating. This exact 
> same situation comes up all the time in non-Darwin contexts too.

I don't think that's true. In this very specific case the platform guarantees 
that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
sizeof(NSUInteger))` for all architectures this platform supports. This exact 
same situation does not come up all the time in other contexts because the 
`int` / `long` / `long long` distinction isn't backed by a portability 
guarantee. The warning is there to say "this code isn't portable!", but in the 
very specific case of `NSInteger` and `NSUInteger` it is and users rely on it 
so it cannot be broken. The warning is therefore spurious, users therefore 
rightly complain.

Repository:
  rC Clang

https://reviews.llvm.org/D42933

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

[PATCH] D46349: [X86] Mark builtins 'const' where possible

2018-05-08 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

Ping


https://reviews.llvm.org/D46349



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread James Y Knight via Phabricator via cfe-commits

jyknight added a comment.

In https://reviews.llvm.org/D42933#1090384, @jfb wrote:

> In https://reviews.llvm.org/D42933#1090286, @smeenai wrote:
>
> > I'd be fine with adding an option to relax the printf checking if the size 
> > and alignment of the specifier and the actual type match, even if the types 
> > themselves differ (`-Wformat-relaxed` or something similar), so that you'd 
> > still get warnings on cases where the specifier mismatch could cause 
> > runtime issues.
>
>
> What are the cases that you're worried about? The only ones I'm trying to 
> capture here are `NSInteger` with `%zd` and `NSUInteger` with `%zu`, are 
> there others?
>
> > I think that would be preferable to special-casing the Apple types.
>
> If there are more that should be captured and a similar point solution 
> doesn't apply, agreed. However I'd like to understand if we agree on the 
> guarantees that the platform offers for the two specific cases I'm targeting.

I also think that special casing these two specifiers doesn't make sense. The 
problem is a general issue -- and one I've often found irritating. This exact 
same situation comes up all the time in non-Darwin contexts too.

E.g. one I find particularly annoying is "%lld" vs "%ld" in printf.

Some 64-bit platforms define e.g. `int64_t` as `long long`, and others as 
`long`. Although both types are size 8, align 8, and mean exactly the same 
thing, they are still distinct. And so, users write code like `int64_t x = 0; 
printf("%ld", x);`...which then emits warnings on the platform that defines 
int64_t as a `long long`. Obviously, the code _could_ be using `PRId64` 
instead...but it often doesn't. So it'd sure be nice if you could restrict the 
warning to only warn about actual problems, and suppress these 
superficially-incompatible-but-not-really warnings.

So, +1 from me for the general-purpose `-Wformat-relaxed` flag (or whatever 
other flag name is appropriate).

Repository:
  rC Clang

https://reviews.llvm.org/D42933

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

[PATCH] D46535: Correct warning on Float->Integer conversions.

2018-05-08 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

The check for whether an input is "out of range" doesn't handle fractions 
correctly.  Testcase:

  int a() { return 2147483647.5; }
  unsigned b() { return -.5; }


Repository:
  rC Clang

https://reviews.llvm.org/D46535



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

[PATCH] D46471: [HIP] Add hip offload kind

2018-05-08 Thread Artem Belevich via Phabricator via cfe-commits

tra accepted this revision.
tra added a comment.

Small nit. LGTM otherwise.




Comment at: lib/Driver/ToolChains/Clang.cpp:133-135
 Work(*C.getSingleOffloadToolChain());
 
+  if (JA.isHostOffloading(Action::OFK_HIP))

CUDA and HIP are mutually exclusive, so this should probably be `else if`


https://reviews.llvm.org/D46471



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

[PATCH] D46593: Allow copy elision in path concatenation

2018-05-08 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

In https://reviews.llvm.org/D46593#1091732, @smeenai wrote:

> Generating the patch from libc++ is fine (and this patch looks like it has 
> sane paths).


Thank you


https://reviews.llvm.org/D46593



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

[PATCH] D46593: Allow copy elision in path concatenation

2018-05-08 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

Generating the patch from libc++ is fine (and this patch looks like it has sane 
paths).


https://reviews.llvm.org/D46593



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

[PATCH] D46593: Allow copy elision in path concatenation

2018-05-08 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 updated this revision to Diff 145745.
xbolva00 added a comment.

Can anybody give me advice from where to generate patch? I do it from libcxx.


https://reviews.llvm.org/D46593

Files:
  include/experimental/filesystem


Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
 
 inline _LIBCPP_INLINE_VISIBILITY
 path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
 }
 
 template 


Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
 
 inline _LIBCPP_INLINE_VISIBILITY
 path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
 }
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46593: Allow copy elision in path concatenation

2018-05-08 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 updated this revision to Diff 145742.
Herald added a subscriber: christof.

https://reviews.llvm.org/D46593

Files:
  libcxx/trunk/include/experimental/filesystem


Index: libcxx/trunk/include/experimental/filesystem
===
--- libcxx/trunk/include/experimental/filesystem
+++ libcxx/trunk/include/experimental/filesystem
@@ -1140,7 +1140,9 @@
 
 inline _LIBCPP_INLINE_VISIBILITY
 path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
 }
 
 template 


Index: libcxx/trunk/include/experimental/filesystem
===
--- libcxx/trunk/include/experimental/filesystem
+++ libcxx/trunk/include/experimental/filesystem
@@ -1140,7 +1140,9 @@
 
 inline _LIBCPP_INLINE_VISIBILITY
 path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
 }
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46593: Allow copy elision in path concatenation

2018-05-08 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 updated this revision to Diff 145740.
Herald added a subscriber: cfe-commits.

Repository:
  rCXX libc++

https://reviews.llvm.org/D46593

Files:
  include/experimental/filesystem


Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
 
 inline _LIBCPP_INLINE_VISIBILITY
 path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
 }
 
 template 


Index: include/experimental/filesystem
===
--- include/experimental/filesystem
+++ include/experimental/filesystem
@@ -1140,7 +1140,9 @@
 
 inline _LIBCPP_INLINE_VISIBILITY
 path operator/(const path& __lhs, const path& __rhs) {
-return path(__lhs) /= __rhs;
+path __result(__lhs);
+__result /= __rhs;
+return __result;
 }
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331786 - [lit] Fix running tests that require 'examples'.

2018-05-08 Thread Zachary Turner via cfe-commits

Author: zturner
Date: Tue May  8 11:20:10 2018
New Revision: 331786

URL: http://llvm.org/viewvc/llvm-project?rev=331786=rev
Log:
[lit] Fix running tests that require 'examples'.

Differential Revision: https://reviews.llvm.org/D46514
Patch by Nikolai Kosjar.

Modified:
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/test/lit.cfg.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=331786=331785=331786=diff
==
--- cfe/trunk/test/lit.cfg.py (original)
+++ cfe/trunk/test/lit.cfg.py Tue May  8 11:20:10 2018
@@ -63,6 +63,7 @@ tools = [
 ]
 
 if config.clang_examples:
+config.available_features.add('examples')
 tools.append('clang-interpreter')
 
 llvm_config.add_tool_substitutions(tools, tool_dirs)


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

[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

2018-05-08 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff added a comment.

> Hmm. In C++, a static object which isn't constant-initialized is 
> zero-initialized, which is required to set any padding bits to zero (N4640 
> [dcl.init]p6) in both structs and unions. In C, a static object which doesn't 
> have an initializer also has all any padding bits set to zero (N1548 
> 6.7.9p10). Now, this object has an initializer (that acts as a 
> constant-initializer in C++), so those rules don't directly apply; but I 
> would argue that the intent of the standards is not that padding bits become 
> undefined when you happen to have an initializer. So I actually think the 
> zeroinitializer emission is more correct.

Using undefined values instead of zero initialization was implemented in 
https://reviews.llvm.org/rL101535. There is no much info about the reason of 
the implementation. Clang uses undefined values for padding bits, in particular 
in unions, when the first member is not widest. The code:

  union C1 {
  char sel;
  double dval;
};
union C1 val_1 = { 0 };

produces:

  @val_1 = dso_local global { i8, [7 x i8] } { i8 0, [7 x i8] undef }, align 8  

Another case is unnamed bit fields.

struct C2 {
  int : 4;
  int x;
};
struct C2 val_2 = { 0 };

produces:

  @val_2 = dso_local global { [4 x i8], i32 } { [4 x i8] undef, i32 0 }, align 4

Strictly speaking, this IR does not mean violation of the standard, but it can 
modify code generation in some cases. If we decided to use zeroinitializer in 
this case, we probably would need to revise using undefined values in 
initializers, otherwise similar declarations like:

union C1 val_1a = { 0 };
union C1 val_1b = { 1 };

would produce different IR representations, with and without undefined values.

The test `SemaCXX/large-array-init.cpp` is removed in this change. This test 
was added in https://reviews.llvm.org/rL325120 to solve 
https://bugs.llvm.org/show_bug.cgi?id=18978, which describes the same problem, 
as solved by this patch. This patch presents more efficient solution, with it 
the tests compiles 50 time faster. If r325120 does not solve additional 
problems, it can be reverted.


Repository:
  rC Clang

https://reviews.llvm.org/D46241



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

[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

2018-05-08 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff updated this revision to Diff 145735.
sepavloff added a comment.

Added treatment of structures/unions


Repository:
  rC Clang

https://reviews.llvm.org/D46241

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGen/const-init.c
  test/CodeGen/designated-initializers.c
  test/CodeGen/union-init2.c
  test/CodeGenCXX/cxx11-initializer-aggregate.cpp
  test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
  test/SemaCXX/large-array-init.cpp

Index: test/SemaCXX/large-array-init.cpp
===
--- test/SemaCXX/large-array-init.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -S -o %t.ll -mllvm -debug-only=exprconstant %s 2>&1 | \
-// RUN: FileCheck %s
-// REQUIRES: asserts
-
-struct S { int i; };
-
-static struct S arr[1] = {{ 0 }};
-// CHECK: The number of elements to initialize: 1.
-
-struct S *foo() { return arr; }
Index: test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
@@ -17,14 +17,14 @@
 
   C c1 = {};
   C c2 = {1};
-  // CHECK: @_ZN8Constant2c1E = global { i8 } zeroinitializer, align 1
+  // CHECK: @_ZN8Constant2c1E = global %"struct.Constant::C" zeroinitializer, align 1
   // CHECK: @_ZN8Constant2c2E = global { i8 } { i8 1 }, align 1
 
   // Test packing bases into tail padding.
   D d1 = {};
   D d2 = {1, 2, 3};
   D d3 = {1};
-  // CHECK: @_ZN8Constant2d1E = global { i32, i8, i8 } zeroinitializer, align 4
+  // CHECK: @_ZN8Constant2d1E = global %"struct.Constant::D" zeroinitializer, align 4
   // CHECK: @_ZN8Constant2d2E = global { i32, i8, i8 } { i32 1, i8 2, i8 3 }, align 4
   // CHECK: @_ZN8Constant2d3E = global { i32, i8, i8 } { i32 1, i8 0, i8 0 }, align 4
 
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,30 @@
   // meaningful.
   B b[30] = {};
 }
+
+namespace ZeroInit {
+  enum { Zero, One };
+  constexpr int zero() { return 0; }
+  constexpr int *null() { return nullptr; }
+  struct Filler {
+int x;
+Filler();
+  };
+  struct S1 {
+int x;
+  };
+
+  // These declarations, if implemented elementwise, require huge
+  // amout of memory and compiler time.
+  unsigned char data_1[1024 * 1024 * 1024 * 2u] = { 0 };
+  unsigned char data_2[1024 * 1024 * 1024 * 2u] = { Zero };
+  unsigned char data_3[1024][1024][1024] = {{{0}}};
+  unsigned char data_4[1024 * 1024 * 1024 * 2u] = { zero() };
+  int *data_5[1024 * 1024 * 512] = { nullptr };
+  int *data_6[1024 * 1024 * 512] = { null() };
+  struct S1 data_7[1024 * 1024 * 512] = {{0}};
+
+  // This variable must be initialized elementwise.
+  Filler data_e1[1024] = {};
+  // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: test/CodeGen/union-init2.c
===
--- test/CodeGen/union-init2.c
+++ test/CodeGen/union-init2.c
@@ -5,7 +5,7 @@
 union x {long long b;union x* a;} r = {.a = };
 
 
-// CHECK: global { [3 x i8], [5 x i8] } { [3 x i8] zeroinitializer, [5 x i8] undef }
+// CHECK: global %union.z zeroinitializer
 union z {
   char a[3];
   long long b;
Index: test/CodeGen/designated-initializers.c
===
--- test/CodeGen/designated-initializers.c
+++ test/CodeGen/designated-initializers.c
@@ -8,7 +8,7 @@
 // CHECK: @u = global %union.anon zeroinitializer
 union { int i; float f; } u = { };
 
-// CHECK: @u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }
+// CHECK: @u2 = global %union.anon.0 zeroinitializer
 union { int i; double f; } u2 = { };
 
 // CHECK: @u3 = global  %union.anon.1 zeroinitializer
Index: test/CodeGen/const-init.c
===
--- test/CodeGen/const-init.c
+++ test/CodeGen/const-init.c
@@ -167,7 +167,7 @@
 int : 1;
 int x;
   } a = {};
-  // CHECK: @g30.a = internal global %struct.anon.1 <{ i8 undef, i32 0 }>, align 1
+  // CHECK: @g30.a = internal global %struct.anon.1 zeroinitializer, align 1
 #pragma pack()
 }
 
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,37 @@
   return type;
 }
 
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+  if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+  }
+
+  if (auto *IL = dyn_cast_or_null(Init)) {
+for (auto I : IL->inits())
+  if

[PATCH] D46540: [X86] ptwrite intrinsic

2018-05-08 Thread Jonas Hahnfeld via Phabricator via cfe-commits

Hahnfeld added a comment.

Could you maybe add some short summaries to your patches? It's hard for 
non-Intel employees to guess what all these instructions do...


https://reviews.llvm.org/D46540



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

[PATCH] D46320: Remove \brief commands from doxygen comments.

2018-05-08 Thread Reid Kleckner via Phabricator via cfe-commits

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

lgtm! (Back from a week long vacation)


https://reviews.llvm.org/D46320



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

[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes

2018-05-08 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:2342
   // Only positive features are "required".
-  if (F.getValue())
+  if (F.getValue()) {
+if (std::any_of(ParsedAttr.Features.begin(), ParsedAttr.Features.end(),

Rather than walking the ParsedAttr.Features for each feature in the map. And 
having to shift the ReqFeatures vectors sometimes. How about doing this

-Walk through all features in ParsedAttr, for each feature with a +, query the 
callee feature map. If it's enabled there, push it to ReqFeatures.
-Walk through all features in the callee feature map and if enabled push those.

This will lead to duplicates in the list, but all the explicitly mentioned 
features will be listed first.


https://reviews.llvm.org/D46541



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

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-05-08 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

Great! Let's close this review then.
And good luck with cling.


https://reviews.llvm.org/D44435



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread JF Bastien via Phabricator via cfe-commits

jfb added a comment.

In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote:

> In https://reviews.llvm.org/D42933#1090268, @jfb wrote:
>
> > I was just looking at this, and I think @arphaman's patch is pretty much 
> > the right approach (with 2 suggested fixes below).
> >
> > I don't think the code we're currently warning on is broken: a user code 
> > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all platforms 
> > which support those types the implementor has guaranteed that 
> > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> > sizeof(NSUInteger))`.
>
>
> Yes, but is this guaranteed to be the case or does it happen to be the case? 
> I'm worried about the less mainstream uses where you might find ObjC code 
> where this does not hold but -Wformat points out a portability concern.


For Darwin platform, yes. That's why this diagnostic should only be squelched 
for Darwin platforms.

>> However the implementation provided more guarantees than C++ does through 
>> its platform typedefs so pendantry need not apply (or rather: clang should 
>> look no further than the typedef, and trust the platform in this particular 
>> case).
>> 
>> We of course should still warn on sign mismatch.
>> 
>> I don't think this should even be a warning with pedantic on: there's no 
>> portability issue at all because all OSes and architectures where this could 
>> ever fire are guaranteed to do the right thing.
> 
> Can you point me to this guarantee, as I was unable to find one (but that 
> doesn't mean it doesn't exist)?

Once this patch is committed I'll update the corresponding documentation on 
developer.apple.com

In https://reviews.llvm.org/D42933#1091411, @smeenai wrote:

> In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D42933#1090268, @jfb wrote:
> >
> > > I was just looking at this, and I think @arphaman's patch is pretty much 
> > > the right approach (with 2 suggested fixes below).
> > >
> > > I don't think the code we're currently warning on is broken: a user code 
> > > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all 
> > > platforms which support those types the implementor has guaranteed that 
> > > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> > > sizeof(NSUInteger))`.
> >
> >
> > Yes, but is this guaranteed to be the case or does it happen to be the 
> > case? I'm worried about the less mainstream uses where you might find ObjC 
> > code where this does not hold but -Wformat points out a portability concern.
>
>
> Also keep in mind that Obj-C might be used on non-Apple platforms, e.g. 
> there's an active review going on for GNUstep ABI v2 right now 
> (https://reviews.llvm.org/D46052), and WinObjC is also a thing. Those 
> platforms might not necessarily provide the same guarantees or consistency 
> that Apple's do.


Indeed, I want to make sure that this change only affects Darwin platforms.

>>> I agree that, if we're playing C++ pedant and look at the typedefs, then 
>>> it's undefined behavior and the code is broken.
>> 
>> This is reason alone to diagnose the code, because the optimizer is welcome 
>> to use that UB to perform transformations the programmer did not expect. 
>> However, I'm not certain our optimizer is currently paying attention to that 
>> UB directly, so this may not be an immediate issue (but could be a pitfall 
>> for the future) but I'm not certain about other optimizers for which 
>> portability would still be a concern.
> 
> I would honestly find it a bit surprising (and scary) if the optimizer 
> actually took advantage of UB in the case where the size and alignment of the 
> specifier and the actual type matches.

Hear, hear! I'm happy to fix any such optimization out of their misguided 
optimism :-)


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

2018-05-08 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny updated this revision to Diff 145711.
jdenny edited the summary of this revision.
jdenny added a comment.

Made the suggested changes.


https://reviews.llvm.org/D45093

Files:
  include/clang/Sema/Sema.h
  lib/Frontend/ASTConsumers.cpp
  lib/Sema/Sema.cpp
  test/Misc/ast-print-bool.c

Index: test/Misc/ast-print-bool.c
===
--- /dev/null
+++ test/Misc/ast-print-bool.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+
+#if DEF_BOOL_CBOOL
+# define bool _Bool
+#elif DEF_BOOL_INT
+# define bool int
+#endif
+
+// BOOL-AS-CBOOL: _Bool i;
+// BOOL-AS-INT:   int i;
+// BOOL-AS-BOOL:  bool i;
+bool i;
+
+#ifndef __cplusplus
+// CBOOL: _Bool j;
+_Bool j;
+#endif
+
+// Induce a diagnostic (and verify we actually managed to do so), which used to
+// permanently alter the -ast-print printing policy for _Bool.  How bool is
+// defined by the preprocessor is examined only once per compilation, when the
+// diagnostic is emitted, and it used to affect the entirety of -ast-print, so
+// test only one definition of bool per compilation.
+#if DIAG
+void fn() { 1; } // expected-warning {{expression result unused}}
+#else
+// expected-no-diagnostics
+#endif
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -52,8 +52,8 @@
 PrintingPolicy Sema::getPrintingPolicy(const ASTContext ,
const Preprocessor ) {
   PrintingPolicy Policy = Context.getPrintingPolicy();
-  // Our printing policy is copied over the ASTContext printing policy whenever
-  // a diagnostic is emitted, so recompute it.
+  // In diagnostics, we print _Bool as bool if the latter is defined as the
+  // former.
   Policy.Bool = Context.getLangOpts().Bool;
   if (!Policy.Bool) {
 if (const MacroInfo *BoolMacro = PP.getMacroInfo(Context.getBoolName())) {
@@ -1284,7 +1284,8 @@
 }
   }
 
-  // Set up the context's printing policy based on our current state.
+  // Copy the diagnostic printing policy over the ASTContext printing policy.
+  // TODO: Stop doing that.  See: https://reviews.llvm.org/D45093#1090292
   Context.setPrintingPolicy(getPrintingPolicy());
 
   // Emit the diagnostic.
Index: lib/Frontend/ASTConsumers.cpp
===
--- lib/Frontend/ASTConsumers.cpp
+++ lib/Frontend/ASTConsumers.cpp
@@ -87,9 +87,10 @@
 << DC->getPrimaryContext() << "\n";
 } else
   Out << "Not a DeclContext\n";
-  } else if (OutputKind == Print)
-D->print(Out, /*Indentation=*/0, /*PrintInstantiation=*/true);
-  else if (OutputKind != None)
+  } else if (OutputKind == Print) {
+PrintingPolicy Policy(D->getASTContext().getLangOpts());
+D->print(Out, Policy, /*Indentation=*/0, /*PrintInstantiation=*/true);
+  } else if (OutputKind != None)
 D->dump(Out, OutputKind == DumpFull);
 }
 
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2111,12 +2111,12 @@
   void checkPartialSpecializationVisibility(SourceLocation Loc,
 NamedDecl *Spec);
 
-  /// \brief Retrieve a suitable printing policy.
+  /// \brief Retrieve a suitable printing policy for diagnostics.
   PrintingPolicy getPrintingPolicy() const {
 return getPrintingPolicy(Context, PP);
   }
 
-  /// \brief Retrieve a suitable printing policy.
+  /// \brief Retrieve a suitable printing policy for diagnostics.
   static PrintingPolicy getPrintingPolicy(const ASTContext ,
   const Preprocessor );
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46475: [HIP] Set proper triple and offload kind for the toolchain

2018-05-08 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 145699.
yaxunl marked an inline comment as done.
yaxunl added a comment.

Revised by John's comments.


https://reviews.llvm.org/D46475

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Driver/Types.h
  lib/Driver/Driver.cpp
  lib/Driver/Types.cpp
  test/Driver/Inputs/hip_multiple_inputs/a.cu
  test/Driver/Inputs/hip_multiple_inputs/b.hip
  test/Driver/hip-inputs.hip

Index: test/Driver/hip-inputs.hip
===
--- /dev/null
+++ test/Driver/hip-inputs.hip
@@ -0,0 +1,23 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -ccc-print-phases -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 -c \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip 2>&1 \
+// RUN: | FileCheck %s
+
+// RUN: not %clang -ccc-print-phases -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 -c \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip 2>&1 \
+// RUN: | FileCheck -check-prefix=MIX %s
+
+// RUN: not %clang -ccc-print-phases -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 -c \
+// RUN:   --hip-link %S/Inputs/hip_multiple_inputs/a.cu 2>&1 \
+// RUN: | FileCheck -check-prefix=MIX %s
+
+// CHECK-NOT: error: Mixed Cuda and HIP compilation is not supported.
+// MIX: error: Mixed Cuda and HIP compilation is not supported.
Index: lib/Driver/Types.cpp
===
--- lib/Driver/Types.cpp
+++ lib/Driver/Types.cpp
@@ -172,6 +172,15 @@
   case TY_CUDA:
   case TY_PP_CUDA:
   case TY_CUDA_DEVICE:
+return true;
+  }
+}
+
+bool types::isHIP(ID Id) {
+  switch (Id) {
+  default:
+return false;
+
   case TY_HIP:
   case TY_PP_HIP:
   case TY_HIP_DEVICE:
@@ -230,6 +239,7 @@
.Case("fpp", TY_Fortran)
.Case("FPP", TY_Fortran)
.Case("gch", TY_PCH)
+   .Case("hip", TY_HIP)
.Case("hpp", TY_CXXHeader)
.Case("iim", TY_PP_CXXModule)
.Case("lib", TY_Object)
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -538,24 +538,46 @@
   InputList ) {
 
   //
-  // CUDA
+  // CUDA/HIP
   //
-  // We need to generate a CUDA toolchain if any of the inputs has a CUDA type.
-  if (llvm::any_of(Inputs, [](std::pair ) {
+  // We need to generate a CUDA toolchain if any of the inputs has a CUDA
+  // or HIP type. However, mixed CUDA/HIP compilation is not supported.
+  bool IsCuda =
+  llvm::any_of(Inputs, [](std::pair ) {
 return types::isCuda(I.first);
-  })) {
+  });
+  bool IsHIP =
+  llvm::any_of(Inputs,
+   [](std::pair ) {
+ return types::isHIP(I.first);
+   }) ||
+  C.getInputArgs().hasArg(options::OPT_hip_link);
+  if (IsCuda && IsHIP) {
+Diag(clang::diag::err_drv_mix_cuda_hip);
+return;
+  }
+  if (IsCuda || IsHIP) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
-llvm::Triple CudaTriple(HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
- : "nvptx-nvidia-cuda");
-// Use the CUDA and host triples as the key into the ToolChains map, because
-// the device toolchain we create depends on both.
+StringRef DeviceTripleStr;
+auto OFK = IsHIP ? Action::OFK_HIP : Action::OFK_Cuda;
+if (IsHIP) {
+  // HIP is only supported on amdgcn.
+  DeviceTripleStr = "amdgcn-amd-amdhsa";
+} else {
+  // CUDA is only supported on nvptx.
+  DeviceTripleStr = HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
+ : "nvptx-nvidia-cuda";
+}
+llvm::Triple CudaTriple(DeviceTripleStr);
+// Use the CUDA/HIP and host triples as the key into the ToolChains map,
+// because the device toolchain we create depends on both.
 auto  = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
 if (!CudaTC) {
   CudaTC = llvm::make_unique(
-  *this, CudaTriple, *HostTC, C.getInputArgs(), Action::OFK_Cuda);
+  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
 }
-C.addOffloadDeviceToolChain(CudaTC.get(), Action::OFK_Cuda);
+C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   }
 
   //
Index: include/clang/Driver/Types.h
===
--- include/clang/Driver/Types.h
+++ include/clang/Driver/Types.h
@@ -77,6 +77,9 @@
   /// isCuda - Is this a CUDA input.
   bool isCuda(ID Id);
 
+

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote:

> In https://reviews.llvm.org/D42933#1090268, @jfb wrote:
>
> > I was just looking at this, and I think @arphaman's patch is pretty much 
> > the right approach (with 2 suggested fixes below).
> >
> > I don't think the code we're currently warning on is broken: a user code 
> > has `NSInteger` with `%zd` or `NSUInteger` with `%zu`, and on all platforms 
> > which support those types the implementor has guaranteed that 
> > `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> > sizeof(NSUInteger))`.
>
>
> Yes, but is this guaranteed to be the case or does it happen to be the case? 
> I'm worried about the less mainstream uses where you might find ObjC code 
> where this does not hold but -Wformat points out a portability concern.


Also keep in mind that Obj-C might be used on non-Apple platforms, e.g. there's 
an active review going on for GNUstep ABI v2 right now 
(https://reviews.llvm.org/D46052), and WinObjC is also a thing. Those platforms 
might not necessarily provide the same guarantees or consistency that Apple's 
do.

>> I agree that, if we're playing C++ pedant and look at the typedefs, then 
>> it's undefined behavior and the code is broken.
> 
> This is reason alone to diagnose the code, because the optimizer is welcome 
> to use that UB to perform transformations the programmer did not expect. 
> However, I'm not certain our optimizer is currently paying attention to that 
> UB directly, so this may not be an immediate issue (but could be a pitfall 
> for the future) but I'm not certain about other optimizers for which 
> portability would still be a concern.

I would honestly find it a bit surprising (and scary) if the optimizer actually 
took advantage of UB in the case where the size and alignment of the specifier 
and the actual type matches.


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes

2018-05-08 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: lib/CodeGen/CodeGenFunction.cpp:2346
+  return Feat.substr(1) == F.getKey();
+   }))
+ReqFeatures.insert(ReqFeatures.begin(), F.getKey());

This and the next line are indented funny.


https://reviews.llvm.org/D46541



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

[PATCH] D46540: [X86] ptwrite intrinsic

2018-05-08 Thread Craig Topper via Phabricator via cfe-commits

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

LGTM


https://reviews.llvm.org/D46540



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

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

2018-05-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: include/clang-c/Index.h:5237
+/**
+ * \brief FixIts that *must* be applied before inserting the text for the
+ * corresponding completion item. Completion items with non-empty fixits will

This seems too large for a brief comment :-) 
Maybe break with a newline after the first sentence.



Comment at: include/clang/Sema/CodeCompleteConsumer.h:564
+
+  /// \brief For this completion result correction is required.
+  std::vector Corrections;

yvvan wrote:
> yvvan wrote:
> > ilya-biryukov wrote:
> > > Adding some docs here would be useful. These fix-its could be interpreted 
> > > too broadly at this point.
> > > I suggest the following semantics and the comment:
> > > 
> > > ```
> > > /// \brief FixIts that *must* be applied before inserting the text for 
> > > the corresponding completion item.
> > > /// Completion items with non-empty fixits will not be returned by 
> > > default, they should be explicitly requested by setting 
> > > CompletionOptions::IncludeCorrections.
> > > /// For the editors to be able to compute position of the cursor for the 
> > > completion item itself, the following conditions are guaranteed to hold 
> > > for RemoveRange of the stored fixits:
> > > ///  - Ranges in the fixits are guaranteed to never contain the 
> > > completion point (or identifier under completion point, if any) inside 
> > > them, except at the start or at the end of the range.
> > > ///  - If a fixit range starts or ends with completion point (or starts 
> > > or ends after the identifier under completion point), it will contain at 
> > > least one character. It allows to unambiguously recompute completion 
> > > point after applying the fixit.
> > > /// The intuition is that provided fixits change code around the 
> > > identifier we complete, but are not allowed to touch the identifier 
> > > itself or the completion point.
> > > /// One example of completion items with corrections are the ones 
> > > replacing '.' with '->' and vice versa:
> > > ///  std::unique_ptr vec_ptr;
> > > ///  vec_ptr.^  // completion returns an item 'push_back', replacing 
> > > '.' with '->'
> > > ///  vec_ptr->^ // completion returns an item 'release', replacing 
> > > '->' with '.'let's put 
> > > ```
> > > 
> > > Do those invariants sound reasonable? Could we add asserts that they hold 
> > > when constructing the completion results?
> > Thanks! I usually feel the lack of patience when writing descriptions :)
> "Could we add asserts that they hold when constructing the completion results"
> 
> The current way of creating them actually assures that by default. And to 
> check it once again it's required to change many things.
> Starting with the fact that there is no SourceRange contains() methos or 
> something similar,
> The current way of creating them actually assures that by default. And to 
> check it once again it's required to change many things.
It's still nice to have a sanity check there, mostly for the sake of new checks 
that are gonna get added.
But even the current one is tricky, since it may actually contain a range that 
ends exactly at the cursor (when completing right after the dot/arrow `foo->|`).

> Starting with the fact that there is no SourceRange contains() methos or 
> something similar,
Moreover, `SourceRange` doesn't have clear semantics AFAIK. It can either be a 
half-open or closed range.
Let's add a FIXME, at least, that we should add those asserts later.




Comment at: include/clang/Sema/CodeCompleteConsumer.h:564
+
+  /// \brief FixIts that *must* be applied before inserting the text for the
+  /// corresponding completion item. Completion items with non-empty fixits 
will

I suggest moving this doc comment to `getFixits` method, as it is the public 
interface.



Comment at: include/clang/Sema/CodeCompleteConsumer.h:592
+   StringRef ParentName, const char *BriefComment,
+   std::vector FixIts);
   ~CodeCompletionString() = default;

We can't store fixits in `CodeCompletionString`, it should not contain source 
locatins, which are only valid for the lifetime of SourceManager.
Note that CCS has a lifetime independent of both the SourceManager and the AST.
Storing them in CodeCompletionResult should be good enough for all our 
use-cases.



Comment at: include/clang/Sema/CodeCompleteConsumer.h:814
 
+  /// \brief FixIts that *must* be applied before inserting the text for the
+  /// corresponding completion item. Completion items with non-empty fixits 
will

We shouldn't duplicate such a large comment in too many places, it would be 
impossible to keep it in sync.
I would suggest only keeping it in `CodeCompletionResult` and add a reference 
to it in other places.
libclang is a bit tricky, though. It is distributed

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-05-08 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov updated this revision to Diff 145691.
ilya-biryukov added a comment.

- Fixed infinite loop with comments that contain doxygen commands


Repository:
  rC Clang

https://reviews.llvm.org/D46000

Files:
  include/clang/AST/CommentLexer.h
  include/clang/AST/RawCommentList.h
  lib/AST/CommentLexer.cpp
  lib/AST/RawCommentList.cpp
  unittests/AST/CMakeLists.txt
  unittests/AST/CommentTextTest.cpp

Index: unittests/AST/CommentTextTest.cpp
===
--- /dev/null
+++ unittests/AST/CommentTextTest.cpp
@@ -0,0 +1,128 @@
+//===- unittest/AST/CommentTextTest.cpp - Comment text extraction test ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Tests for user-friendly output formatting of comments, i.e.
+// RawComment::getFormattedText().
+//
+//===--===//
+
+#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/CommentOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include 
+
+namespace clang {
+
+class CommentTextTest : public ::testing::Test {
+protected:
+  std::string formatComment(llvm::StringRef CommentText) {
+llvm::IntrusiveRefCntPtr EmptyFS(
+new vfs::InMemoryFileSystem);
+FileSystemOptions Opts;
+FileManager FileMgr(Opts, EmptyFS);
+
+DiagnosticsEngine Diags(new DiagnosticIDs, new DiagnosticOptions);
+SourceManager SourceMgr(Diags, FileMgr);
+
+auto File = SourceMgr.createFileID(
+llvm::MemoryBuffer::getMemBuffer(CommentText, "test-comment"));
+
+auto CommentStartOffset = CommentText.find("/");
+assert(CommentStartOffset != llvm::StringRef::npos);
+
+SourceRange CommentRange(
+SourceMgr.getLocForStartOfFile(File).getLocWithOffset(
+CommentStartOffset),
+SourceMgr.getLocForEndOfFile(File));
+CommentOptions EmptyOpts;
+// FIXME: technically, merged that we set here is incorrect, but that
+// shouldn't matter.
+RawComment Comment(SourceMgr, CommentRange, EmptyOpts, /*Merged=*/true);
+return Comment.getFormattedText(SourceMgr, Diags);
+  }
+};
+
+TEST_F(CommentTextTest, FormattedText) {
+  // clang-format off
+  auto ExpectedOutput =
+R"(This function does this and that.
+For example,
+   Runnning it in that case will give you
+   this result.
+That's about it.)";
+  // Two-slash comments.
+  EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+// This function does this and that.
+// For example,
+//Runnning it in that case will give you
+//this result.
+// That's about it.)cpp"));
+
+  // Three-slash comments.
+  EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/// This function does this and that.
+/// For example,
+///Runnning it in that case will give you
+///this result.
+/// That's about it.)cpp"));
+
+  // Block comments.
+  EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/* This function does this and that.
+ * For example,
+ *Runnning it in that case will give you
+ *this result.
+ * That's about it.*/)cpp"));
+
+  // Doxygen-style block comments.
+  EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/** This function does this and that.
+  * For example,
+  *Runnning it in that case will give you
+  *this result.
+  * That's about it.*/)cpp"));
+
+  // Weird indentation.
+  EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+   // This function does this and that.
+  //  For example,
+  // Runnning it in that case will give you
+//   this result.
+   // That's about it.)cpp"));
+  // clang-format on
+}
+
+TEST_F(CommentTextTest, KeepsDoxygenControlSeqs) {
+  // clang-format off
+  auto ExpectedOutput =
+R"(\brief This is the brief part of the comment.
+\param a something about a.
+@param b something about b.)";
+
+  EXPECT_EQ(ExpectedOutput, formatComment(
+R"cpp(
+/// \brief This is the brief part of the comment.
+/// \param a something about a.
+/// @param b something about b.)cpp"));
+  // clang-format on
+}
+
+} // namespace clang
Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -9,6 +9,7 @@
   ASTVectorTest.cpp
   CommentLexer.cpp
   CommentParser.cpp
+  CommentTextTest.cpp
   DataCollectionTest.cpp
   DeclPrinterTest.cpp
   DeclTest.cpp
Index: lib/AST/RawCommentList.cpp
===
---

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

2018-05-08 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/FuchsiaTidyModule.cpp:41
+CheckFactories.registerCheck(
+"fuchsia-restrict-includes");
 CheckFactories.registerCheck(

I think this should be named `fuchsia-restrict-system-includes`.



Comment at: clang-tidy/fuchsia/RestrictIncludesCheck.cpp:60
+StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (!llvm::is_contained(Check.getAllowedIncludes(), FileName) && IsAngled)
+// Bucket the allowed include directives by the id of the file they were

I'm not certain that `IsAngled` is sufficient. For instance, say the user 
prohibits use of `vector`, nothing prevents the user from writing `#include 
"vector"` to access the system header (assuming there is no user header with 
the same name). e.g., https://godbolt.org/g/Scfv3U

Perhaps we could use `SourceManager::isInSystemHeader()` or extract some logic 
from it to test whether the actual included file is in the system header search 
path?



Comment at: docs/ReleaseNotes.rst:116
+
+  Checks for allowed includes and suggests removal of any others. If no 
includes
+  are specified, the check will exit without issuing any warnings.

Be explicit that this only impacts system headers.



Comment at: docs/clang-tidy/checks/fuchsia-restrict-includes.rst:6-7
+
+Checks for allowed includes and suggests removal of any others. If no includes
+are specified, the check will exit without issuing any warnings. 
+

Should also be explicit about system headers.



Comment at: test/clang-tidy/Inputs/fuchsia-restrict-includes/system/r.h:2
+void f() {}
\ No newline at end of file


Please add in the EOF.



Comment at: 
test/clang-tidy/Inputs/fuchsia-restrict-includes/system/transitive.h:2
+#include 
\ No newline at end of file


Please add in the EOF.



Comment at: test/clang-tidy/fuchsia-restrict-includes.cpp:31
+}
\ No newline at end of file


Please add in the EOF.


https://reviews.llvm.org/D43778



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

[PATCH] D46439: Fix incorrect packed aligned structure layout

2018-05-08 Thread Momchil Velikov via Phabricator via cfe-commits

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

Update: updated comment, added a test.




Comment at: lib/Sema/SemaDecl.cpp:15651
 }
   } else {
 ObjCIvarDecl **ClsFields =

rsmith wrote:
> Do we need to do any attribute processing in this Objective-C interface / 
> implementation / category case?
I think we didn't need it, or else `ProcessDeclAttributeList` would have been 
called with a null `Record` argument.


https://reviews.llvm.org/D46439



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

[PATCH] D46439: Fix incorrect packed aligned structure layout

2018-05-08 Thread Momchil Velikov via Phabricator via cfe-commits

chill updated this revision to Diff 145687.

https://reviews.llvm.org/D46439

Files:
  lib/Sema/SemaDecl.cpp
  test/Layout/itanium-pack-and-align.cpp


Index: test/Layout/itanium-pack-and-align.cpp
===
--- /dev/null
+++ test/Layout/itanium-pack-and-align.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only 
-fdump-record-layouts %s \
+// RUN:| FileCheck %s
+
+struct S {
+  char x;
+  int y;
+} __attribute__((packed, aligned(8)));
+
+struct alignas(8) T {
+  char x;
+  int y;
+} __attribute__((packed));
+
+S s;
+T t;
+// CHECK:  0 | struct T
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NEXT:|  nvsize=8, nvalign=8]
+
+// CHECK:  0 | struct S
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NETX:|  nvsize=8, nvalign=8]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15573,6 +15573,10 @@
 if (!Completed)
   Record->completeDefinition();
 
+// Handle attributes before checking the layout.
+if (Attr)
+  ProcessDeclAttributeList(S, Record, Attr);
+
 // We may have deferred checking for a deleted destructor. Check now.
 if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
   auto *Dtor = CXXRecord->getDestructor();
@@ -15703,9 +15707,6 @@
   CDecl->setIvarRBraceLoc(RBrac);
 }
   }
-
-  if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
 }
 
 /// \brief Determine whether the given integral value is representable within


Index: test/Layout/itanium-pack-and-align.cpp
===
--- /dev/null
+++ test/Layout/itanium-pack-and-align.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only -fdump-record-layouts %s \
+// RUN:| FileCheck %s
+
+struct S {
+  char x;
+  int y;
+} __attribute__((packed, aligned(8)));
+
+struct alignas(8) T {
+  char x;
+  int y;
+} __attribute__((packed));
+
+S s;
+T t;
+// CHECK:  0 | struct T
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NEXT:|  nvsize=8, nvalign=8]
+
+// CHECK:  0 | struct S
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NETX:|  nvsize=8, nvalign=8]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15573,6 +15573,10 @@
 if (!Completed)
   Record->completeDefinition();
 
+// Handle attributes before checking the layout.
+if (Attr)
+  ProcessDeclAttributeList(S, Record, Attr);
+
 // We may have deferred checking for a deleted destructor. Check now.
 if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
   auto *Dtor = CXXRecord->getDestructor();
@@ -15703,9 +15707,6 @@
   CDecl->setIvarRBraceLoc(RBrac);
 }
   }
-
-  if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
 }
 
 /// \brief Determine whether the given integral value is representable within
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331768 - [OPENMP, NVPTX] Fix linkage of the global entries.

2018-05-08 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Tue May  8 07:16:57 2018
New Revision: 331768

URL: http://llvm.org/viewvc/llvm-project?rev=331768=rev
Log:
[OPENMP, NVPTX] Fix linkage of the global entries.

The linkage of the global entries must be weak to enable support of
redefinition of the same target regions in multiple compilation units.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/declare_target_codegen.cpp
cfe/trunk/test/OpenMP/declare_target_codegen_globalization.cpp
cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_data_sharing.cpp
cfe/trunk/test/OpenMP/nvptx_declare_target_var_ctor_dtor_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp

cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
cfe/trunk/test/OpenMP/target_codegen.cpp
cfe/trunk/test/OpenMP/target_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_for_debug_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_if_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/target_private_codegen.cpp
cfe/trunk/test/OpenMP/target_reduction_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_teams_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_private_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_private_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_num_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_thread_limit_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=331768=331767=331768=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue May  8 07:16:57 2018
@@ -3863,9 +3863,9 @@ void CGOpenMPRuntime::createOffloadEntry
 llvm::ConstantInt::get(CGM.Int32Ty, Flags),
 llvm::ConstantInt::get(CGM.Int32Ty, 0)};
   std::string EntryName = getName({"omp_offloading", "entry", ""});
-  llvm::GlobalVariable *Entry =
-  createConstantGlobalStruct(CGM, getTgtOffloadEntryQTy(), Data,
- Twine(EntryName).concat(Name), Linkage);
+  llvm::GlobalVariable *Entry = createConstantGlobalStruct(
+  CGM, getTgtOffloadEntryQTy(), Data, Twine(EntryName).concat(Name),
+  llvm::GlobalValue::WeakAnyLinkage);
 
   // The entry has to be created in the section the linker expects it to be.
   std::string Section = getName({"omp_offloading", "entries"});
@@ -6281,13 +6281,13 @@ void CGOpenMPRuntime::emitTargetOutlined
 
   if

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added a comment.

In https://reviews.llvm.org/D46187#1088722, @NoQ wrote:

> It seems that you're using debug checkers of the analyzer to gain some free 
> tools for exploring the source code (such as displaying a call graph) for 
> free, right?
>
> I believe we could also benefit from a method of extracting the analyzer's 
> `clang -cc1` run-line from clang-tidy.


It should be possible now using `clang-tidy -extra-arg=-v some/file.cpp` (or 
`clang-check -extra-arg=-v ...`).


Repository:
  rC Clang

https://reviews.llvm.org/D46187



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

[PATCH] D46188: [clang-tidy] Add a flag to enable debug checkers

2018-05-08 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added a comment.

In https://reviews.llvm.org/D46188#1091237, @lebedev.ri wrote:

> In https://reviews.llvm.org/D46188#1091221, @alexfh wrote:
>
> > I don't think debug CSA checkers belong to clang-tidy. If one needs to dump 
> > CFG, they are probably doing quite involved stuff already, so going a step 
> > further and running `clang -cc1 -analyze` would not be difficult. The 
> > motivation for exposing alpha checkers (letting users test them and provide 
> > feedback) doesn't work for debug checkers.
>
>
> There is word play here.
>  They are debug CSA checkers. But not all of them are debugging the CSA. Many 
> of them are just utility checkers that could be just as easily a clang-tidy 
> debug/utility checkers:

I roughly understand what debug CSA checkers are. However, I don't see much 
value in exposing them in clang-tidy. While it may be valuable to look at the 
CFG dump when developing a clang-tidy check, for example, but this can be done 
using clang

  $ clang -cc1 -help | grep cfg
   -cfg-add-implicit-dtors Add C++ implicit destructors to CFGs for all analyses
   -cfg-add-initializers   Add C++ initializers to CFGs for all analyses
   -cfg-dump   Display Control-Flow Graphs
   -cfg-view   View Control-Flow Graphs using GraphViz
   -unoptimized-cfgGenerate unoptimized CFGs for all analyses

All CSA debug checkers can also be invoked using clang -cc1 --analyze. The only 
thing clang-tidy could bring is a possibility to more easily run all these 
tools on a file from a project that is using a compilation database. However, 
that doesn't seem like a very valuable functionality, because 1. working on 
small artificial test cases (or test cases reduced from real code) is easier, 
2. in a rare case when a file from a real project needs to be analyzed, it's 
possible to get compilation arguments and feed them to clang (e.g. using 
clang-tidy -extra-arg=-v file.cpp).

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46188

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

[PATCH] D46382: [OpenCL] Factor out language version printing

2018-05-08 Thread Sven van Haastregt via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331766: [OpenCL] Factor out language version printing 
(authored by svenvh, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46382?vs=144991=145684#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46382

Files:
  cfe/trunk/include/clang/Basic/LangOptions.h
  cfe/trunk/lib/Basic/LangOptions.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Parse/ParseDecl.cpp


Index: cfe/trunk/lib/Basic/LangOptions.cpp
===
--- cfe/trunk/lib/Basic/LangOptions.cpp
+++ cfe/trunk/lib/Basic/LangOptions.cpp
@@ -43,3 +43,8 @@
   return true;
   return false;
 }
+
+VersionTuple LangOptions::getOpenCLVersionTuple() const {
+  const int Ver = OpenCLCPlusPlus ? OpenCLCPlusPlusVersion : OpenCLVersion;
+  return VersionTuple(Ver / 100, (Ver % 100) / 10);
+}
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -76,7 +76,6 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetOptions.h"
 #include 
@@ -2157,11 +2156,9 @@
   // this option was added for compatibility with OpenCL 1.0.
   if (Args.getLastArg(OPT_cl_strict_aliasing)
&& Opts.OpenCLVersion > 100) {
-std::string VerSpec = llvm::to_string(Opts.OpenCLVersion / 100) +
-  std::string(".") +
-  llvm::to_string((Opts.OpenCLVersion % 100) / 10);
 Diags.Report(diag::warn_option_invalid_ocl_version)
-  << VerSpec << Args.getLastArg(OPT_cl_strict_aliasing)->getAsString(Args);
+<< Opts.getOpenCLVersionTuple().getAsString()
+<< Args.getLastArg(OPT_cl_strict_aliasing)->getAsString(Args);
   }
 
   // We abuse '-f[no-]gnu-keywords' to force overriding all GNU-extension
Index: cfe/trunk/lib/Parse/ParseDecl.cpp
===
--- cfe/trunk/lib/Parse/ParseDecl.cpp
+++ cfe/trunk/lib/Parse/ParseDecl.cpp
@@ -29,7 +29,6 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/ScopedPrinter.h"
 
 using namespace clang;
 
@@ -3806,11 +3805,8 @@
 Diag(Tok, DiagID)
   << PrevSpec << FixItHint::CreateRemoval(Tok.getLocation());
   else if (DiagID == diag::err_opencl_unknown_type_specifier) {
-const int OpenCLVer = getLangOpts().OpenCLVersion;
-std::string VerSpec = llvm::to_string(OpenCLVer / 100) +
-  std::string (".") +
-  llvm::to_string((OpenCLVer % 100) / 10);
-Diag(Tok, DiagID) << VerSpec << PrevSpec << isStorageClass;
+Diag(Tok, DiagID) << 
getLangOpts().getOpenCLVersionTuple().getAsString()
+  << PrevSpec << isStorageClass;
   } else
 Diag(Tok, DiagID) << PrevSpec;
 }
Index: cfe/trunk/include/clang/Basic/LangOptions.h
===
--- cfe/trunk/include/clang/Basic/LangOptions.h
+++ cfe/trunk/include/clang/Basic/LangOptions.h
@@ -254,6 +254,9 @@
   bool assumeFunctionsAreConvergent() const {
 return (CUDA && CUDAIsDevice) || OpenCL;
   }
+
+  /// \brief Return the OpenCL C or C++ version as a VersionTuple.
+  VersionTuple getOpenCLVersionTuple() const;
 };
 
 /// \brief Floating point control options


Index: cfe/trunk/lib/Basic/LangOptions.cpp
===
--- cfe/trunk/lib/Basic/LangOptions.cpp
+++ cfe/trunk/lib/Basic/LangOptions.cpp
@@ -43,3 +43,8 @@
   return true;
   return false;
 }
+
+VersionTuple LangOptions::getOpenCLVersionTuple() const {
+  const int Ver = OpenCLCPlusPlus ? OpenCLCPlusPlusVersion : OpenCLVersion;
+  return VersionTuple(Ver / 100, (Ver % 100) / 10);
+}
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -76,7 +76,6 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetOptions.h"
 #include 
@@ -2157,11 +2156,9 @@
   // this option was added for compatibility with OpenCL 1.0.
   if (Args.getLastArg(OPT_cl_strict_aliasing)
&& Opts.OpenCLVersion > 100) {
-std::string VerSpec = llvm::to_string(Opts.OpenCLVersion / 100) +
-  std::string(".") +
-

r331766 - [OpenCL] Factor out language version printing

2018-05-08 Thread Sven van Haastregt via cfe-commits

Author: svenvh
Date: Tue May  8 06:47:43 2018
New Revision: 331766

URL: http://llvm.org/viewvc/llvm-project?rev=331766=rev
Log:
[OpenCL] Factor out language version printing

Generate a printable OpenCL language version number in a single place
and select between the OpenCL C or OpenCL C++ version accordingly.

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

Modified:
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/lib/Basic/LangOptions.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=331766=331765=331766=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Tue May  8 06:47:43 2018
@@ -254,6 +254,9 @@ public:
   bool assumeFunctionsAreConvergent() const {
 return (CUDA && CUDAIsDevice) || OpenCL;
   }
+
+  /// \brief Return the OpenCL C or C++ version as a VersionTuple.
+  VersionTuple getOpenCLVersionTuple() const;
 };
 
 /// \brief Floating point control options

Modified: cfe/trunk/lib/Basic/LangOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/LangOptions.cpp?rev=331766=331765=331766=diff
==
--- cfe/trunk/lib/Basic/LangOptions.cpp (original)
+++ cfe/trunk/lib/Basic/LangOptions.cpp Tue May  8 06:47:43 2018
@@ -43,3 +43,8 @@ bool LangOptions::isNoBuiltinFunc(String
   return true;
   return false;
 }
+
+VersionTuple LangOptions::getOpenCLVersionTuple() const {
+  const int Ver = OpenCLCPlusPlus ? OpenCLCPlusPlusVersion : OpenCLVersion;
+  return VersionTuple(Ver / 100, (Ver % 100) / 10);
+}

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=331766=331765=331766=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue May  8 06:47:43 2018
@@ -76,7 +76,6 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Regex.h"
-#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetOptions.h"
 #include 
@@ -2157,11 +2156,9 @@ static void ParseLangArgs(LangOptions 
   // this option was added for compatibility with OpenCL 1.0.
   if (Args.getLastArg(OPT_cl_strict_aliasing)
&& Opts.OpenCLVersion > 100) {
-std::string VerSpec = llvm::to_string(Opts.OpenCLVersion / 100) +
-  std::string(".") +
-  llvm::to_string((Opts.OpenCLVersion % 100) / 10);
 Diags.Report(diag::warn_option_invalid_ocl_version)
-  << VerSpec << Args.getLastArg(OPT_cl_strict_aliasing)->getAsString(Args);
+<< Opts.getOpenCLVersionTuple().getAsString()
+<< Args.getLastArg(OPT_cl_strict_aliasing)->getAsString(Args);
   }
 
   // We abuse '-f[no-]gnu-keywords' to force overriding all GNU-extension

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=331766=331765=331766=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue May  8 06:47:43 2018
@@ -29,7 +29,6 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/ScopedPrinter.h"
 
 using namespace clang;
 
@@ -3806,11 +3805,8 @@ void Parser::ParseDeclarationSpecifiers(
 Diag(Tok, DiagID)
   << PrevSpec << FixItHint::CreateRemoval(Tok.getLocation());
   else if (DiagID == diag::err_opencl_unknown_type_specifier) {
-const int OpenCLVer = getLangOpts().OpenCLVersion;
-std::string VerSpec = llvm::to_string(OpenCLVer / 100) +
-  std::string (".") +
-  llvm::to_string((OpenCLVer % 100) / 10);
-Diag(Tok, DiagID) << VerSpec << PrevSpec << isStorageClass;
+Diag(Tok, DiagID) << 
getLangOpts().getOpenCLVersionTuple().getAsString()
+  << PrevSpec << isStorageClass;
   } else
 Diag(Tok, DiagID) << PrevSpec;
 }


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

1 2 >

1 - 100 of 149 matches

Mail list logo