[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc created this revision.
chandlerc added reviewers: rsmith, mehdi_amini.
chandlerc added a subscriber: cfe-commits.
Herald added a subscriber: mcrosier.

Much to my surprise, '-disable-llvm-optzns' which I thought was the
magical flag I wanted to get at the raw LLVM IR coming out of Clang
deosn't do that. It still runs some passes over the IR. I don't want
that, I really want the *raw* IR coming out of Clang and I strongly
suspect everyone else using it is in the same camp.

I've not talked to anyone debugging Clang and LLVM interactions that
needed these different flags so I'd like to remove the less widely known
one (I think) and consolidate on a simple, strong model for the one flag
left.

This is part of simplifying how Clang drives LLVM to make it cleaner to
wire up to the new pass manager.


https://reviews.llvm.org/D28047

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/debug-info-scope.c
  test/CodeGen/sanitize-address-field-padding.cpp
  test/CodeGenCUDA/convergent.cu
  test/CodeGenCUDA/fp-contract.cu
  test/CodeGenCUDA/link-device-bitcode.cu
  test/CodeGenCUDA/nothrow.cu
  test/CodeGenCXX/alias-available-externally.cpp
  test/CodeGenCoroutines/coro-alloc.cpp
  test/CodeGenCoroutines/coro-builtins.c
  test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp
  test/CodeGenObjC/arc-linetable.m
  test/Driver/save-temps.c

Index: test/Driver/save-temps.c
===
--- test/Driver/save-temps.c
+++ test/Driver/save-temps.c
@@ -2,7 +2,7 @@
 // RUN:   | FileCheck %s
 // CHECK: "-o" "save-temps.i"
 // CHECK: "-emit-llvm-uselists"
-// CHECK: "-disable-llvm-passes"
+// CHECK: "-disable-llvm-optzns"
 // CHECK: "-o" "save-temps.bc"
 // CHECK: "-o" "save-temps.s"
 // CHECK: "-o" "save-temps.o"
@@ -14,7 +14,7 @@
 // RUN:   | FileCheck %s -check-prefix=CWD
 // CWD: "-o" "save-temps.i"
 // CWD: "-emit-llvm-uselists"
-// CWD: "-disable-llvm-passes"
+// CWD: "-disable-llvm-optzns"
 // CWD: "-o" "save-temps.bc"
 // CWD: "-o" "save-temps.s"
 // CWD: "-o" "save-temps.o"
@@ -63,16 +63,16 @@
 // RUN: %clang -target x86_64-apple-darwin -save-temps=obj -o obj/dir/a.out -arch x86_64 %s -### 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-OBJ
 // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.i"
-// CHECK-OBJ: "-disable-llvm-passes"
+// CHECK-OBJ: "-disable-llvm-optzns"
 // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.bc"
 // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.s"
 // CHECK-OBJ: "-o" "obj/dir{{/|}}save-temps.o"
 // CHECK-OBJ: "-o" "obj/dir{{/|}}a.out"
 //
 // RUN: %clang -target x86_64-apple-darwin -save-temps=obj -arch x86_64 %s -### 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-OBJ-NOO
 // CHECK-OBJ-NOO: "-o" "save-temps.i"
-// CHECK-OBJ-NOO: "-disable-llvm-passes"
+// CHECK-OBJ-NOO: "-disable-llvm-optzns"
 // CHECK-OBJ-NOO: "-o" "save-temps.bc"
 // CHECK-OBJ-NOO: "-o" "save-temps.s"
 // CHECK-OBJ-NOO: "-o" "save-temps.o"
Index: test/CodeGenObjC/arc-linetable.m
===
--- test/CodeGenObjC/arc-linetable.m
+++ test/CodeGenObjC/arc-linetable.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -fblocks -fobjc-arc -debug-info-kind=standalone -dwarf-version=4 -disable-llvm-passes -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -fblocks -fobjc-arc -debug-info-kind=standalone -dwarf-version=4 -disable-llvm-optzns -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
 
 // Legend: EXP = Return expression, RET = ret instruction
 
Index: test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp
===
--- test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp
+++ test/CodeGenCoroutines/microsoft-abi-operator-coawait.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -std=c++14 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -std=c++14 -disable-llvm-optzns | FileCheck %s
 struct no_suspend {
   bool await_ready() { return true; }
   template  void await_suspend(F) {}
Index: test/CodeGenCoroutines/coro-builtins.c
===
--- test/CodeGenCoroutines/coro-builtins.c
+++ test/CodeGenCoroutines/coro-builtins.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc18.0.0 -fcoroutines-ts -emit-llvm %s -o - -disable-llvm-optzns | FileCheck %s
 
 void *myAlloc(long long);
 
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-allo

[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.

2016-12-22 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments.



Comment at: docs/LibASTMatchersReference.html:2442
   };
-fieldDecl(isBitField())
+fieldDecl(hasBitWidth(2))
   matches 'int a;' and 'int c;' but not 'int b;'.

Fix not connected to patch?



Comment at: docs/LibASTMatchersReference.html:4763
+fieldDecl(hasInClassInitializer(integerLiteral(equals(2
+  matches 'int a;' but not 'int b;'.
+

It would be good to add int c; without initializer and show that 
fieldDecl(hasInClassInitializer(anything())) will match to a and b



Comment at: include/clang/ASTMatchers/ASTMatchers.h:547
 /// \endcode
-/// fieldDecl(isBitField())
+/// fieldDecl(hasBitWidth(2))
 ///   matches 'int a;' and 'int c;' but not 'int b;'.

Samw


https://reviews.llvm.org/D28034



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


[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.

2016-12-22 Thread Egor Churaev via Phabricator via cfe-commits
echuraev created this revision.
echuraev added a reviewer: Anastasia.
echuraev added subscribers: cfe-commits, bader, yaxunl.

We compile user opencl kernel code with spir triple. But built-ins are written 
in OpenCL and we compile it with triple x86_64 to be able to use x86 
intrinsics. And we need address spaces to match in both cases. So, we change 
fake address space map in OpenCL for matching with spir.

On CPU address spaces are not really important but we'd like to preserve 
address space information in order to perform optimizations relying on this 
info like enhanced alias analysis.


https://reviews.llvm.org/D28048

Files:
  lib/AST/ASTContext.cpp
  test/CodeGen/blocks-opencl.cl
  test/CodeGenOpenCL/address-space-constant-initializers.cl
  test/CodeGenOpenCL/address-spaces-mangling.cl
  test/CodeGenOpenCL/address-spaces.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  test/CodeGenOpenCL/const-str-array-decay.cl
  test/CodeGenOpenCL/constant-addr-space-globals.cl
  test/CodeGenOpenCL/local-initializer-undef.cl
  test/CodeGenOpenCL/local.cl
  test/CodeGenOpenCL/memcpy.cl
  test/CodeGenOpenCL/str_literals.cl
  test/SemaOpenCL/extern.cl

Index: test/SemaOpenCL/extern.cl
===
--- test/SemaOpenCL/extern.cl
+++ test/SemaOpenCL/extern.cl
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -x cl -cl-opt-disable -cl-std=CL1.2 -emit-llvm -ffake-address-space-map %s -o - -verify | FileCheck %s
 // expected-no-diagnostics
 
-// CHECK: @foo = external addrspace(3) constant float
+// CHECK: @foo = external addrspace(2) constant float
 extern constant float foo;
 
 kernel void test(global float* buf) {
Index: test/CodeGenOpenCL/str_literals.cl
===
--- test/CodeGenOpenCL/str_literals.cl
+++ test/CodeGenOpenCL/str_literals.cl
@@ -3,7 +3,7 @@
 __constant char * __constant x = "hello world";
 __constant char * __constant y = "hello world";
 
-// CHECK: unnamed_addr addrspace(3) constant
-// CHECK-NOT: addrspace(3) unnamed_addr constant
-// CHECK: @x = addrspace(3) constant i8 addrspace(3)*
-// CHECK: @y = addrspace(3) constant i8 addrspace(3)*
+// CHECK: unnamed_addr addrspace(2) constant
+// CHECK-NOT: addrspace(2) unnamed_addr constant
+// CHECK: @x = addrspace(2) constant i8 addrspace(2)*
+// CHECK: @y = addrspace(2) constant i8 addrspace(2)*
Index: test/CodeGenOpenCL/memcpy.cl
===
--- test/CodeGenOpenCL/memcpy.cl
+++ test/CodeGenOpenCL/memcpy.cl
@@ -2,7 +2,7 @@
 
 // CHECK-LABEL: @test
 // CHECK-NOT: addrspacecast
-// CHECK: call void @llvm.memcpy.p1i8.p3i8
+// CHECK: call void @llvm.memcpy.p1i8.p2i8
 kernel void test(global float *g, constant float *c) {
   __builtin_memcpy(g, c, 32);
 }
Index: test/CodeGenOpenCL/local.cl
===
--- test/CodeGenOpenCL/local.cl
+++ test/CodeGenOpenCL/local.cl
@@ -3,7 +3,7 @@
 void func(local int*);
 
 __kernel void foo(void) {
-  // CHECK: @foo.i = internal addrspace(2) global i32 undef
+  // CHECK: @foo.i = internal addrspace(3) global i32 undef
   __local int i;
   func(&i);
 }
Index: test/CodeGenOpenCL/local-initializer-undef.cl
===
--- test/CodeGenOpenCL/local-initializer-undef.cl
+++ test/CodeGenOpenCL/local-initializer-undef.cl
@@ -6,10 +6,10 @@
 float z;
 } Foo;
 
-// CHECK-DAG: @test.lds_int = internal addrspace(2) global i32 undef
-// CHECK-DAG: @test.lds_int_arr = internal addrspace(2) global [128 x i32] undef
-// CHECK-DAG: @test.lds_struct = internal addrspace(2) global %struct.Foo undef
-// CHECK-DAG: @test.lds_struct_arr = internal addrspace(2) global [64 x %struct.Foo] undef
+// CHECK-DAG: @test.lds_int = internal addrspace(3) global i32 undef
+// CHECK-DAG: @test.lds_int_arr = internal addrspace(3) global [128 x i32] undef
+// CHECK-DAG: @test.lds_struct = internal addrspace(3) global %struct.Foo undef
+// CHECK-DAG: @test.lds_struct_arr = internal addrspace(3) global [64 x %struct.Foo] undef
 __kernel void test()
 {
 __local int lds_int;
Index: test/CodeGenOpenCL/constant-addr-space-globals.cl
===
--- test/CodeGenOpenCL/constant-addr-space-globals.cl
+++ test/CodeGenOpenCL/constant-addr-space-globals.cl
@@ -12,9 +12,9 @@
 // in the constant address space).
 
 void foo(constant const int *p1, const int *p2, const int *p3);
-// CHECK: @k.arr1 = internal addrspace(3) constant [3 x i32] [i32 1, i32 2, i32 3]
-// CHECK: @k.arr2 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 4, i32 5, i32 6]
-// CHECK: @k.arr3 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 7, i32 8, i32 9]
+// CHECK: @k.arr1 = internal addrspace(2) constant [3 x i32] [i32 1, i32 2, i32 3]
+// CHECK: @k.arr2 = private unnamed_addr addrspace(2) constant [3 x i32] [i32 4, i32 5, i32 6]
+// CHECK: @k.arr3 = pr

[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.

2016-12-22 Thread Malcolm Parsons via Phabricator via cfe-commits
malcolm.parsons added inline comments.



Comment at: docs/LibASTMatchersReference.html:2442
   };
-fieldDecl(isBitField())
+fieldDecl(hasBitWidth(2))
   matches 'int a;' and 'int c;' but not 'int b;'.

Prazek wrote:
> Fix not connected to patch?
Yes. The documentation is generated from the comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:547
 /// \endcode
-/// fieldDecl(isBitField())
+/// fieldDecl(hasBitWidth(2))
 ///   matches 'int a;' and 'int c;' but not 'int b;'.

Prazek wrote:
> Samw
Yes.


https://reviews.llvm.org/D28034



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


[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.

2016-12-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.

https://reviews.llvm.org/D28052

Files:
  change-namespace/ChangeNamespace.cpp
  change-namespace/ChangeNamespace.h
  unittests/change-namespace/ChangeNamespaceTests.cpp

Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -825,6 +825,109 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, NamespaceAliasInGlobal) {
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace gl = glob;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { gl::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace gl = glob;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { gl::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInNamespace) {
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace gl = glob;\n"
+ "void f() { gl::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace gl = glob;\n"
+ "void f() { gl::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInAncestorNamespace) {
+  NewNamespace = "na::nx";
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "namespace nb {\n"
+ "void f() { ga::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "\n"
+ "namespace nx {\n"
+ "void f() { ga::Glob g; }\n"
+ "} // namespace nx\n"
+ "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInOtherNamespace) {
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "namespace nb {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "\n"
+ "} // namespace na\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { glob::Glo g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, UsingDeclAfterReference) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
Index: change-namespace/

[PATCH] D26465: [Diag] Optimize DiagnosticIDs::getDiagnosticSeverity

2016-12-22 Thread Olivier Goffart via Phabricator via cfe-commits
ogoffart marked an inline comment as done.
ogoffart added a comment.

In https://reviews.llvm.org/D26465#607860, @arphaman wrote:

> What did you test the parsing on? Will this patch get similar improvements 
> for code that compiles without errors and warnings?


It was benchamerked with https://github.com/woboq/woboq_codebrowser generating 
itself.  
The code does not contain warnings.  (unless you really consider the 
warn_cxx98_compat_* to be warnings)




Comment at: lib/Basic/DiagnosticIDs.cpp:423
+Mapping = &Pos->State->getOrAddMapping((diag::kind)DiagID);
+  }
 

arphaman wrote:
> I think it would be better if you wrap this piece of code in a static 
> function that returns `DiagnosticMapping &`, as it should allow you to get 
> rid of all these `.` to `->` changes below.
The problem is that most things are private in DiagnosticsEngine, so i made it 
a privte member of DiagnosticIds (which is a friend)


https://reviews.llvm.org/D26465



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


[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment.

There are two issues I have with the flags right now. The first one you are 
addressing with this patch. The second issue is they remain CC1-only flags. Can 
we introduce a proper "-emit-raw-llvm" flag to work like "-emit-llvm" but 
without the additional IR processing. We can make "-disable-llvm-optzns" an 
alias on the CC1-level for backwards compatibility. It is fine as a second 
step, but it would be nice to consider the big picture before shuffling things 
around.


https://reviews.llvm.org/D28047



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


[PATCH] D26465: [Diag] Optimize DiagnosticIDs::getDiagnosticSeverity

2016-12-22 Thread Olivier Goffart via Phabricator via cfe-commits
ogoffart updated this revision to Diff 82325.
ogoffart marked an inline comment as done.

https://reviews.llvm.org/D26465

Files:
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/DiagnosticIDs.h
  lib/Basic/Diagnostic.cpp
  lib/Basic/DiagnosticIDs.cpp
  lib/Serialization/ASTReader.cpp

Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5328,6 +5328,10 @@
 diag::Severity Map = (diag::Severity)F.PragmaDiagMappings[Idx++];
 DiagnosticMapping Mapping = Diag.makeUserMapping(Map, Loc);
 Diag.GetCurDiagState()->setMapping(DiagID, Mapping);
+
+// We are overriding a default behavior
+Diag.DiagStates.front().getOrAddMapping(DiagID).setMightBeOverriden(
+true);
   }
 }
   }
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -396,6 +396,23 @@
   return toLevel(getDiagnosticSeverity(DiagID, Loc, Diag));
 }
 
+DiagnosticMapping &
+DiagnosticIDs::getDiagnosticMapping(unsigned DiagID, SourceLocation Loc,
+const DiagnosticsEngine &Diag) const {
+  // Calling GetDiagStatePointForLoc might be costly as it needs to create
+  // FullSourceLoc and compare them to the ones in the states. So first query
+  // the base state and check if it might be overridden.
+  DiagnosticsEngine::DiagState &BaseState = Diag.DiagStates.front();
+  assert(&BaseState == Diag.DiagStatePoints.front().State);
+  DiagnosticMapping &Mapping = BaseState.getOrAddMapping((diag::kind)DiagID);
+  if (!Mapping.mightBeOverriden())
+return Mapping;
+
+  DiagnosticsEngine::DiagStatePointsTy::iterator Pos =
+  Diag.GetDiagStatePointForLoc(Loc);
+  return Pos->State->getOrAddMapping((diag::kind)DiagID);
+}
+
 /// \brief Based on the way the client configured the Diagnostic
 /// object, classify the specified diagnostic ID into a Level, consumable by
 /// the DiagnosticClient.
@@ -406,17 +423,11 @@
 DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
  const DiagnosticsEngine &Diag) const {
   assert(getBuiltinDiagClass(DiagID) != CLASS_NOTE);
-
   // Specific non-error diagnostics may be mapped to various levels from ignored
   // to error.  Errors can only be mapped to fatal.
   diag::Severity Result = diag::Severity::Fatal;
 
-  DiagnosticsEngine::DiagStatePointsTy::iterator
-Pos = Diag.GetDiagStatePointForLoc(Loc);
-  DiagnosticsEngine::DiagState *State = Pos->State;
-
-  // Get the mapping information, or compute it lazily.
-  DiagnosticMapping &Mapping = State->getOrAddMapping((diag::kind)DiagID);
+  DiagnosticMapping &Mapping = getDiagnosticMapping(DiagID, Loc, Diag);
 
   // TODO: Can a null severity really get here?
   if (Mapping.getSeverity() != diag::Severity())
Index: lib/Basic/Diagnostic.cpp
===
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -201,6 +201,11 @@
   }
   DiagnosticMapping Mapping = makeUserMapping(Map, L);
 
+  if (Loc.isValid()) {
+// We are potentially overriding a default behavior
+DiagStates.front().getOrAddMapping(Diag).setMightBeOverriden(true);
+  }
+
   // Common case; setting all the diagnostics of a group in one place.
   if (Loc.isInvalid() || Loc == LastStateChangePos) {
 GetCurDiagState()->setMapping(Diag, Mapping);
Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -84,6 +84,7 @@
   unsigned IsPragma : 1;
   unsigned HasNoWarningAsError : 1;
   unsigned HasNoErrorAsFatal : 1;
+  unsigned MightBeOverriden : 1;
 
 public:
   static DiagnosticMapping Make(diag::Severity Severity, bool IsUser,
@@ -94,6 +95,8 @@
 Result.IsPragma = IsPragma;
 Result.HasNoWarningAsError = 0;
 Result.HasNoErrorAsFatal = 0;
+Result.MightBeOverriden = 0;
+
 return Result;
   }
 
@@ -108,6 +111,11 @@
 
   bool hasNoErrorAsFatal() const { return HasNoErrorAsFatal; }
   void setNoErrorAsFatal(bool Value) { HasNoErrorAsFatal = Value; }
+
+  /// Only makes sense for the base state: specify if the diagnostic might be
+  /// changed for other source location
+  bool mightBeOverriden() const { return MightBeOverriden; }
+  void setMightBeOverriden(bool Value) { MightBeOverriden = Value; }
 };
 
 /// \brief Used for handling and querying diagnostic IDs.
@@ -261,6 +269,10 @@
   getDiagnosticLevel(unsigned DiagID, SourceLocation Loc,
  const DiagnosticsEngine &Diag) const LLVM_READONLY;
 
+  DiagnosticMapping &
+  getDiagnosticMapping(unsigned DiagID, SourceLocation Loc,
+   const DiagnosticsEngine &Diag) const LLVM_READONLY;
+
   diag::Severity
   getDiag

[PATCH] D26350: Keep invalid Switch in the AST

2016-12-22 Thread Olivier Goffart via Phabricator via cfe-commits
ogoffart added a comment.

ping2


https://reviews.llvm.org/D26350



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


[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.

2016-12-22 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a subscriber: pekka.jaaskelainen.
Anastasia added a comment.

I believe it's not that important what those numbers actually are, but having 
the consistency with SPIR might be better indeed at least even for 
documentation purposes!

@pekka.jaaskelainen, I think you are using x86 target in the POCL toolchain, do 
you envision any issues with this change?


https://reviews.llvm.org/D28048



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


[PATCH] D27981: Fix problems in "[OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand."

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

LGTM! If you checked there are no warnings any more, it seems fine to commit! :)


https://reviews.llvm.org/D27981



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


r290337 - clang-format: Less eagerly try to keep label-value pairs on a line.

2016-12-22 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Thu Dec 22 06:37:06 2016
New Revision: 290337

URL: http://llvm.org/viewvc/llvm-project?rev=290337&view=rev
Log:
clang-format: Less eagerly try to keep label-value pairs on a line.

Before:
  string v =
  StrCat("aaa: ", SomeFunction(,
   aaa),
 bbb);

After:
  string v = StrCat("aaa: ",
SomeFunction(, aaa),
bbb);

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=290337&r1=290336&r2=290337&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Dec 22 06:37:06 2016
@@ -2001,7 +2001,7 @@ unsigned TokenAnnotator::splitPenalty(co
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))
-return 100;
+return 45;
   if (Right.is(tok::plus) && Left.isLabelString() &&
   (Right.NextOperator || Right.OperatorIndex != 0))
 return 25;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290337&r1=290336&r2=290337&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Dec 22 06:37:06 2016
@@ -5220,6 +5220,10 @@ TEST_F(FormatTest, KeepStringLabelValueP
   verifyFormat("string v = StrCat(\": \" +\n"
"  (aaa + a));",
getLLVMStyleWithColumns(40));
+  verifyFormat(
+  "string v = StrCat(\"aaa: \",\n"
+  "  SomeFunction(, .aaa),\n"
+  "  bbb);");
 }
 
 TEST_F(FormatTest, UnderstandsEquals) {


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


[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc created this revision.
chandlerc added reviewers: rsmith, rnk, mehdi_amini.
chandlerc added a subscriber: cfe-commits.
chandlerc added a dependency: D28047: Remove the '-disable-llvm-passes' flag 
(which I didn't even know existed, and I suspect many others aren't aware of 
either) and strength '-disable-llvm-optzns' to do the same thing..
Herald added subscribers: nhaehnle, nemanjai, mcrosier, jholewinski.
Herald added a reviewer: tstellarAMD.

These were really, really tangled together:

- We used the noinline LLVM attribute for -fno-inline
  - But not for -fno-inline-functions (breaking LTO)
  - But we did use it for -finline-hint-functions (yay, LTO is happy!)
  - But we didn't for -O0 (LTO is sad yet again...)
- We had weird structuring of CodeGenOpts with both an inlining enumeration and 
a boolean. They interacted in weird ways and needlessly.
- A *lot* of set smashing went on with setting these, and then got worse when 
we considered optnone and other inlining-effecting attributes.
- A bunch of inline affecting attributes were managed in a completely different 
place from -fno-inline.
- Even with -fno-inline we failed to put the LLVM noinline attribute onto many 
generated function definitions because they didn't show up as AST-level 
functions.
- If you passed -O0 but -finline-functions we would run the normal inliner pass 
in LLVM despite it being in the O0 pipeline, which really doesn't make much 
sense.

Sadly, this causes a bunch of churn in tests because we don't run the
optimizer in the tests and check the contents of attribute sets. It
would be awesome if attribute sets were a bit more FileCheck friendly,
but oh well.

I think this is a significant improvement and should remove the semantic
need to change what inliner pass we run in order to comply with the
requested inlining semantics by relying completely on attributes. It
also cleans up tho optnone and related handling a bit.

One unfortunate aspect of this is that for generating alwaysinline
routines like those in OpenMP we end up removing noinline and then
adding alwaysinline. I tried a bunch of other approaches, but because we
recompute function attributes from scratch and don't have a declaration
here I couldn't find anything substantially cleaner than this.

Depends on https://reviews.llvm.org/D28047


https://reviews.llvm.org/D28053

Files:
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CXX/special/class.dtor/p3-0x.cpp
  test/CodeGen/2008-04-08-NoExceptions.c
  test/CodeGen/address-safety-attr-kasan.cpp
  test/CodeGen/address-safety-attr.cpp
  test/CodeGen/address-space-field1.c
  test/CodeGen/alias.c
  test/CodeGen/attr-minsize.cpp
  test/CodeGen/attributes.c
  test/CodeGen/incomplete-function-type-2.c
  test/CodeGen/inline-optim.c
  test/CodeGen/mips16-attr.c
  test/CodeGen/mrtd.c
  test/CodeGen/ms-declspecs.c
  test/CodeGen/ppc64-complex-parms.c
  test/CodeGen/ppc64-complex-return.c
  test/CodeGen/ppc64-extend.c
  test/CodeGen/sanitize-thread-attr.cpp
  test/CodeGen/sanitize-thread-no-checking-at-run-time.m
  test/CodeGen/unwind-attr.c
  test/CodeGenCXX/attr.cpp
  test/CodeGenCXX/cxx11-exception-spec.cpp
  test/CodeGenCXX/cxx11-noreturn.cpp
  test/CodeGenCXX/derived-to-base.cpp
  test/CodeGenCXX/global-dtor-no-atexit.cpp
  test/CodeGenCXX/global-init.cpp
  test/CodeGenCXX/inline-hint.cpp
  test/CodeGenCXX/main-norecurse.cpp
  test/CodeGenCXX/microsoft-abi-array-cookies.cpp
  test/CodeGenCXX/no-exceptions.cpp
  test/CodeGenCXX/optnone-class-members.cpp
  test/CodeGenCXX/optnone-def-decl.cpp
  test/CodeGenCXX/reference-cast.cpp
  test/CodeGenCXX/threadsafe-statics.cpp
  test/CodeGenCXX/thunks.cpp
  test/CodeGenCXX/virtual-base-cast.cpp
  test/CodeGenObjC/gnu-exceptions.m
  test/CodeGenObjC/objc-literal-tests.m
  test/CodeGenObjCXX/lambda-expressions.mm
  test/CodeGenOpenCL/amdgpu-attrs.cl
  test/Driver/darwin-iphone-defaults.m
  test/PCH/objc_container.m

Index: test/PCH/objc_container.m
===
--- test/PCH/objc_container.m
+++ test/PCH/objc_container.m
@@ -21,5 +21,5 @@
 // CHECK-IR: {{call.*objc_msgSend}}
 // CHECK-IR: ret void
 
-// CHECK-IR: attributes #0 = { nounwind {{.*}} }
+// CHECK-IR: attributes #0 = { noinline nounwind {{.*}} }
 // CHECK-IR: attributes #1 = { nonlazybind }
Index: test/Driver/darwin-iphone-defaults.m
===
--- test/Driver/darwin-iphone-defaults.m
+++ test/Driver/darwin-iphone-defaults.m
@@ -26,4 +26,4 @@
   [I1 alloc];
 }
 
-// CHECK: attributes [[F0]] = { ssp{{.*}} }
+// CHECK: attributes [[F0]] = { noinline ssp{{.*}} }
Index: test/CodeGenOpenCL/amdgpu-attrs.cl

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-22 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo updated this revision to Diff 82336.

https://reviews.llvm.org/D27753

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  test/Analysis/dirty-scalar-unbounded.cpp

Index: test/Analysis/dirty-scalar-unbounded.cpp
===
--- /dev/null
+++ test/Analysis/dirty-scalar-unbounded.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify -analyzer-config alpha.security.DirtyScalar:criticalOnly=true %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify -analyzer-config alpha.security.DirtyScalar:criticalOnly=false -DDIRTYSCALARSTRICT=1 %s
+
+#include "Inputs/system-header-simulator.h"
+
+typedef long ssize_t;
+
+ssize_t recv(int s, void *buf, size_t len, int flags);
+
+void gets_tainted_ival(int val) {
+  (void)val;
+}
+
+void gets_tainted_uval(unsigned int val) {
+  (void)val;
+}
+
+int tainted_usage() {
+  int size;
+  scanf("%d", &size);
+  char *buff = new char[size]; // expected-warning{{Tainted variable is used without proper bound checking}}
+  for (int i = 0; i < size; ++i) {
+#if DIRTYSCALARSTRICT
+// expected-warning@-2{{Tainted variable is used without proper bound checking}}
+#endif
+scanf("%d", &buff[i]);
+  }
+  buff[size - 1] = 0; // expected-warning{{Tainted variable is used without proper bound checking}}
+  *(buff + size - 2) = 0; // expected-warning{{Tainted variable is used without proper bound checking}}
+  gets_tainted_ival(size);
+#if DIRTYSCALARSTRICT
+// expected-warning@-2{{Tainted variable is used without proper bound checking}}
+#endif
+
+  return 0;
+}
+
+int tainted_usage_checked() {
+  int size;
+  scanf("%d", &size);
+  if (size < 0 || size > 255)
+return -1;
+  char *buff = new char[size]; // no warning
+  for (int i = 0; i < size; ++i) { // no warning
+scanf("%d", &buff[i]); // no warning
+  }
+  buff[size - 1] = 0;  // no warning
+  *(buff + size - 2) = 0;  // no warning
+  gets_tainted_ival(size); // no warning
+
+  unsigned int idx;
+  scanf("%d", &idx);
+  if (idx > 255)
+return -1;
+  gets_tainted_uval(idx); // no warning
+
+  return 0;
+}
+
+int detect_tainted(char const **messages) {
+  int sock, index;
+  scanf("%d", &sock);
+  if (recv(sock, &index, sizeof(index), 0) != sizeof(index)) {
+#if DIRTYSCALARSTRICT
+// expected-warning@-2{{Tainted variable is used without proper bound checking}}
+#endif
+return -1;
+  }
+  int index2 = index;
+  printf("%s\n", messages[index]);  // expected-warning{{Tainted variable is used without proper bound checking}}
+  printf("%s\n", messages[index2]); // expected-warning{{Tainted variable is used without proper bound checking}}
+
+  return 0;
+}
+
+int skip_sizes_likely_used_for_table_access(char const **messages) {
+  int sock;
+  char byte;
+
+  scanf("%d", &sock);
+  if (recv(sock, &byte, sizeof(byte), 0) != sizeof(byte)) {
+#if DIRTYSCALARSTRICT
+// expected-warning@-2{{Tainted variable is used without proper bound checking}}
+#endif
+return -1;
+  }
+  char byte2 = byte;
+  printf("%s\n", messages[byte]);  // no warning
+  printf("%s\n", messages[byte2]); // no warning
+
+  return 0;
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -222,6 +222,7 @@
 .Case("getline", TaintPropagationRule(2, 0))
 .Case("getdelim", TaintPropagationRule(3, 0))
 .Case("fgetln", TaintPropagationRule(0, ReturnValueIndex))
+.Case("recv", TaintPropagationRule(InvalidArgIndex, 1, true))
 .Default(TaintPropagationRule());
 
   if (!Rule.isNull())
Index: lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DirtyScalarChecker.cpp
@@ -0,0 +1,231 @@
+//===-- DirtyScalarChecker.cpp *- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Reports the usage of dirty integers in code.
+// A dirty value is tainted and wasn't bound checked properly by the programmer.
+// By default (criticalOnly == true) reports dirty usage in
+//   - memcpy, malloc, calloc, strcpy, strncpy, memmove functions
+//   - array indexing
+//   - memory allocation with new
+//   - pointer arithmetic
+// otherwise (criticalOnly == false) it also reports usage as
+//   - function argument
+//   - loop condition
+//
+//===--

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-22 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked 6 inline comments as done.
gerazo added a comment.

Thank you very much for your help. I've added all suggested modifications 
including tests covering all checker option settings.


https://reviews.llvm.org/D27753



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


[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

2016-12-22 Thread Zoltán Gera via Phabricator via cfe-commits
gerazo marked an inline comment as done.
gerazo added a comment.

So thank you again for the valuable questions.
In this checker, I give warnings for values which are both tainted and were 
also not checked by the programmer. So unlike GenericTaintChecker, I do 
implement the boundedness check here for certain, interesting constructs (which 
is controlled by the critical option). GenericTaintChecker focuses purely on 
taintedness, almost like a service for other checkers. I've added a new rule to 
it, improving the taintedness logic, but I felt mixing the bound checking logic 
into it would make the two ideas inseparable.

I've also checked others using tainted values. ArrayBoundCheckerV2 also works 
with array indexing and modifies its behaviour on finding tainted values. 
However, the main difference is that ArrayBoundCheckerV2 uses region 
information to check bounds which may or may not be present, while this checker 
can give a warning whenever any information from the constraint manager does 
not prove that any bound checking were performed on the value before (and 
potentially works on many other constructs where region information shouldn't 
be there at all). Not having correct region information with tainted values is 
typical when reading data from unknown sources. This dirty approach is better 
in this regard. ArrayBoundCheckerV2 on the other hand can give warnings solely 
based on region information. Yes, it can happen that both will give a warning 
as well for the same construct. Do you think it is distracting? Should I remove 
the array indexing checks from this checker (still it gives warning for pointer 
arithmetic as well)?




Comment at: test/Analysis/dirty-scalar-unbounded.cpp:1
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=alpha.security.taint,alpha.security.DirtyScalar -verify 
-analyzer-config alpha.security.DirtyScalar:criticalOnly=false %s
+

a.sidorin wrote:
> 1. It will be good to have tests for option set to true.
> 2. Is there any test that makes usage of 'RecurseLimit` variable?
Now both settings are covered. For RecurseLimit, I've added a named constant 
and better explanation. This is only a practical limit to not let the analysis 
dive too deep into complex loop expressions. The limit currently set should be 
adequate, so it would not make sense to set it programmatically.


https://reviews.llvm.org/D27753



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


[clang-tools-extra] r290340 - [clang-tidy] cppcoreguidelines-slicing: display discarded state size in bytes

2016-12-22 Thread Clement Courbet via cfe-commits
Author: courbet
Date: Thu Dec 22 08:12:31 2016
New Revision: 290340

URL: http://llvm.org/viewvc/llvm-project?rev=290340&view=rev
Log:
[clang-tidy] cppcoreguidelines-slicing: display discarded state size in bytes

https://reviews.llvm.org/D27212

Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp?rev=290340&r1=290339&r2=290340&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/SlicingCheck.cpp Thu 
Dec 22 08:12:31 2016
@@ -122,10 +122,11 @@ void SlicingCheck::check(const MatchFind
   BaseDecl->getASTContext().getASTRecordLayout(BaseDecl);
   const auto &DerivedLayout =
   DerivedDecl->getASTContext().getASTRecordLayout(DerivedDecl);
-  const auto StateSize = DerivedLayout.getDataSize() - 
BaseLayout.getDataSize();
+  const CharUnits StateSize =
+  DerivedLayout.getDataSize() - BaseLayout.getDataSize();
   if (StateSize.isPositive()) {
 diag(Call->getExprLoc(), "slicing object from type %0 to %1 discards "
- "%2*sizeof(char) bytes of state")
+ "%2 bytes of state")
 << DerivedDecl << BaseDecl << 
static_cast(StateSize.getQuantity());
   }
 }

Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp?rev=290340&r1=290339&r2=290340&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-slicing.cpp Thu 
Dec 22 08:12:31 2016
@@ -31,18 +31,18 @@ DerivedWithMemberVariables ReturnsDerive
 void positivesWithMemberVariables() {
   DerivedWithMemberVariables b;
   Base a{b};
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes 
of state [cppcoreguidelines-slicing]
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state 
[cppcoreguidelines-slicing]
   a = b;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes 
of state
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state
   TakesBaseByValue(b);
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes 
of state
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state
 
   TwiceDerivedWithNoMemberVariables c;
   a = c;
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'TwiceDerivedWithNoMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) 
bytes of state
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'TwiceDerivedWithNoMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state
 
   a = ReturnsDerived();
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}}*sizeof(char) bytes 
of state
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'DerivedWithMemberVariables' to 'Base' discards {{[0-9]*}} bytes of state
 }
 
 void positivesWithOverride() {


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


Re: [clang-tools-extra] r290202 - [clang-tidy] Add modernize-use-default-member-init check

2016-12-22 Thread Aaron Ballman via cfe-commits
On Wed, Dec 21, 2016 at 6:10 PM, NAKAMURA Takumi  wrote:
> It'd be the issue if the test depended on installed headers. The builder
> doesn't have MS headers installed.

Those type names do not require any headers to be installed. They're
keywords in C++11 and later, but MSVC did not support them until MSVC
2015.

~Aaron

>
> On Thu, Dec 22, 2016 at 1:27 AM Aaron Ballman 
> wrote:
>>
>> On Tue, Dec 20, 2016 at 5:58 PM, Malcolm Parsons
>>  wrote:
>> > On 20 December 2016 at 22:32, Aaron Ballman 
>> > wrote:
>> >> On Tue, Dec 20, 2016 at 4:26 PM, Malcolm Parsons via cfe-commits
>> >>  wrote:
>> >>> Author: malcolm.parsons
>> >>> Date: Tue Dec 20 15:26:07 2016
>> >>> New Revision: 290202
>> >>>
>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=290202&view=rev
>> >>> Log:
>> >>> [clang-tidy] Add modernize-use-default-member-init check
>> >>>
>> >>> Summary: Fixes PR18858
>> >>
>> >> This appears to have broken one of the bots:
>> >>
>> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/33046
>> >
>> > error: unknown type name 'char16_t'
>> > error: unknown type name 'char32_t'
>> >
>> > I see commented tests in other clang-tidy tests:
>> >
>> > // TODO: enable these tests once all supported compilers
>> > // support char16_t and char32_t (VS2013 does not)
>> >
>> > // disabled for now until it is clear
>> > // how to enable them in the test
>> > //} catch (const char16_t*) {
>>
>> We no longer support MSVC 2013, so that should not be an issue.
>> Takumi, I'm not certain what to make of this builder (MSVC centos?).
>> What version of Visual Studio is it compiling with? Or is this an
>> instance where we need to set the -fms-compatibility-version=19 when
>> running the test with clang-cl?
>>
>> ~Aaron
>>
>> >
>> > --
>> > Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

2016-12-22 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments.



Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:96
 
-  // Do not trigger on non-const value parameters when:
-  // 1. they are in a constructor definition since they can likely trigger
-  //modernize-pass-by-value which will suggest to move the argument.
-  if (!IsConstQualified && (llvm::isa(Function) ||
-!Function->doesThisDeclarationHaveABody()))
-return;
+  const FunctionDecl &Definition = *Function->getDefinition();
 

aaron.ballman wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > malcolm.parsons wrote:
> > > > malcolm.parsons wrote:
> > > > > aaron.ballman wrote:
> > > > > > Instead of using `hasBody()` and `getDefinition()`, you should use 
> > > > > > `FunctionDecl::getBody()` and pass in an argument to receive the 
> > > > > > function's definition.
> > > > > I don't want the body - the `CXXCtorInitializer`s are not in it.
> > > > I should use `hasBody()` and pass in an argument.
> > > You are calling `Function-hasBody()` followed by 
> > > `Function->getDefinition()`, which is what `FunctionDecl::getBody()` 
> > > does. The difference is that `FunctionDecl::getBody()` will also load 
> > > serialized bodies from the AST (if the body happens to be in a PCH, for 
> > > instance).
> > Do I want to load serialized bodies?
> I would imagine you would want to deserialize, since you care about the 
> contents of the body, not just the presence of one. This may or may not be 
> important with modules (Richard Smith would be able to give a more definitive 
> answer there).
I would suggest writing minimal sane code that works until we have good reasons 
to write more complex code to support module-enabled builds (and appropriate 
tests).



Comment at: clang-tidy/utils/DeclRefExprUtils.h:51
+
+/// Returns true if DeclRefExpr is the argument of a copy-constructor call
+/// expression within Decl.

Please enclose `true`, `DeclRefExpr` and other inline code snippets in double 
backquotes.


https://reviews.llvm.org/D28022



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


r290341 - [analyzer] Improve suppress-on-sink behavior in incomplete analyses.

2016-12-22 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Dec 22 08:48:52 2016
New Revision: 290341

URL: http://llvm.org/viewvc/llvm-project?rev=290341&view=rev
Log:
[analyzer] Improve suppress-on-sink behavior in incomplete analyses.

Warnings with suppress-on-sink are discarded during FlushReports when
BugReporter notices that all paths in ExplodedGraph that pass through the
warning eventually run into a sink node.

However, suppress-on-sink fails to filter out false positives when the analysis
terminates too early - by running into analyzer limits, such as block count
limits or graph size limits - and the interruption hits the narrow window
between throwing the leak report and reaching the no-return function call. In
such case the report is there, however suppression-on-sink doesn't work, because
the sink node was never constructed in the incomplete ExplodedGraph.

This patch implements a very partial solution: also suppress reports thrown
against a statement-node that corresponds to a statement that belongs to a
no-return block of the CFG.

rdar://problem/28832541

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

Added:
cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=290341&r1=290340&r2=290341&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Thu Dec 22 08:48:52 2016
@@ -21,6 +21,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -3285,6 +3286,19 @@ struct FRIEC_WLItem {
 };
 }
 
+static const CFGBlock *findBlockForNode(const ExplodedNode *N) {
+  ProgramPoint P = N->getLocation();
+  if (auto BEP = P.getAs())
+return BEP->getBlock();
+
+  // Find the node's current statement in the CFG.
+  if (const Stmt *S = PathDiagnosticLocation::getStmt(N))
+return N->getLocationContext()->getAnalysisDeclContext()
+  ->getCFGStmtMap()->getBlock(S);
+
+  return nullptr;
+}
+
 static BugReport *
 FindReportInEquivalenceClass(BugReportEquivClass& EQ,
  SmallVectorImpl &bugReports) {
@@ -,6 +3347,18 @@ FindReportInEquivalenceClass(BugReportEq
   continue;
 }
 
+// See if we are in a no-return CFG block. If so, treat this similarly
+// to being post-dominated by a sink. This works better when the analysis
+// is incomplete and we have never reached a no-return function
+// we're post-dominated by.
+// This is not quite enough to handle the incomplete analysis case.
+// We may be post-dominated in subsequent blocks, or even
+// inter-procedurally. However, it is not clear if more complicated
+// cases are generally worth suppressing.
+if (const CFGBlock *B = findBlockForNode(errorNode))
+  if (B->hasNoReturnElement())
+continue;
+
 // At this point we know that 'N' is not a sink and it has at least one
 // successor.  Use a DFS worklist to find a non-sink end-of-path node.
 typedef FRIEC_WLItem WLItem;

Added: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c?rev=290341&view=auto
==
--- cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c (added)
+++ cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c Thu Dec 22 08:48:52 
2016
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
max-nodes=12 -verify %s
+
+// Here we test how "suppress on sink" feature of certain bugtypes interacts
+// with reaching analysis limits.
+
+// If we report a warning of a bug-type with "suppress on sink" attribute set
+// (such as MallocChecker's memory leak warning), then failing to reach the
+// reason for the sink (eg. no-return function such as "exit()") due to 
analysis
+// limits (eg. max-nodes option), we may produce a false positive.
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+extern void exit(int) __attribute__ ((__noreturn__));
+
+void clang_analyzer_warnIfReached(void);
+
+void test_single_cfg_block_sink() {
+  void *p = malloc(1); // no-warning (wherever the leak warning may occur here)
+
+  // Due to max-nodes option in the run line, we should reach the first call
+  // but bail out before the second call.
+  // If the test on these two lines starts failing, see if modifying
+  // the max-nodes run-line helps.
+  clang_analyzer_warnIfReached

[PATCH] D28023: [analyzer] Fix leak false positives before no-return functions caused by incomplete analyses.

2016-12-22 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290341: [analyzer] Improve suppress-on-sink behavior in 
incomplete analyses. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D28023?vs=82229&id=82339#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28023

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
  cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c


Index: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
===
--- cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
+++ cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config 
max-nodes=12 -verify %s
+
+// Here we test how "suppress on sink" feature of certain bugtypes interacts
+// with reaching analysis limits.
+
+// If we report a warning of a bug-type with "suppress on sink" attribute set
+// (such as MallocChecker's memory leak warning), then failing to reach the
+// reason for the sink (eg. no-return function such as "exit()") due to 
analysis
+// limits (eg. max-nodes option), we may produce a false positive.
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+extern void exit(int) __attribute__ ((__noreturn__));
+
+void clang_analyzer_warnIfReached(void);
+
+void test_single_cfg_block_sink() {
+  void *p = malloc(1); // no-warning (wherever the leak warning may occur here)
+
+  // Due to max-nodes option in the run line, we should reach the first call
+  // but bail out before the second call.
+  // If the test on these two lines starts failing, see if modifying
+  // the max-nodes run-line helps.
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  clang_analyzer_warnIfReached(); // no-warning
+
+  // Even though we do not reach this line, we should still suppress
+  // the leak report.
+  exit(0);
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/Analysis/CFG.h"
+#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -3285,6 +3286,19 @@
 };
 }
 
+static const CFGBlock *findBlockForNode(const ExplodedNode *N) {
+  ProgramPoint P = N->getLocation();
+  if (auto BEP = P.getAs())
+return BEP->getBlock();
+
+  // Find the node's current statement in the CFG.
+  if (const Stmt *S = PathDiagnosticLocation::getStmt(N))
+return N->getLocationContext()->getAnalysisDeclContext()
+  ->getCFGStmtMap()->getBlock(S);
+
+  return nullptr;
+}
+
 static BugReport *
 FindReportInEquivalenceClass(BugReportEquivClass& EQ,
  SmallVectorImpl &bugReports) {
@@ -,6 +3347,18 @@
   continue;
 }
 
+// See if we are in a no-return CFG block. If so, treat this similarly
+// to being post-dominated by a sink. This works better when the analysis
+// is incomplete and we have never reached a no-return function
+// we're post-dominated by.
+// This is not quite enough to handle the incomplete analysis case.
+// We may be post-dominated in subsequent blocks, or even
+// inter-procedurally. However, it is not clear if more complicated
+// cases are generally worth suppressing.
+if (const CFGBlock *B = findBlockForNode(errorNode))
+  if (B->hasNoReturnElement())
+continue;
+
 // At this point we know that 'N' is not a sink and it has at least one
 // successor.  Use a DFS worklist to find a non-sink end-of-path node.
 typedef FRIEC_WLItem WLItem;


Index: cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
===
--- cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
+++ cfe/trunk/test/Analysis/max-nodes-suppress-on-sink.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config max-nodes=12 -verify %s
+
+// Here we test how "suppress on sink" feature of certain bugtypes interacts
+// with reaching analysis limits.
+
+// If we report a warning of a bug-type with "suppress on sink" attribute set
+// (such as MallocChecker's memory leak warning), then failing to reach the
+// reason for the sink (eg. no-return function such as "exit()") due to analysis
+// limits (eg. max-nodes option), we may produce a false positive.
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+extern void exit(int) __attribute__ ((__noreturn__));
+
+void clang_analyzer_warnIfReached(void);
+
+void test_single_cfg_bloc

[PATCH] D28034: [ASTMatchers] Add hasInClassInitializer traversal matcher for FieldDecl.

2016-12-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, modulo the unrelated documentation changes.


https://reviews.llvm.org/D28034



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


[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

2016-12-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:96
 
-  // Do not trigger on non-const value parameters when:
-  // 1. they are in a constructor definition since they can likely trigger
-  //modernize-pass-by-value which will suggest to move the argument.
-  if (!IsConstQualified && (llvm::isa(Function) ||
-!Function->doesThisDeclarationHaveABody()))
-return;
+  const FunctionDecl &Definition = *Function->getDefinition();
 

alexfh wrote:
> aaron.ballman wrote:
> > malcolm.parsons wrote:
> > > aaron.ballman wrote:
> > > > malcolm.parsons wrote:
> > > > > malcolm.parsons wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Instead of using `hasBody()` and `getDefinition()`, you should 
> > > > > > > use `FunctionDecl::getBody()` and pass in an argument to receive 
> > > > > > > the function's definition.
> > > > > > I don't want the body - the `CXXCtorInitializer`s are not in it.
> > > > > I should use `hasBody()` and pass in an argument.
> > > > You are calling `Function-hasBody()` followed by 
> > > > `Function->getDefinition()`, which is what `FunctionDecl::getBody()` 
> > > > does. The difference is that `FunctionDecl::getBody()` will also load 
> > > > serialized bodies from the AST (if the body happens to be in a PCH, for 
> > > > instance).
> > > Do I want to load serialized bodies?
> > I would imagine you would want to deserialize, since you care about the 
> > contents of the body, not just the presence of one. This may or may not be 
> > important with modules (Richard Smith would be able to give a more 
> > definitive answer there).
> I would suggest writing minimal sane code that works until we have good 
> reasons to write more complex code to support module-enabled builds (and 
> appropriate tests).
I think either approach is minimal, sane code and we should understand why we 
shouldn't use the interface provided by `FunctionDecl::getBody()` for getting 
the definition before deciding to not use it.

I *think* we're fine to not use `getBody()` because the matcher should match 
over all of the function declarations with a definition, and so eventually it 
will find the definition with the actual body. In the case that the function 
definition is serialized, I think this already does the right thing and 
deserializes it. (I think the only case where we need the deserialization may 
be when we have a function declaration and we need to traverse from that to the 
function body, which may be in a different `FunctionDecl`.) So the use of 
`hasBody()` in the matcher above seems like the correct solution to me.


https://reviews.llvm.org/D28022



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


[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.

2016-12-22 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:719
+// The alias is defined in some namespace.
+assert(AliasQualifiedName.size() >= AliasName.size() + 2);
+llvm::StringRef AliasNs =

maybe `assert(StringRef(AliasQualifiedName).end_with("::"+AliasName))`?



Comment at: change-namespace/ChangeNamespace.cpp:741
 if (TargetDecl == FromDecl) {
   ReplaceName = FromDecl->getNameAsString();
   Matched = true;

Do we also need to check  `FromDecl->getNameAsString().size() < 
ReplaceName.size()` before assigning the name to `ReplaceName`?



Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:832
+ "}\n"
+ "namespace gl = glob;\n"
+ "namespace na {\n"

Add a testcase for the namespace alias decl with preceding `::`,  e.g. 
`namespace gl = ::glob`?


https://reviews.llvm.org/D28052



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


Re: r290297 - Add the alloc_size attribute to clang, attempt 2.

2016-12-22 Thread via cfe-commits

Hi George,
I'm guessing this may have caused PR31453.  Simple reproducer included 
in bug report.


 Chad

On 2016-12-21 21:50, George Burgess IV via cfe-commits wrote:

Author: gbiv
Date: Wed Dec 21 20:50:20 2016
New Revision: 290297

URL: http://llvm.org/viewvc/llvm-project?rev=290297&view=rev
Log:
Add the alloc_size attribute to clang, attempt 2.

This is a recommit of r290149, which was reverted in r290169 due to 
msan

failures. msan was failing because we were calling
`isMostDerivedAnUnsizedArray` on an invalid designator, which caused us
to read uninitialized memory. To fix this, the logic of the caller of
said function was simplified, and we now have a `!Invalid` assert in
`isMostDerivedAnUnsizedArray`, so we can catch this particular bug more
easily in the future.

Fingers crossed that this patch sticks this time. :)

Original commit message:

This patch does three things:
- Gives us the alloc_size attribute in clang, which lets us infer the
  number of bytes handed back to us by malloc/realloc/calloc/any user
  functions that act in a similar manner.
- Teaches our constexpr evaluator that evaluating some `const` 
variables

  is OK sometimes. This is why we have a change in
  test/SemaCXX/constant-expression-cxx11.cpp and other seemingly
  unrelated tests. Richard Smith okay'ed this idea some time ago in
  person.
- Uniques some Blocks in CodeGen, which was reviewed separately at
  D26410. Lack of uniquing only really shows up as a problem when
  combined with our new eagerness in the face of const.

Added:
cfe/trunk/test/CodeGen/alloc-size.c
cfe/trunk/test/CodeGenCXX/alloc-size.cpp
cfe/trunk/test/Sema/alloc-size.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGenCXX/block-in-ctor-dtor.cpp
cfe/trunk/test/CodeGenCXX/global-init.cpp
cfe/trunk/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=290297&r1=290296&r2=290297&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Dec 21 20:50:20 2016
@@ -780,6 +780,15 @@ def EmptyBases : InheritableAttr, Target
   let Documentation = [EmptyBasesDocs];
 }

+def AllocSize : InheritableAttr {
+  let Spellings = [GCC<"alloc_size">];
+  let Subjects = SubjectList<[HasFunctionProto], WarnDiag,
+ "ExpectedFunctionWithProtoType">;
+  let Args = [IntArgument<"ElemSizeParam">, 
IntArgument<"NumElemsParam", 1>];

+  let TemplateDependent = 1;
+  let Documentation = [AllocSizeDocs];
+}
+
 def EnableIf : InheritableAttr {
   let Spellings = [GNU<"enable_if">];
   let Subjects = SubjectList<[Function]>;

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=290297&r1=290296&r2=290297&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Dec 21 20:50:20 2016
@@ -206,6 +206,44 @@ to enforce the provided alignment assump
   }];
 }

+def AllocSizeDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``alloc_size`` attribute can be placed on functions that return 
pointers in
+order to hint to the compiler how many bytes of memory will be 
available at the

+returned poiner. ``alloc_size`` takes one or two arguments.
+
+- ``alloc_size(N)`` implies that argument number N equals the number 
of

+  available bytes at the returned pointer.
+- ``alloc_size(N, M)`` implies that the product of argument number N 
and
+  argument number M equals the number of available bytes at the 
returned

+  pointer.
+
+Argument numbers are 1-based.
+
+An example of how to use ``alloc_size``
+
+.. code-block:: c
+
+  void *my_malloc(int a) __attribute__((alloc_size(1)));
+  void *my_calloc(int a, int b) __attribute__((alloc_size(1, 2)));
+
+  int main() {
+void *const p = my_malloc(100);
+assert(__builtin_object_size(p, 0) == 100);
+void *const a = my_calloc(20, 5);
+assert(__builtin_object_size(a, 0) == 100);
+  }
+
+.. Note:: This attribute works differently in clang than it does in 
GCC.
+  Specifically, clang will only trace ``const`` pointers (as above); 
we give up
+  on pointers that are not marked as ``const``. In the vast majority 
of cases,

+  this is unimportant, because LLVM has support for 

[PATCH] D27424: Add the diagnose_if attribute to clang.

2016-12-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I really like this attribute, thank you for working on it!




Comment at: include/clang/Basic/Attr.td:1584
+def DiagnoseIf : InheritableAttr {
+  let Spellings = [GNU<"diagnose_if">];
+  let Subjects = SubjectList<[Function]>;

I think this should also have a C++ spelling under the clang namespace.



Comment at: include/clang/Basic/AttrDocs.td:359
+  int val3 = abs(val);
+  int val4 = must_abs(val); // because run-time checks are not emitted for
+// diagnose_if attributes, this executes without

Capitalize the B in because.



Comment at: include/clang/Basic/DiagnosticCommonKinds.td:164
   InGroup;
+def ext_clang_diagnose_if : Extension<"'diagnose_if' is a clang extension">,
+InGroup;

If we do add the clang namespaced spelling, we should have a test to ensure 
that this diagnostic is not triggered when the attribute is spelled 
`[[clang::diagnose_if(...)]]`.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2145
+def err_diagnose_if_invalid_diagnostic_type : Error<
+  "invalid diagnostic type for diagnose_if. Try 'error' or 'warning'.">;
 def err_constexpr_body_no_return : Error<

Diagnostics don't have full stops, so I think a better way to write this is: 
`invalid diagnostic type for 'diagnose_if'; use \"error\" or \"warning\" 
instead`.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3361
 "pass_object_size attribute">;
-def note_ovl_candidate_disabled_by_enable_if_attr : Note<
+def err_diagnose_if_succeeded: Error<"%0">;
+def warn_diagnose_if_succeeded : Warning<"%0">, InGroup;

Space before the colon.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4380
 InGroup;
+def note_from_diagnose_if : Note<"from diagnose_if attribute on %0:">;
 def warn_property_method_deprecated :

Please single quote the use of diagnose_if.



Comment at: include/clang/Sema/Overload.h:664
+
+/// Basically an TinyPtrVector that doesn't own the
+/// vector: If NumTriggeredDiagnoseIfs is 0 or 1, this is a DiagnoseIfAttr

an -> a



Comment at: include/clang/Sema/Sema.h:2616
+  DiagnoseIfAttr *
+  checkArgDependentDiagnoseIf(FunctionDecl *Function, ArrayRef Args,
+  SmallVectorImpl &Nonfatal,

Can this (and the other new functions) accept the `FunctionDecl` as a `const 
FunctionDecl *` instead?



Comment at: lib/Sema/SemaDeclAttr.cpp:857
+public:
+  ParmVarDeclChecker(FunctionDecl *FD) {
+Parms.insert(FD->param_begin(), FD->param_end());

This should accept `FD` as a const pointer.



Comment at: lib/Sema/SemaExpr.cpp:368
+
+if (DiagnoseIfAttr *A =
+checkArgIndependentDiagnoseIf(FD, DiagnoseIfWarnings)) {

`const DiagnoseIfAttr *`?



Comment at: lib/Sema/SemaExpr.cpp:388
+
+  for (DiagnoseIfAttr *W : DiagnoseIfWarnings)
+emitDiagnoseIfDiagnostic(Loc, W);

`const auto *`?



Comment at: lib/Sema/SemaExpr.cpp:5186
+
+  for (DiagnoseIfAttr *Attr : Nonfatal)
+S.emitDiagnoseIfDiagnostic(Fn->getLocStart(), Attr);

`const auto *`?



Comment at: lib/Sema/SemaOverload.cpp:827
   destroyCandidates();
-  ConversionSequenceAllocator.Reset();
-  NumInlineSequences = 0;
+  // DiagnoseIfAttrs are just pointers, so we don't need to destroy them.
+  SlabAllocator.Reset();

DiagnoseIfAttrs aren't the only thing allocated with the slab allocator though, 
right?  Since this is being generalized, I wonder if asserting somewhere that 
the slab allocator only deals with pointers would make sense?



Comment at: lib/Sema/SemaOverload.cpp:6151
+  bool MissingImplicitThis) {
+  auto EnableIfAttrs = getOrderedEnableIfAttrs(Function);
+  if (EnableIfAttrs.empty())

Please spell out the type instead of using `auto`, since the type isn't spelled 
in the initialization.



Comment at: lib/Sema/SemaOverload.cpp:6178-6179
+  SmallVectorImpl &Nonfatal) 
{
+  for (Attr *A : Function->attrs())
+if (auto *DIA = dyn_cast(A))
+  if (ArgDependent == DIA->getArgDependent()) {

Instead of doing this, you can use `specific_attrs()`



Comment at: lib/Sema/SemaOverload.cpp:9009
+static bool isCandidateUnavailableDueToDiagnoseIf(const OverloadCandidate &OC) 
{
+  return OC.NumTriggeredDiagnoseIfs == 1 &&
+ OC.DiagnoseIfInfo.get()->isError();

Can you run into a situation with stacked diagnose_if errors, so that there's > 
1?



Comment at: lib/Sema/SemaOverload.cp

[PATCH] D28058: [OpenCL] Correct ndrange_t implementation

2016-12-22 Thread Dmitry Borisenkov via Phabricator via cfe-commits
dmitry created this revision.
dmitry added a reviewer: yaxunl.
dmitry added subscribers: Anastasia, cfe-commits.

Since we don't have an ideal option for ndrange_t implementation (which was 
discussed there: https://reviews.llvm.org/D23086), I propose to stick with 
identification by name approach suggested by @Anastasia (solution 2 from the 
last comment of the discussion - 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160815/168540.html).

We take implementation of ndrange_t from lib/Headers/opencl-c.h and we perform 
identification of the type by its name.


https://reviews.llvm.org/D28058

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/AST/Type.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Headers/opencl-c.h
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  test/Headers/opencl-c-header.cl
  test/PCH/ocl_types.h
  test/SemaOpenCL/cl20-device-side-enqueue.cl
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1491,7 +1491,6 @@
   case BuiltinType::OCLEvent:
   case BuiltinType::OCLClkEvent:
   case BuiltinType::OCLQueue:
-  case BuiltinType::OCLNDRange:
   case BuiltinType::OCLReserveID:
 #define BUILTIN_TYPE(Id, SingletonId)
 #define SIGNED_TYPE(Id, SingletonId) case BuiltinType::Id:
Index: test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir-unknown-unknown" -verify -pedantic -fsyntax-only -DB32
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -triple "spir64-unknown-unknown" -verify -pedantic -fsyntax-only -Wconversion -DWCONV
 
+typedef struct {int a;} ndrange_t;
 // Diagnostic tests for different overloads of enqueue_kernel from Table 6.13.17.1 of OpenCL 2.0 Spec.
 kernel void enqueue_kernel_tests() {
   queue_t default_queue;
Index: test/PCH/ocl_types.h
===
--- test/PCH/ocl_types.h
+++ test/PCH/ocl_types.h
@@ -32,9 +32,6 @@
 // queue_t
 typedef queue_t q_t;
 
-// ndrange_t
-typedef ndrange_t range_t;
-
 // reserve_id_t
 typedef reserve_id_t reserveid_t;
 
Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -cl-std=CL1.1| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -cl-std=CL1.1| FileCheck %s
 
+// CHECK-NOT: ndrange_t
+// CHECK20: ndrange_t
 // CHECK: _Z16convert_char_rtec
 // CHECK-NOT: _Z3ctzc
 // CHECK20: _Z3ctzc
@@ -30,32 +32,30 @@
 
 // ===
 // Compile for OpenCL 2.0 for the first time. The module should change.
-// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
 // RUN: not diff %t/1_0.pcm %t/opencl_c.pcm
 // RUN: chmod u-w %t/opencl_c.pcm
 
 // ===
 // Compile for OpenCL 2.0 for the second time. The module should not change.
-// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck --check-prefix=CHECK20 --check-prefix=CHECK-MOD %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -O0 -emit-llvm -o - -cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-pa

Re: [clang-tools-extra] r290202 - [clang-tidy] Add modernize-use-default-member-init check

2016-12-22 Thread NAKAMURA Takumi via cfe-commits
Does the test invoke cl.exe? I supposed it were clang.

On Thu, Dec 22, 2016 at 11:24 PM Aaron Ballman 
wrote:

> On Wed, Dec 21, 2016 at 6:10 PM, NAKAMURA Takumi 
> wrote:
> > It'd be the issue if the test depended on installed headers. The builder
> > doesn't have MS headers installed.
>
> Those type names do not require any headers to be installed. They're
> keywords in C++11 and later, but MSVC did not support them until MSVC
> 2015.
>
> ~Aaron
>
> >
> > On Thu, Dec 22, 2016 at 1:27 AM Aaron Ballman 
> > wrote:
> >>
> >> On Tue, Dec 20, 2016 at 5:58 PM, Malcolm Parsons
> >>  wrote:
> >> > On 20 December 2016 at 22:32, Aaron Ballman 
> >> > wrote:
> >> >> On Tue, Dec 20, 2016 at 4:26 PM, Malcolm Parsons via cfe-commits
> >> >>  wrote:
> >> >>> Author: malcolm.parsons
> >> >>> Date: Tue Dec 20 15:26:07 2016
> >> >>> New Revision: 290202
> >> >>>
> >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=290202&view=rev
> >> >>> Log:
> >> >>> [clang-tidy] Add modernize-use-default-member-init check
> >> >>>
> >> >>> Summary: Fixes PR18858
> >> >>
> >> >> This appears to have broken one of the bots:
> >> >>
> >> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/33046
> >> >
> >> > error: unknown type name 'char16_t'
> >> > error: unknown type name 'char32_t'
> >> >
> >> > I see commented tests in other clang-tidy tests:
> >> >
> >> > // TODO: enable these tests once all supported compilers
> >> > // support char16_t and char32_t (VS2013 does not)
> >> >
> >> > // disabled for now until it is clear
> >> > // how to enable them in the test
> >> > //} catch (const char16_t*) {
> >>
> >> We no longer support MSVC 2013, so that should not be an issue.
> >> Takumi, I'm not certain what to make of this builder (MSVC centos?).
> >> What version of Visual Studio is it compiling with? Or is this an
> >> instance where we need to set the -fms-compatibility-version=19 when
> >> running the test with clang-cl?
> >>
> >> ~Aaron
> >>
> >> >
> >> > --
> >> > Malcolm Parsons
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang-tools-extra] r290202 - [clang-tidy] Add modernize-use-default-member-init check

2016-12-22 Thread Aaron Ballman via cfe-commits
On Thu, Dec 22, 2016 at 11:43 AM, NAKAMURA Takumi  wrote:
> Does the test invoke cl.exe? I supposed it were clang.

It shouldn't, but the test also does not specify a target triple, so
the behavior depends on what version of MSVC you built clang (and
clang-tidy) with. What version of MSVC is used to build that
configuration?

~Aaron

>
> On Thu, Dec 22, 2016 at 11:24 PM Aaron Ballman 
> wrote:
>>
>> On Wed, Dec 21, 2016 at 6:10 PM, NAKAMURA Takumi 
>> wrote:
>> > It'd be the issue if the test depended on installed headers. The builder
>> > doesn't have MS headers installed.
>>
>> Those type names do not require any headers to be installed. They're
>> keywords in C++11 and later, but MSVC did not support them until MSVC
>> 2015.
>>
>> ~Aaron
>>
>> >
>> > On Thu, Dec 22, 2016 at 1:27 AM Aaron Ballman 
>> > wrote:
>> >>
>> >> On Tue, Dec 20, 2016 at 5:58 PM, Malcolm Parsons
>> >>  wrote:
>> >> > On 20 December 2016 at 22:32, Aaron Ballman 
>> >> > wrote:
>> >> >> On Tue, Dec 20, 2016 at 4:26 PM, Malcolm Parsons via cfe-commits
>> >> >>  wrote:
>> >> >>> Author: malcolm.parsons
>> >> >>> Date: Tue Dec 20 15:26:07 2016
>> >> >>> New Revision: 290202
>> >> >>>
>> >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=290202&view=rev
>> >> >>> Log:
>> >> >>> [clang-tidy] Add modernize-use-default-member-init check
>> >> >>>
>> >> >>> Summary: Fixes PR18858
>> >> >>
>> >> >> This appears to have broken one of the bots:
>> >> >>
>> >> >> http://bb.pgr.jp/builders/ninja-x64-msvc-RA-centos6/builds/33046
>> >> >
>> >> > error: unknown type name 'char16_t'
>> >> > error: unknown type name 'char32_t'
>> >> >
>> >> > I see commented tests in other clang-tidy tests:
>> >> >
>> >> > // TODO: enable these tests once all supported compilers
>> >> > // support char16_t and char32_t (VS2013 does not)
>> >> >
>> >> > // disabled for now until it is clear
>> >> > // how to enable them in the test
>> >> > //} catch (const char16_t*) {
>> >>
>> >> We no longer support MSVC 2013, so that should not be an issue.
>> >> Takumi, I'm not certain what to make of this builder (MSVC centos?).
>> >> What version of Visual Studio is it compiling with? Or is this an
>> >> instance where we need to set the -fms-compatibility-version=19 when
>> >> running the test with clang-cl?
>> >>
>> >> ~Aaron
>> >>
>> >> >
>> >> > --
>> >> > Malcolm Parsons
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

How about standardizing on -disable-llvm-passes instead of 
-disable-llvm-optzns? I never liked "optzns" and can't remember how to spell 
it. Also, this flag disables LLVM instrumentation passes (ASan) as well as 
optimization passes, so I think -disable-llvm-passes is the better name.


https://reviews.llvm.org/D28047



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


[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.

2016-12-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 82345.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- addressed review comments.


https://reviews.llvm.org/D28052

Files:
  change-namespace/ChangeNamespace.cpp
  change-namespace/ChangeNamespace.h
  unittests/change-namespace/ChangeNamespaceTests.cpp

Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -825,6 +825,114 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, NamespaceAliasInGlobal) {
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace glob2 { class Glob2 {}; }\n"
+ "namespace gl = glob;\n"
+ "namespace gl2 = ::glob2;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { gl::Glob g; gl2::Glob2 g2; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected =
+  "namespace glob {\n"
+  "class Glob {};\n"
+  "}\n"
+  "namespace glob2 { class Glob2 {}; }\n"
+  "namespace gl = glob;\n"
+  "namespace gl2 = ::glob2;\n"
+  "\n"
+  "namespace x {\n"
+  "namespace y {\n"
+  "void f() { gl::Glob g; gl2::Glob2 g2; }\n"
+  "} // namespace y\n"
+  "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInNamespace) {
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace gl = glob;\n"
+ "void f() { gl::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace gl = glob;\n"
+ "void f() { gl::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInAncestorNamespace) {
+  NewNamespace = "na::nx";
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "namespace nb {\n"
+ "void f() { ga::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "\n"
+ "namespace nx {\n"
+ "void f() { ga::Glob g; }\n"
+ "} // namespace nx\n"
+ "} // namespace na\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, NamespaceAliasInOtherNamespace) {
+  std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "namespace nb {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace other { namespace gl = glob; }\n"
+ "namespace na {\n"
+ "namespace ga = glob;\n"
+ "\n"
+ "} // namespace na\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, UsingDeclAfterReference) {
   std::

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

The big change here is that `clang -O0` now applies the noinline attribute 
everywhere. I can see why someone might expect things to work that way, but it 
seems surprising to me at first glance.

Before this change I can imagine someone distributing a static archive of 
bitcode which could be used to produce debug binaries and release binaries. 
There are some issues with that (lack of lifetime markers), but it should 
mostly work.

Before this change, this command line would do inlining: `clang -S -emit-llvm 
foo.c -o - | opt -O2 -S` Now it won't, even though I didn't explicitly pass 
-O0. Are we happy with that?


https://reviews.llvm.org/D28053



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Renato Golin via Phabricator via cfe-commits
rengolin accepted this revision.
rengolin added a comment.
This revision is now accepted and ready to land.

My floating-point-fu is lacking, so I'll trust you guys that this makes sense. 
:)

I didn't see anything egregious, so LGTM with the nit inline.

cheers,
--renato




Comment at: lib/builtins/floatuntitf.c:73
+long_double_bits fb;
+fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */
+  | ((a >> 64) & 0xLL);  /* mantissa */

nit: an hexadecimal pattern here would be clearer. same above.


https://reviews.llvm.org/D27898



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


[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

In https://reviews.llvm.org/D28047#629746, @rnk wrote:

> How about standardizing on -disable-llvm-passes instead of 
> -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell 
> it. Also, this flag disables LLVM instrumentation passes (ASan) as well as 
> optimization passes, so I think -disable-llvm-passes is the better name.


I agree with that.
To be friendly with developers/users we could print an error mentioning to use 
`-disable-llvm-passes` when they try to use `-disable-llvm-optzns` (which we 
accept as a `-mllvm -disable-llvm-optzns` hardcoded in the driver by the way).


https://reviews.llvm.org/D28047



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


[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

> It would be awesome if attribute sets were a bit more FileCheck friendly, but 
> oh well.

I've been wondering about that, what's the point of attribute sets in the 
textual IR?
I understand the idea for saving space in the Bitcode, but the IR does not have 
to use the same representation.


https://reviews.llvm.org/D28053



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


r290352 - [analyzer] Update GTestChecker to tighten API detection

2016-12-22 Thread Devin Coughlin via cfe-commits
Author: dcoughlin
Date: Thu Dec 22 11:52:57 2016
New Revision: 290352

URL: http://llvm.org/viewvc/llvm-project?rev=290352&view=rev
Log:
[analyzer] Update GTestChecker to tighten API detection

Update the GTestChecker to tighten up the API detection and make it
cleaner in response to post-commit feedback. Also add tests for when
temporary destructors are enabled to make sure we get the expected behavior
when inlining constructors for temporaries.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
cfe/trunk/test/Analysis/gtest.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp?rev=290352&r1=290351&r2=290352&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp Thu Dec 22 11:52:57 
2016
@@ -7,7 +7,7 @@
 //
 
//===--===//
 //
-// This checker models the behavior of un-inlined APIs from the the gtest
+// This checker models the behavior of un-inlined APIs from the gtest
 // unit-testing library to avoid false positives when using assertions from
 // that library.
 //
@@ -85,7 +85,7 @@ using namespace ento;
 // does not inline these since it does not yet reliably call temporary
 // destructors).
 //
-// This checker compensates for the missing inlining by propgagating the
+// This checker compensates for the missing inlining by propagating the
 // _success value across the bool and copy constructors so the assertion 
behaves
 // as expected.
 
@@ -102,7 +102,7 @@ public:
 
 private:
   void modelAssertionResultBoolConstructor(const CXXConstructorCall *Call,
-   CheckerContext &C) const;
+   bool IsRef, CheckerContext &C) 
const;
 
   void modelAssertionResultCopyConstructor(const CXXConstructorCall *Call,
CheckerContext &C) const;
@@ -122,35 +122,25 @@ private:
 
 GTestChecker::GTestChecker() : AssertionResultII(nullptr), SuccessII(nullptr) 
{}
 
-/// Model a call to an un-inlined AssertionResult(boolean expression).
+/// Model a call to an un-inlined AssertionResult(bool) or
+/// AssertionResult(bool &, ...).
 /// To do so, constrain the value of the newly-constructed instance's 
'success_'
 /// field to be equal to the passed-in boolean value.
+///
+/// \param IsRef Whether the boolean parameter is a reference or not.
 void GTestChecker::modelAssertionResultBoolConstructor(
-const CXXConstructorCall *Call, CheckerContext &C) const {
-  assert(Call->getNumArgs() > 0);
-
-  // Depending on the version of gtest the constructor can be either of:
-  //
-  // v1.7 and earlier:
-  //  AssertionResult(bool success)
-  //
-  // v1.8 and greater:
-  //  template 
-  //  AssertionResult(const T& success,
-  //  typename internal::EnableIf<
-  //  !internal::ImplicitlyConvertible::value>::type*)
+const CXXConstructorCall *Call, bool IsRef, CheckerContext &C) const {
+  assert(Call->getNumArgs() >= 1 && Call->getNumArgs() <= 2);
 
+  ProgramStateRef State = C.getState();
   SVal BooleanArgVal = Call->getArgSVal(0);
-  if (Call->getDecl()->getParamDecl(0)->getType()->getAs()) {
-// We have v1.8+, so load the value from the reference.
+  if (IsRef) {
+// The argument is a reference, so load from it to get the boolean value.
 if (!BooleanArgVal.getAs())
   return;
 BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs());
   }
 
-  ProgramStateRef State = C.getState();
-
   SVal ThisVal = Call->getCXXThisVal();
 
   SVal ThisSuccess = getAssertionResultSuccessFieldValue(
@@ -169,7 +159,7 @@ void GTestChecker::modelAssertionResultB
 /// 'success_' field.
 void GTestChecker::modelAssertionResultCopyConstructor(
 const CXXConstructorCall *Call, CheckerContext &C) const {
-  assert(Call->getNumArgs() > 0);
+  assert(Call->getNumArgs() == 1);
 
   // The first parameter of the the copy constructor must be the other
   // instance to initialize this instances fields from.
@@ -206,22 +196,44 @@ void GTestChecker::checkPostCall(const C
   if (CtorParent->getIdentifier() != AssertionResultII)
 return;
 
-  if (CtorDecl->getNumParams() == 0)
-return;
-
+  unsigned ParamCount = CtorDecl->getNumParams();
 
-  // Call the appropriate modeling method based on the type of the first
-  // constructor parameter.
-  const ParmVarDecl *ParamDecl = CtorDecl->getParamDecl(0);
-  QualType ParamType = ParamDecl->getType();
-  if (CtorDecl->getNumParams() <= 2 &&
-  ParamType.getNonReferenceType()->getCanonicalTypeUnqualified() ==
-  C.getASTContext().BoolTy) {
-// The first parameter is either a boolean or reference to a boolean
-modelAssertionRes

[PATCH] D27773: [analyzer] Add checker modeling gtest APIs.

2016-12-22 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added a comment.

I cleaned up the parameter detection and added more tests in r290352.


https://reviews.llvm.org/D27773



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


[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

In https://reviews.llvm.org/D28053#629768, @rnk wrote:

> The big change here is that `clang -O0` now applies the noinline attribute 
> everywhere. I can see why someone might expect things to work that way, but 
> it seems surprising to me at first glance.
>
> Before this change I can imagine someone distributing a static archive of 
> bitcode which could be used to produce debug binaries and release binaries. 
> There are some issues with that (lack of lifetime markers), but it should 
> mostly work.


Note that LTO runs a bunch of optimizations during the compile phase that you 
wouldn't get with that.

Steven had issues in the past when mixing O0 and 
https://reviews.llvm.org/owners/package/2/ in LTO, I don't remember the details 
unfortunately (and Steven is OOO).
Right now we don't add the `optnone` attribute when building with O0, maybe we 
should as well if we go this route.


https://reviews.llvm.org/D28053



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: lib/builtins/floatuntitf.c:73
+long_double_bits fb;
+fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */
+  | ((a >> 64) & 0xLL);  /* mantissa */

rengolin wrote:
> nit: an hexadecimal pattern here would be clearer. same above.
Do you mean something like: `(foo << 48) & 0x`?


https://reviews.llvm.org/D27898



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


[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:453
+// There is no effect at O0 when we won't inline anyways.
+if (Opts.OptimizationLevel > 1) {
+  const Option &InlineOpt = InlineArg->getOption();

I'd switch the two if


https://reviews.llvm.org/D28053



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Renato Golin via Phabricator via cfe-commits
rengolin added inline comments.



Comment at: lib/builtins/floatuntitf.c:73
+long_double_bits fb;
+fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */
+  | ((a >> 64) & 0xLL);  /* mantissa */

mgorny wrote:
> rengolin wrote:
> > nit: an hexadecimal pattern here would be clearer. same above.
> Do you mean something like: `(foo << 48) & 0x`?
No, just the `16383` to `0x3FFF`


https://reviews.llvm.org/D27898



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Renato Golin via Phabricator via cfe-commits
rengolin added inline comments.



Comment at: lib/builtins/floatuntitf.c:73
+long_double_bits fb;
+fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */
+  | ((a >> 64) & 0xLL);  /* mantissa */

rengolin wrote:
> mgorny wrote:
> > rengolin wrote:
> > > nit: an hexadecimal pattern here would be clearer. same above.
> > Do you mean something like: `(foo << 48) & 0x`?
> No, just the `16383` to `0x3FFF`
Though, now I think your proposal may be better. :)

Whatever works, I'm not too concerned about that.


https://reviews.llvm.org/D27898



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Steve Canon via Phabricator via cfe-commits
scanon added a comment.

Please s/mantissa/significand/. I know that "mantissa" is used in a number of 
places in llvm sources, but it's incorrect terminology. Significand is the 
IEEE-754 nomenclature, which any new work should follow.


https://reviews.llvm.org/D27898



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


[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:453
+// There is no effect at O0 when we won't inline anyways.
+if (Opts.OptimizationLevel > 1) {
+  const Option &InlineOpt = InlineArg->getOption();

mehdi_amini wrote:
> I'd switch the two if
The test `> 1` and the comment about `-O0` are confusing to me as well. What 
about the following (IIUC):

```
if (Opts.OptimizationLevel <= 1) {
  // "always-inline" required for correctness at O0 and O1.
  Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining);
else if (Opts.OptimizationLevel > 1) {
  // Normal inlining at O2 and above
  Opts.setInlining((CodeGenOptions::NormalInlining);
  // -fno-inline-functions overrides
  if (Arg *InlineArg = Args.getLastArg(
  options::OPT_finline_functions, options::OPT_finline_hint_functions,
  options::OPT_fno_inline_functions, options::OPT_fno_inline)) {
 if (InlineOpt.matches(options::OPT_finline_functions))
Opts.setInlining(CodeGenOptions::NormalInlining);
  else if (InlineOpt.matches(options::OPT_finline_hint_functions))
Opts.setInlining(CodeGenOptions::OnlyHintInlining);
  else
Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining);
  }
}
```



https://reviews.llvm.org/D28053



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Steve Canon via Phabricator via cfe-commits
scanon added inline comments.



Comment at: lib/builtins/floattitf.c:65
+if (a & ((tu_int)1 << LDBL_MANT_DIG)) {
+a >>= 1;
+++e;

Strictly speaking there's no need to adjust `a` here. If we rounded up into a 
new binade, then `a` is necessarily `0b1000...0`, and the leading 1 bit will 
get killed by the mask when we assemble `fb.u.high.all` regardless of its 
position. Same comment applies to floatuntitf.


https://reviews.llvm.org/D27898



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: lib/builtins/floatuntitf.c:73
+long_double_bits fb;
+fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */
+  | ((a >> 64) & 0xLL);  /* mantissa */

rengolin wrote:
> rengolin wrote:
> > mgorny wrote:
> > > rengolin wrote:
> > > > nit: an hexadecimal pattern here would be clearer. same above.
> > > Do you mean something like: `(foo << 48) & 0x`?
> > No, just the `16383` to `0x3FFF`
> Though, now I think your proposal may be better. :)
> 
> Whatever works, I'm not too concerned about that.
Well, I used the decimal form because that's how the IEEE 754 standard 
specifies it, so I guessed the correlation would be clearer that way.


https://reviews.llvm.org/D27898



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


[PATCH] D27898: [compiler-rt] [builtins] Implement __floattitf() & __floatuntitf()

2016-12-22 Thread Steve Canon via Phabricator via cfe-commits
scanon added inline comments.



Comment at: lib/builtins/floatuntitf.c:73
+long_double_bits fb;
+fb.u.high.all = (du_int)(e + 16383) << 48/* exponent */
+  | ((a >> 64) & 0xLL);  /* mantissa */

mgorny wrote:
> rengolin wrote:
> > rengolin wrote:
> > > mgorny wrote:
> > > > rengolin wrote:
> > > > > nit: an hexadecimal pattern here would be clearer. same above.
> > > > Do you mean something like: `(foo << 48) & 0x`?
> > > No, just the `16383` to `0x3FFF`
> > Though, now I think your proposal may be better. :)
> > 
> > Whatever works, I'm not too concerned about that.
> Well, I used the decimal form because that's how the IEEE 754 standard 
> specifies it, so I guessed the correlation would be clearer that way.
FWIW, I think both are pretty obvious, but if you want to be totally explicit:

const int bias = 0x3fff;
... (e + bias) ...


https://reviews.llvm.org/D27898



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


r290353 - Make alloc_size only applicable to Functions.

2016-12-22 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Thu Dec 22 12:48:34 2016
New Revision: 290353

URL: http://llvm.org/viewvc/llvm-project?rev=290353&view=rev
Log:
Make alloc_size only applicable to Functions.

I don't remember why I didn't make alloc_size only applicable to
Functions a year ago, but I can't see any compelling reason not to do
so now.

Fixes PR31453.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/test/Sema/alloc-size.c

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=290353&r1=290352&r2=290353&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Dec 22 12:48:34 2016
@@ -782,8 +782,7 @@ def EmptyBases : InheritableAttr, Target
 
 def AllocSize : InheritableAttr {
   let Spellings = [GCC<"alloc_size">];
-  let Subjects = SubjectList<[HasFunctionProto], WarnDiag,
- "ExpectedFunctionWithProtoType">;
+  let Subjects = SubjectList<[Function]>;
   let Args = [IntArgument<"ElemSizeParam">, IntArgument<"NumElemsParam", 1>];
   let TemplateDependent = 1;
   let Documentation = [AllocSizeDocs];

Modified: cfe/trunk/test/Sema/alloc-size.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/alloc-size.c?rev=290353&r1=290352&r2=290353&view=diff
==
--- cfe/trunk/test/Sema/alloc-size.c (original)
+++ cfe/trunk/test/Sema/alloc-size.c Thu Dec 22 12:48:34 2016
@@ -14,10 +14,12 @@ void *fail8(int a, int b) __attribute__(
 
 int fail9(int a) __attribute__((alloc_size(1))); 
//expected-warning{{'alloc_size' attribute only applies to return values that 
are pointers}}
 
-int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' 
attribute only applies to non-K&R-style functions}}
+int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' 
attribute only applies to functions}}
 
 void *fail11(void *a) __attribute__((alloc_size(1))); 
//expected-error{{'alloc_size' attribute argument may only refer to a function 
parameter of integer type}}
 
 void *fail12(int a) __attribute__((alloc_size("abc"))); 
//expected-error{{'alloc_size' attribute requires parameter 1 to be an integer 
constant}}
 void *fail12(int a) __attribute__((alloc_size(1, "abc"))); 
//expected-error{{'alloc_size' attribute requires parameter 2 to be an integer 
constant}}
 void *fail13(int a) __attribute__((alloc_size(1U<<31))); 
//expected-error{{integer constant expression evaluates to value 2147483648 
that cannot be represented in a 32-bit signed integer type}}
+
+int (*PR31453)(int) __attribute__((alloc_size(1))); 
//expected-warning{{'alloc_size' attribute only applies to functions}}


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


r290356 - Fix warning introduced by r290297.

2016-12-22 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Thu Dec 22 13:00:31 2016
New Revision: 290356

URL: http://llvm.org/viewvc/llvm-project?rev=290356&view=rev
Log:
Fix warning introduced by r290297.

Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=290356&r1=290355&r2=290356&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Dec 22 13:00:31 2016
@@ -256,7 +256,7 @@ static bool checkPositiveIntArgument(Sem
   if (!checkUInt32Argument(S, Attr, Expr, UVal, Idx))
 return false;
 
-  if (UVal > std::numeric_limits::max()) {
+  if (UVal > (uint32_t)std::numeric_limits::max()) {
 llvm::APSInt I(32); // for toString
 I = UVal;
 S.Diag(Expr->getExprLoc(), diag::err_ice_too_large)


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


Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.

2016-12-22 Thread Dimitry Andric via cfe-commits
On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits 
 wrote:
> Author: chandlerc
> Date: Tue Dec 20 02:28:19 2016
> New Revision: 290169
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev
> Log:
> Revert r290149: Add the alloc_size attribute to clang.
> 
> This commit fails MSan when running test/CodeGen/object-size.c in
> a confusing way. After some discussion with George, it isn't really
> clear what is going on here. We can make the MSan failure go away by
> testing for the invalid bit, but *why* things are invalid isn't clear.
> And yet, other code in the surrounding area is doing precisely this and
> testing for invalid.
> 
> George is going to take a closer look at this to better understand the
> nature of the failure and recommit it, for now backing it out to clean
> up MSan builds.

Hmm, was this reapplied later on?  I'm still getting the following 
AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 as 
the cause:

FAIL: AddressSanitizer-Unit :: 
Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest (2124 of 30204)
 TEST 'AddressSanitizer-Unit :: 
Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED 

Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from AddressSanitizer
[ RUN  ] AddressSanitizer.ReallocFreedPointerTest
/share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: 
Failure
Death test: ptr = realloc(ptr, 77)
Result: failed to die.
 Error msg:
[  DEATH   ]
[  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (30 ms)
[--] 1 test from AddressSanitizer (30 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (31 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] AddressSanitizer.ReallocFreedPointerTest

 1 FAILED TEST
  YOU HAVE 24 DISABLED TESTS



Testing: 0 .
FAIL: AddressSanitizer-Unit :: 
Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest (2233 of 
30204)
 TEST 'AddressSanitizer-Unit :: 
Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED 

Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from AddressSanitizer
[ RUN  ] AddressSanitizer.ReallocFreedPointerTest
/share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: 
Failure
Death test: ptr = realloc(ptr, 77)
Result: failed to die.
 Error msg:
[  DEATH   ]
[  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (24 ms)
[--] 1 test from AddressSanitizer (24 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (25 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] AddressSanitizer.ReallocFreedPointerTest

 1 FAILED TEST
  YOU HAVE 24 DISABLED TESTS



Testing: 0 .
FAIL: AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc 
(2399 of 30204)
 TEST 'AddressSanitizer-i386-freebsd :: 
TestCases/Posix/free_hook_realloc.cc' FAILED 
Script:
--
/home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/./bin/clang 
--driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer 
-fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m32 -O2 
/share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc
 -o 
/home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/projects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp
/home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/projects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp
 2>&1 | FileCheck 
/share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc
--
Exit Code: 2

Command Output (stderr):
--
FileCheck error: '-' is empty.
FileCheck command line:  FileCheck 
/share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc

--


Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
Testing Time: 880.55s

Failing Tests (3):
AddressSanitizer-Unit :: 
Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest
AddressSanitizer-Unit :: 
Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest
AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc

  Expected Passes: 26881
  Expected Failures  : 144
  Unsupported Tests  : 3176
  Unexpected Failures: 3

1 warning(s) in tests.

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPG

[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D28047#629824, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28047#629746, @rnk wrote:
>
> > How about standardizing on -disable-llvm-passes instead of 
> > -disable-llvm-optzns? I never liked "optzns" and can't remember how to 
> > spell it. Also, this flag disables LLVM instrumentation passes (ASan) as 
> > well as optimization passes, so I think -disable-llvm-passes is the better 
> > name.
>
>
> I agree with that.
>  To be friendly with developers/users we could print an error mentioning to 
> use `-disable-llvm-passes` when they try to use `-disable-llvm-optzns` (which 
> we accept as a `-mllvm -disable-llvm-optzns` hardcoded in the driver by the 
> way).


I only have a mild preference for how we spell the name based on the fact that 
i've told people to use -disable-llvm-optzns for years. If we want to go with 
...-passes, fine with me.

I'll probably just add an alias so that you can use either spelling, but change 
the tests to consolidate on whichever spelling we end up with.

SG? I can regenerate the patch if desired, but it'll just be a lot of perl...


https://reviews.llvm.org/D28047



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


[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D28053#629830, @mehdi_amini wrote:

> > It would be awesome if attribute sets were a bit more FileCheck friendly, 
> > but oh well.
>
> I've been wondering about that, what's the point of attribute sets in the 
> textual IR?
>  I understand the idea for saving space in the Bitcode, but the IR does not 
> have to use the same representation.


Will return to the actual patch review, but as an aside, I would *strongly* 
support going back to printing attributes in the canonical location in the 
function declaration / definition. It has huge advantages:

- Can FileCheck individual function's attributes rather than a set at a time
- Can place those checks after a CHECK-LABEL for the function name
- Can have a CHECK-LABEL for the function name, CHECKs for attributes *and* 
CHECKs for function body stuff, which today is extremely hard to combine into a 
single test.

Happy to help with this if you want to pursue it.


https://reviews.llvm.org/D28053



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


[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

LGTM, but please wait for @rnk  to confirm :)

Also any opinion about a driver option -emit-raw-llvm as @joerg suggested?


https://reviews.llvm.org/D28047



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


[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In https://reviews.llvm.org/D28047#629889, @chandlerc wrote:

> I only have a mild preference for how we spell the name based on the fact 
> that i've told people to use -disable-llvm-optzns for years. If we want to go 
> with ...-passes, fine with me.


We have some freedom to change the -cc1 interface. I liked being able to pass 
-g to -cc1, but then we went to -debug-info-kind=(limited|full|linetables) or 
whatever. =?

> I'll probably just add an alias so that you can use either spelling, but 
> change the tests to consolidate on whichever spelling we end up with.
> 
> SG? I can regenerate the patch if desired, but it'll just be a lot of perl...

Sounds good! We definitely don't need both -disable-llvm-optzns and 
-disable-llvm-passes.


https://reviews.llvm.org/D28047



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


Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.

2016-12-22 Thread George Burgess IV via cfe-commits
Yes, this was reapplied in r290297 with fixes for the msan issue we caught;
these asan unit test failures are news to me. Can you give me the command
that you're using to run these tests, please?

On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric  wrote:

> On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> > Author: chandlerc
> > Date: Tue Dec 20 02:28:19 2016
> > New Revision: 290169
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev
> > Log:
> > Revert r290149: Add the alloc_size attribute to clang.
> >
> > This commit fails MSan when running test/CodeGen/object-size.c in
> > a confusing way. After some discussion with George, it isn't really
> > clear what is going on here. We can make the MSan failure go away by
> > testing for the invalid bit, but *why* things are invalid isn't clear.
> > And yet, other code in the surrounding area is doing precisely this and
> > testing for invalid.
> >
> > George is going to take a closer look at this to better understand the
> > nature of the failure and recommit it, for now backing it out to clean
> > up MSan builds.
>
> Hmm, was this reapplied later on?  I'm still getting the following
> AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149
> as the cause:
>
> FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressS
> anitizer.ReallocFreedPointerTest (2124 of 30204)
>  TEST 'AddressSanitizer-Unit ::
> Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED
> 
> Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from AddressSanitizer
> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
> Failure
> Death test: ptr = realloc(ptr, 77)
> Result: failed to die.
>  Error msg:
> [  DEATH   ]
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (30 ms)
> [--] 1 test from AddressSanitizer (30 ms total)
>
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (31 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest
>
>  1 FAILED TEST
>   YOU HAVE 24 DISABLED TESTS
>
>
> 
> Testing: 0 .
> FAIL: AddressSanitizer-Unit :: Asan-i386-with-calls-Test/Addr
> essSanitizer.ReallocFreedPointerTest (2233 of 30204)
>  TEST 'AddressSanitizer-Unit ::
> Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest'
> FAILED 
> Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from AddressSanitizer
> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
> Failure
> Death test: ptr = realloc(ptr, 77)
> Result: failed to die.
>  Error msg:
> [  DEATH   ]
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (24 ms)
> [--] 1 test from AddressSanitizer (24 ms total)
>
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (25 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest
>
>  1 FAILED TEST
>   YOU HAVE 24 DISABLED TESTS
>
>
> 
> Testing: 0 .
> FAIL: AddressSanitizer-i386-freebsd :: TestCases/Posix/free_hook_realloc.cc
> (2399 of 30204)
>  TEST 'AddressSanitizer-i386-freebsd ::
> TestCases/Posix/free_hook_realloc.cc' FAILED 
> Script:
> --
> /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/./bin/clang
> --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer
> -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m32
> -O2 /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/Tes
> tCases/Posix/free_hook_realloc.cc -o /home/dim/obj/llvm-290338-trun
> k-freebsd12-i386-ninja-rel-1/projects/compiler-rt/test/asan/
> I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp
> /home/dim/obj/llvm-290338-trunk-freebsd12-i386-ninja-rel-1/p
> rojects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/
> Posix/Output/free_hook_realloc.cc.tmp 2>&1 | FileCheck
> /share/dim/src/llvm/trunk/projects/compiler-rt/test/asan/Tes
> tCases/Posix/free_hook_realloc.cc
> --
> Exit Code: 2
>
> Command Output (stderr):
> --
> FileCheck error: '-' is empty.
> FileCheck command line:  FileCheck /share/dim/src/llvm/trunk/proj
> ects/compiler-rt/test/asan/TestCases/Posix/free_hook_realloc.cc
>
> --
>
> 
> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
> Testing Time: 880.55s
> 

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc updated this revision to Diff 82356.
chandlerc added a comment.
Herald added a subscriber: wdng.

Fix an thinko found in review, clean up comments, and clean up pass pipeline
selection as that actually needs to happen in this patch as well.

Also update one test using -O1 and observing this, including making it use
FileCheck more effectively.


https://reviews.llvm.org/D28053

Files:
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CXX/special/class.dtor/p3-0x.cpp
  test/CodeGen/2008-04-08-NoExceptions.c
  test/CodeGen/address-safety-attr-kasan.cpp
  test/CodeGen/address-safety-attr.cpp
  test/CodeGen/address-space-field1.c
  test/CodeGen/alias.c
  test/CodeGen/attr-minsize.cpp
  test/CodeGen/attributes.c
  test/CodeGen/incomplete-function-type-2.c
  test/CodeGen/inline-optim.c
  test/CodeGen/mips16-attr.c
  test/CodeGen/mrtd.c
  test/CodeGen/ms-declspecs.c
  test/CodeGen/ppc64-complex-parms.c
  test/CodeGen/ppc64-complex-return.c
  test/CodeGen/ppc64-extend.c
  test/CodeGen/sanitize-thread-attr.cpp
  test/CodeGen/sanitize-thread-no-checking-at-run-time.m
  test/CodeGen/unwind-attr.c
  test/CodeGenCXX/attr.cpp
  test/CodeGenCXX/cxx11-exception-spec.cpp
  test/CodeGenCXX/cxx11-noreturn.cpp
  test/CodeGenCXX/derived-to-base.cpp
  test/CodeGenCXX/global-dtor-no-atexit.cpp
  test/CodeGenCXX/global-init.cpp
  test/CodeGenCXX/inline-hint.cpp
  test/CodeGenCXX/main-norecurse.cpp
  test/CodeGenCXX/microsoft-abi-array-cookies.cpp
  test/CodeGenCXX/no-exceptions.cpp
  test/CodeGenCXX/optnone-class-members.cpp
  test/CodeGenCXX/optnone-def-decl.cpp
  test/CodeGenCXX/reference-cast.cpp
  test/CodeGenCXX/threadsafe-statics.cpp
  test/CodeGenCXX/thunks.cpp
  test/CodeGenCXX/virtual-base-cast.cpp
  test/CodeGenObjC/gnu-exceptions.m
  test/CodeGenObjC/objc-literal-tests.m
  test/CodeGenObjCXX/lambda-expressions.mm
  test/CodeGenOpenCL/amdgpu-attrs.cl
  test/Driver/darwin-iphone-defaults.m
  test/PCH/objc_container.m

Index: test/PCH/objc_container.m
===
--- test/PCH/objc_container.m
+++ test/PCH/objc_container.m
@@ -21,5 +21,5 @@
 // CHECK-IR: {{call.*objc_msgSend}}
 // CHECK-IR: ret void
 
-// CHECK-IR: attributes #0 = { nounwind {{.*}} }
+// CHECK-IR: attributes #0 = { noinline nounwind {{.*}} }
 // CHECK-IR: attributes #1 = { nonlazybind }
Index: test/Driver/darwin-iphone-defaults.m
===
--- test/Driver/darwin-iphone-defaults.m
+++ test/Driver/darwin-iphone-defaults.m
@@ -26,4 +26,4 @@
   [I1 alloc];
 }
 
-// CHECK: attributes [[F0]] = { ssp{{.*}} }
+// CHECK: attributes [[F0]] = { noinline ssp{{.*}} }
Index: test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- test/CodeGenOpenCL/amdgpu-attrs.cl
+++ test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -141,26 +141,26 @@
 // CHECK-NOT: "amdgpu-num-sgpr"="0"
 // CHECK-NOT: "amdgpu-num-vgpr"="0"
 
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { nounwind "amdgpu-flat-work-group-size"="32,64"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { nounwind "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { nounwind "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32]] = { nounwind "amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[NUM_VGPR_64]] = { nounwind "amdgpu-num-vgpr"="64"
+// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { noinline nounwind "amdgpu-flat-work-group-size"="32,64"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { noinline nounwind "amdgpu-waves-per-eu"="2"
+// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { noinline nounwind "amdgpu-waves-per-eu"="2,4"
+// CHECK-DAG: attributes [[NUM_SGPR_32]] = { noinline nounwind "amdgpu-num-sgpr"="32"
+// CHECK-DAG: attributes [[NUM_VGPR_64]] = { noinline nounwind "amdgpu-num-vgpr"="64"
 
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_SGPR_32]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_VGPR_64]] = { nounwind "amdgpu-flat-work-group-size"="32,64" "amdgpu-num-vgpr"="64"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { nounwind "amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { nounwind "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D28053#629834, @mehdi_amini wrote:

> In https://reviews.llvm.org/D28053#629768, @rnk wrote:
>
> > The big change here is that `clang -O0` now applies the noinline attribute 
> > everywhere. I can see why someone might expect things to work that way, but 
> > it seems surprising to me at first glance.
> >
> > Before this change I can imagine someone distributing a static archive of 
> > bitcode which could be used to produce debug binaries and release binaries. 
> > There are some issues with that (lack of lifetime markers), but it should 
> > mostly work.
>
>
> Note that LTO runs a bunch of optimizations during the compile phase that you 
> wouldn't get with that.


There are other issues with this workflow as well. For example, Clang at -O0 
will intentionally reuse local allocas to save stack space without the backend 
having to track lifetime. This destroys aliasing information left and right 
though, so you really wouldn't want this for a mixed-use bitcode archive.

I think a much more effective technique (if folks actually want to do this) 
would be (with whatever flag spelling you want)

  % clang -O -emit-llvm -mllvm -disable-llvm-passes

This will generate a frontend-optimized but backend pristine bitcode file that 
can be processed more or less depending on the desire of the user...

But I also don't think we really need to do a ton of work to support this 
hypothetical. If someone really wants to make this work, they should 
specifically request some mode and we should surface a clear flag documented to 
provide it. Joerg's idea about '-emit-raw-llvm' would be a nice step in this 
direction IMO.

> Steven had issues in the past when mixing O0 and 
> https://reviews.llvm.org/owners/package/2/ in LTO, I don't remember the 
> details unfortunately (and Steven is OOO).
>  Right now we don't add the `optnone` attribute when building with O0, maybe 
> we should as well if we go this route.

Fundamentally, yes. This patch is trying to follow the path we've been going 
down where we try to propagate optimization restrictions to LTO. We do this for 
-Os and -Oz, but not -O0. This propagates one aspect (inlining), but I would be 
interested in using optnone to more effectively propagate O0.

That said, I agree with Reid that it is a bit surprising. After thinking about 
it a lot, the reason I personally find it surprising is actually that I find 
-O0 being the default surprising. But I'm OK with not changing that here, and 
maybe never changing that at the CC1 layer.

Thoughts? Generally happy with this direction?

There is another semantic change here that I intended but Mehdi highlighted I 
failed to fully implement: I'm also *not* using 'noinline' at -O1, and instead 
just building a simpler pass pipeline. I think this better fits with the 
intent: -O1 vs -O2 vs -O3 don't really change the *target* of optimization but 
instead *how much* optimization to do. As such, they feel like they don't need 
to be in attributes where -O0, -Os, and -Oz do.




Comment at: lib/Frontend/CompilerInvocation.cpp:453
+// There is no effect at O0 when we won't inline anyways.
+if (Opts.OptimizationLevel > 1) {
+  const Option &InlineOpt = InlineArg->getOption();

mehdi_amini wrote:
> mehdi_amini wrote:
> > I'd switch the two if
> The test `> 1` and the comment about `-O0` are confusing to me as well. What 
> about the following (IIUC):
> 
> ```
> if (Opts.OptimizationLevel <= 1) {
>   // "always-inline" required for correctness at O0 and O1.
>   Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining);
> else if (Opts.OptimizationLevel > 1) {
>   // Normal inlining at O2 and above
>   Opts.setInlining((CodeGenOptions::NormalInlining);
>   // -fno-inline-functions overrides
>   if (Arg *InlineArg = Args.getLastArg(
>   options::OPT_finline_functions, options::OPT_finline_hint_functions,
>   options::OPT_fno_inline_functions, options::OPT_fno_inline)) {
>  if (InlineOpt.matches(options::OPT_finline_functions))
> Opts.setInlining(CodeGenOptions::NormalInlining);
>   else if (InlineOpt.matches(options::OPT_finline_hint_functions))
> Opts.setInlining(CodeGenOptions::OnlyHintInlining);
>   else
> Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining);
>   }
> }
> ```
> 
Yea, this was confusing, and I hadn't actually implemented my intent here 
either.

My goal was to make the `Inlining` setting correspond to whether we *forcibly* 
disable some aspects of inlining or not, rather than anything to do with what 
inliner pass we run.

This means we should only be setting `OnlyAlwaysInlining` in O0, not at O1, and 
the O1 behavior should be up to the backend's construction of a pass pipeline 
appropriate to that optimization level. (I actually think using the always 
inliner is a mistake here, but that's a separate discussion.)

To make all of this work requires me to merge in the next pat

[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

In https://reviews.llvm.org/D28047#629892, @mehdi_amini wrote:

> LGTM, but please wait for @rnk  to confirm :)
>
> Also any opinion about a driver option -emit-raw-llvm as @joerg suggested?


I like the idea generally, but I don't have an immediate use case and would 
defer that to a subsequent patch authored by someone who wants this? I really 
only wanted to remove the divergence between two flags here.


https://reviews.llvm.org/D28047



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


r290364 - [OPENMP] Fix for PR31417: assert failure when compiling trivial openmp

2016-12-22 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Dec 22 13:44:05 2016
New Revision: 290364

URL: http://llvm.org/viewvc/llvm-project?rev=290364&view=rev
Log:
[OPENMP] Fix for PR31417: assert failure when compiling trivial openmp
program

Offload related code is not quite ready yet, but some simple examples
must not crash the compiler. Patch fixes the problem in offloading code
with exceptions.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=290364&r1=290363&r2=290364&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Thu Dec 22 13:44:05 2016
@@ -244,6 +244,9 @@ void CGOpenMPRuntimeNVPTX::emitEntryHead
 
 void CGOpenMPRuntimeNVPTX::emitEntryFooter(CodeGenFunction &CGF,
EntryFunctionState &EST) {
+  if (!EST.ExitBB)
+EST.ExitBB = CGF.createBasicBlock(".exit");
+
   CGBuilderTy &Bld = CGF.Builder;
   llvm::BasicBlock *TerminateBB = 
CGF.createBasicBlock(".termination.notifier");
   CGF.EmitBranch(TerminateBB);
@@ -259,6 +262,7 @@ void CGOpenMPRuntimeNVPTX::emitEntryFoot
   CGF.EmitBranch(EST.ExitBB);
 
   CGF.EmitBlock(EST.ExitBB);
+  EST.ExitBB = nullptr;
 }
 
 /// \brief Returns specified OpenMP runtime function for the current OpenMP

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h?rev=290364&r1=290363&r2=290364&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h Thu Dec 22 13:44:05 2016
@@ -25,11 +25,8 @@ namespace CodeGen {
 
 class CGOpenMPRuntimeNVPTX : public CGOpenMPRuntime {
 public:
-  class EntryFunctionState {
-  public:
-llvm::BasicBlock *ExitBB;
-
-EntryFunctionState() : ExitBB(nullptr){};
+  struct EntryFunctionState {
+llvm::BasicBlock *ExitBB = nullptr;
   };
 
   class WorkerFunctionState {

Modified: cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp?rev=290364&r1=290363&r2=290364&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp Thu Dec 22 13:44:05 2016
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix 
CHECK --check-prefix CHECK-64
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix 
CHECK --check-prefix CHECK-32
+// RUN: %clang_cc1 -verify -fopenmp -fexceptions -fcxx-exceptions -x c++ 
-triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck 
%s --check-prefix CHECK --check-prefix CHECK-32
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
@@ -25,7 +26,7 @@ int foo(int n) {
   double cn[5][n];
   TT d;
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l86}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l87}}_worker()
   // CHECK: br label {{%?}}[[AWAIT_WORK:.+]]
   //
   // CHECK: [[AWAIT_WORK]]
@@ -53,7 +54,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l86]]()
+  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l87]]()
   // CHECK: [[NTID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
   // CHECK: [[WS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
   // CHECK: [[A:%.+]] = sub i32 [[WS]], 1
@@ -68,7 +69,7 @@ int foo(int n) {
   // CHECK: br i1 [[IS_WORKER]], label {{%?}}[[WORKER:.+]], label 
{{%?}}[[MASTER:.+]]
   //
   // CHECK: [[WORKER]]
-  // CHECK: call void [[T1]]_worker()
+  // CHECK: {{call|invoke}} void [[T1]]_worker()
   // CHECK: br label {{%?}}[[EXIT]]
   //
   // CHECK: [[MASTER]]
@@ -92,7 +93,7 @@ int foo(int n) {
   {
   }
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l157}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l158}}_worker()
   // CHECK: br label {{%?}}[[AWA

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:453
+// There is no effect at O0 when we won't inline anyways.
+if (Opts.OptimizationLevel > 1) {
+  const Option &InlineOpt = InlineArg->getOption();

chandlerc wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > I'd switch the two if
> > The test `> 1` and the comment about `-O0` are confusing to me as well. 
> > What about the following (IIUC):
> > 
> > ```
> > if (Opts.OptimizationLevel <= 1) {
> >   // "always-inline" required for correctness at O0 and O1.
> >   Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining);
> > else if (Opts.OptimizationLevel > 1) {
> >   // Normal inlining at O2 and above
> >   Opts.setInlining((CodeGenOptions::NormalInlining);
> >   // -fno-inline-functions overrides
> >   if (Arg *InlineArg = Args.getLastArg(
> >   options::OPT_finline_functions, 
> > options::OPT_finline_hint_functions,
> >   options::OPT_fno_inline_functions, options::OPT_fno_inline)) {
> >  if (InlineOpt.matches(options::OPT_finline_functions))
> > Opts.setInlining(CodeGenOptions::NormalInlining);
> >   else if (InlineOpt.matches(options::OPT_finline_hint_functions))
> > Opts.setInlining(CodeGenOptions::OnlyHintInlining);
> >   else
> > Opts.setInlining(CodeGenOptions::OnlyAlwaysInlining);
> >   }
> > }
> > ```
> > 
> Yea, this was confusing, and I hadn't actually implemented my intent here 
> either.
> 
> My goal was to make the `Inlining` setting correspond to whether we 
> *forcibly* disable some aspects of inlining or not, rather than anything to 
> do with what inliner pass we run.
> 
> This means we should only be setting `OnlyAlwaysInlining` in O0, not at O1, 
> and the O1 behavior should be up to the backend's construction of a pass 
> pipeline appropriate to that optimization level. (I actually think using the 
> always inliner is a mistake here, but that's a separate discussion.)
> 
> To make all of this work requires me to merge in the next patch I was working 
> on which I've done. It doesn't really scope creep much.
> 
> I've updated the formulation of the conditions and the comments to hopefully 
> be more clear.
> 
> I'm still doing the weird order of testing because i want to always run 
> `getLastArg` to mark these flags as used.
> always run getLastArg to mark these flags as used.

Oh I missed this side effect in the test, that's nasty. Is it covered by a 
test? (i.e. if someone refactor this code in the future will we warn for unused 
and break a test?).


https://reviews.llvm.org/D28053



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


Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.

2016-12-22 Thread Dimitry Andric via cfe-commits
This is when running "ninja check-all", in a tree with llvm, clang and 
compiler-rt checked out.  The first program that shows a failure is 
projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test:

[==] Running 92 tests from 3 test cases.
[--] Global test environment set-up.
[--] 14 tests from AddressSanitizerInterface
...
[ RUN  ] AddressSanitizer.ReallocFreedPointerTest
/share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: 
Failure
Death test: ptr = realloc(ptr, 77)
Result: failed to die.
 Error msg:
[  DEATH   ]
[  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (48 ms)

A similar failure shows when running 
projects/compiler-rt/lib/asan/tests/default/Asan-i386-with-calls-Test:

[==] Running 92 tests from 3 test cases.
[--] Global test environment set-up.
[--] 14 tests from AddressSanitizerInterface
...
[ RUN  ] AddressSanitizer.ReallocFreedPointerTest
/share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377: 
Failure
Death test: ptr = realloc(ptr, 77)
Result: failed to die.
 Error msg:
[  DEATH   ]
[  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (55 ms)

Interestingly, the Asan-i386-inline-Noinst-Test and 
Asan-i386-with-calls-Noinst-Test do not show this particular failure.

The other test that fails is 
projects/compiler-rt/test/asan/I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp,
 which simply returns 1 without printing any output. Debugging the program 
shows that it seems to be skipping completely over the realloc() call, and 
jumping directly to the _exit(1), but this may be due to optimization.

-Dimitry

> On 22 Dec 2016, at 20:27, George Burgess IV  
> wrote:
> 
> Yes, this was reapplied in r290297 with fixes for the msan issue we caught; 
> these asan unit test failures are news to me. Can you give me the command 
> that you're using to run these tests, please?
> 
> On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric  wrote:
> On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits 
>  wrote:
> > Author: chandlerc
> > Date: Tue Dec 20 02:28:19 2016
> > New Revision: 290169
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev
> > Log:
> > Revert r290149: Add the alloc_size attribute to clang.
> >
> > This commit fails MSan when running test/CodeGen/object-size.c in
> > a confusing way. After some discussion with George, it isn't really
> > clear what is going on here. We can make the MSan failure go away by
> > testing for the invalid bit, but *why* things are invalid isn't clear.
> > And yet, other code in the surrounding area is doing precisely this and
> > testing for invalid.
> >
> > George is going to take a closer look at this to better understand the
> > nature of the failure and recommit it, for now backing it out to clean
> > up MSan builds.
> 
> Hmm, was this reapplied later on?  I'm still getting the following 
> AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149 as 
> the cause:
> 
> FAIL: AddressSanitizer-Unit :: 
> Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest (2124 of 30204)
>  TEST 'AddressSanitizer-Unit :: 
> Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED 
> 
> Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from AddressSanitizer
> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
>  Failure
> Death test: ptr = realloc(ptr, 77)
> Result: failed to die.
>  Error msg:
> [  DEATH   ]
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (30 ms)
> [--] 1 test from AddressSanitizer (30 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (31 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest
> 
>  1 FAILED TEST
>   YOU HAVE 24 DISABLED TESTS
> 
> 
> 
> Testing: 0 .
> FAIL: AddressSanitizer-Unit :: 
> Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest (2233 of 
> 30204)
>  TEST 'AddressSanitizer-Unit :: 
> Asan-i386-with-calls-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED 
> 
> Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from AddressSanitizer
> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
>  Failure
> Death test: ptr = realloc(ptr, 77)
> Result: failed to die.
>  Error msg:
> [  DEATH   ]
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

> That said, I agree with Reid that it is a bit surprising. After thinking 
> about it a lot, the reason I personally find it surprising is actually that I 
> find -O0 being the default surprising. But I'm OK with not changing that 
> here, and maybe never changing that at the CC1 layer.

We could reduce the "surprising effect" for LTO behavior by requiring an 
explicit -O when -flto is passed to the driver.

> Thoughts? Generally happy with this direction?

LGTM.


https://reviews.llvm.org/D28053



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


r290367 - Testbed and skeleton of a new expression parser

2016-12-22 Thread Sean Callanan via cfe-commits
Author: spyffe
Date: Thu Dec 22 14:03:14 2016
New Revision: 290367

URL: http://llvm.org/viewvc/llvm-project?rev=290367&view=rev
Log:
Testbed and skeleton of a new expression parser

Recommitted after formal approval.

LLVM's JIT is now the foundation of dynamic-compilation features for many 
languages. Clang also has low-level support for dynamic compilation 
(ASTImporter and ExternalASTSource, notably). How the compiler is set up for 
dynamic parsing is generally left up to individual clients, for example LLDB's 
C/C++/Objective-C expression parser and the ROOT project.

Although this arrangement offers external clients the flexibility to implement 
dynamic features as they see fit, the lack of an in-tree client means that 
subtle bugs can be introduced that cause regressions in the external clients 
but aren't caught by tests (or users) until much later. LLDB for example 
regularly encounters complicated ODR violation scenarios where it is not 
immediately clear who is at fault.

Other external clients (notably, Cling) rely on similar functionality, and 
another goal is to break this functionality up into composable parts so that 
any client can be built easily on top of Clang without requiring extensive 
additional code.

I propose that the parts required to build a simple expression parser be added 
to Clang. Initially, I aim to have the following features:

A piece that looks up external declarations from a variety of sources (e.g., 
from previous dynamic compilations, from modules, or from DWARF) and uses clear 
conflict resolution rules to reconcile differences, with easily understood 
errors. This functionality will be supported by in-tree tests.
A piece that works hand in hand with the LLVM JIT to resolve the locations of 
external declarations so that e.g. variables can be redeclared and (for 
high-performance applications like DTrace) external variables can be accessed 
directly from the registers where they reside.
This commit adds a tester that parses a sequence of source files and then uses 
them as source data for an expression. External references are resolved using 
an ExternalASTSource that responds to name queries using an ASTImporter. This 
is the setup that LLDB uses, and the motivating reason for MinimalImport in 
ASTImporter. When complete, this tester will implement the first of the above 
goals.

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

Added:
cfe/trunk/test/Import/
cfe/trunk/test/Import/clang-flags/
cfe/trunk/test/Import/clang-flags/Inputs/
cfe/trunk/test/Import/clang-flags/Inputs/S.c
cfe/trunk/test/Import/clang-flags/test.c
cfe/trunk/test/Import/empty-struct/
cfe/trunk/test/Import/empty-struct/Inputs/
cfe/trunk/test/Import/empty-struct/Inputs/S.c
cfe/trunk/test/Import/empty-struct/test.c
cfe/trunk/test/Import/error-in-expression/
cfe/trunk/test/Import/error-in-expression/Inputs/
cfe/trunk/test/Import/error-in-expression/Inputs/S.c
cfe/trunk/test/Import/error-in-expression/test.c
cfe/trunk/test/Import/error-in-import/
cfe/trunk/test/Import/error-in-import/Inputs/
cfe/trunk/test/Import/error-in-import/Inputs/S.c
cfe/trunk/test/Import/error-in-import/test.c
cfe/trunk/test/Import/missing-import/
cfe/trunk/test/Import/missing-import/test.c
cfe/trunk/tools/clang-import-test/
cfe/trunk/tools/clang-import-test/CMakeLists.txt
cfe/trunk/tools/clang-import-test/clang-import-test.cpp
Modified:
cfe/trunk/test/CMakeLists.txt
cfe/trunk/tools/CMakeLists.txt

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=290367&r1=290366&r2=290367&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Thu Dec 22 14:03:14 2016
@@ -39,6 +39,7 @@ list(APPEND CLANG_TEST_DEPS
   c-index-test diagtool
   clang-tblgen
   clang-offload-bundler
+  clang-import-test
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)

Added: cfe/trunk/test/Import/clang-flags/Inputs/S.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/clang-flags/Inputs/S.c?rev=290367&view=auto
==
--- cfe/trunk/test/Import/clang-flags/Inputs/S.c (added)
+++ cfe/trunk/test/Import/clang-flags/Inputs/S.c Thu Dec 22 14:03:14 2016
@@ -0,0 +1,2 @@
+STRUCT S {
+};

Added: cfe/trunk/test/Import/clang-flags/test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/clang-flags/test.c?rev=290367&view=auto
==
--- cfe/trunk/test/Import/clang-flags/test.c (added)
+++ cfe/trunk/test/Import/clang-flags/test.c Thu Dec 22 14:03:14 2016
@@ -0,0 +1,5 @@
+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s -Xcc 
-DSTRUCT=struct
+void expr() {
+  STRUCT S MyS;
+  void *MyPtr = &MyS;
+}

Added: cfe/trunk/test/Im

[PATCH] D27180: Testbed and skeleton of a new expression parser

2016-12-22 Thread Sean Callanan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290367: Testbed and skeleton of a new expression parser 
(authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D27180?vs=81991&id=82360#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/test/Import/clang-flags/Inputs/S.c
  cfe/trunk/test/Import/clang-flags/test.c
  cfe/trunk/test/Import/empty-struct/Inputs/S.c
  cfe/trunk/test/Import/empty-struct/test.c
  cfe/trunk/test/Import/error-in-expression/Inputs/S.c
  cfe/trunk/test/Import/error-in-expression/test.c
  cfe/trunk/test/Import/error-in-import/Inputs/S.c
  cfe/trunk/test/Import/error-in-import/test.c
  cfe/trunk/test/Import/missing-import/test.c
  cfe/trunk/tools/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Index: cfe/trunk/test/CMakeLists.txt
===
--- cfe/trunk/test/CMakeLists.txt
+++ cfe/trunk/test/CMakeLists.txt
@@ -39,6 +39,7 @@
   c-index-test diagtool
   clang-tblgen
   clang-offload-bundler
+  clang-import-test
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)
Index: cfe/trunk/test/Import/empty-struct/test.c
===
--- cfe/trunk/test/Import/empty-struct/test.c
+++ cfe/trunk/test/Import/empty-struct/test.c
@@ -0,0 +1,5 @@
+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
+void expr() {
+  struct S MyS;
+  void *MyPtr = &MyS;
+}
Index: cfe/trunk/test/Import/empty-struct/Inputs/S.c
===
--- cfe/trunk/test/Import/empty-struct/Inputs/S.c
+++ cfe/trunk/test/Import/empty-struct/Inputs/S.c
@@ -0,0 +1,2 @@
+struct S {
+};
Index: cfe/trunk/test/Import/clang-flags/Inputs/S.c
===
--- cfe/trunk/test/Import/clang-flags/Inputs/S.c
+++ cfe/trunk/test/Import/clang-flags/Inputs/S.c
@@ -0,0 +1,2 @@
+STRUCT S {
+};
Index: cfe/trunk/test/Import/clang-flags/test.c
===
--- cfe/trunk/test/Import/clang-flags/test.c
+++ cfe/trunk/test/Import/clang-flags/test.c
@@ -0,0 +1,5 @@
+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s -Xcc -DSTRUCT=struct
+void expr() {
+  STRUCT S MyS;
+  void *MyPtr = &MyS;
+}
Index: cfe/trunk/test/Import/error-in-expression/Inputs/S.c
===
--- cfe/trunk/test/Import/error-in-expression/Inputs/S.c
+++ cfe/trunk/test/Import/error-in-expression/Inputs/S.c
@@ -0,0 +1,2 @@
+struct S {
+};
Index: cfe/trunk/test/Import/error-in-expression/test.c
===
--- cfe/trunk/test/Import/error-in-expression/test.c
+++ cfe/trunk/test/Import/error-in-expression/test.c
@@ -0,0 +1,6 @@
+// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
+// CHECK: {{.*}}no viable conversion{{.*}}
+void expr() {
+  struct S MyS;
+  void *MyPtr = MyS;
+}
Index: cfe/trunk/test/Import/error-in-import/test.c
===
--- cfe/trunk/test/Import/error-in-import/test.c
+++ cfe/trunk/test/Import/error-in-import/test.c
@@ -0,0 +1,6 @@
+// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
+// CHECK: {{.*}}expected unqualified-id{{.*}}
+void expr() {
+  struct S MyS;
+  void *MyPtr = &MyS;
+}
Index: cfe/trunk/test/Import/error-in-import/Inputs/S.c
===
--- cfe/trunk/test/Import/error-in-import/Inputs/S.c
+++ cfe/trunk/test/Import/error-in-import/Inputs/S.c
@@ -0,0 +1,2 @@
+struct S [
+];
Index: cfe/trunk/test/Import/missing-import/test.c
===
--- cfe/trunk/test/Import/missing-import/test.c
+++ cfe/trunk/test/Import/missing-import/test.c
@@ -0,0 +1,6 @@
+// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
+// CHECK: {{.*}}Couldn't open{{.*}}Inputs/S.c{{.*}}
+void expr() {
+  struct S MyS;
+  void *MyPtr = &MyS;
+}
Index: cfe/trunk/tools/CMakeLists.txt
===
--- cfe/trunk/tools/CMakeLists.txt
+++ cfe/trunk/tools/CMakeLists.txt
@@ -5,6 +5,7 @@
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
+add_clang_subdirectory(clang-import-test)
 add_clang_subdirectory(clang-offload-bundler)
 
 add_clang_subdirectory(c-index-test)
Index: cfe/trunk/tools/clang-import-test/CMakeLists.txt
===
--- cfe/trunk/tools/clang-import-test/CMakeLists.txt
+++ cfe/trunk/tools/clang-import-test/CMakeLists.txt
@@ -0,0 +1,27 @@
+set(LLVM_LINK_COMPONENT

Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.

2016-12-22 Thread George Burgess IV via cfe-commits
Okay, I'm seeing this failure now if I tag my system's `realloc`
declaration with `alloc_size`. (Which FreeBSD seems to do in their
headers). Because all that clang does with `alloc_size` is use it to answer
`__builtin_object_size` queries and lower it to LLVM's `allocsize`
attribute, this is presumably a latent bug in LLVM's `allocsize` attribute.

Let me mess around for a bit and see what I can dig up. :)

On Thu, Dec 22, 2016 at 11:59 AM, Dimitry Andric  wrote:

> This is when running "ninja check-all", in a tree with llvm, clang and
> compiler-rt checked out.  The first program that shows a failure is
> projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test:
>
> [==] Running 92 tests from 3 test cases.
> [--] Global test environment set-up.
> [--] 14 tests from AddressSanitizerInterface
> ...
> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
> Failure
> Death test: ptr = realloc(ptr, 77)
> Result: failed to die.
>  Error msg:
> [  DEATH   ]
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (48 ms)
>
> A similar failure shows when running projects/compiler-rt/lib/asan/
> tests/default/Asan-i386-with-calls-Test:
>
> [==] Running 92 tests from 3 test cases.
> [--] Global test environment set-up.
> [--] 14 tests from AddressSanitizerInterface
> ...
> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
> Failure
> Death test: ptr = realloc(ptr, 77)
> Result: failed to die.
>  Error msg:
> [  DEATH   ]
> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (55 ms)
>
> Interestingly, the Asan-i386-inline-Noinst-Test and
> Asan-i386-with-calls-Noinst-Test do not show this particular failure.
>
> The other test that fails is projects/compiler-rt/test/asan
> /I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp, which
> simply returns 1 without printing any output. Debugging the program shows
> that it seems to be skipping completely over the realloc() call, and
> jumping directly to the _exit(1), but this may be due to optimization.
>
> -Dimitry
>
> > On 22 Dec 2016, at 20:27, George Burgess IV 
> wrote:
> >
> > Yes, this was reapplied in r290297 with fixes for the msan issue we
> caught; these asan unit test failures are news to me. Can you give me the
> command that you're using to run these tests, please?
> >
> > On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric 
> wrote:
> > On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> > > Author: chandlerc
> > > Date: Tue Dec 20 02:28:19 2016
> > > New Revision: 290169
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev
> > > Log:
> > > Revert r290149: Add the alloc_size attribute to clang.
> > >
> > > This commit fails MSan when running test/CodeGen/object-size.c in
> > > a confusing way. After some discussion with George, it isn't really
> > > clear what is going on here. We can make the MSan failure go away by
> > > testing for the invalid bit, but *why* things are invalid isn't clear.
> > > And yet, other code in the surrounding area is doing precisely this and
> > > testing for invalid.
> > >
> > > George is going to take a closer look at this to better understand the
> > > nature of the failure and recommit it, for now backing it out to clean
> > > up MSan builds.
> >
> > Hmm, was this reapplied later on?  I'm still getting the following
> AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149
> as the cause:
> >
> > FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressS
> anitizer.ReallocFreedPointerTest (2124 of 30204)
> >  TEST 'AddressSanitizer-Unit ::
> Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED
> 
> > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
> > [==] Running 1 test from 1 test case.
> > [--] Global test environment set-up.
> > [--] 1 test from AddressSanitizer
> > [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
> > /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
> Failure
> > Death test: ptr = realloc(ptr, 77)
> > Result: failed to die.
> >  Error msg:
> > [  DEATH   ]
> > [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (30 ms)
> > [--] 1 test from AddressSanitizer (30 ms total)
> >
> > [--] Global test environment tear-down
> > [==] 1 test from 1 test case ran. (31 ms total)
> > [  PASSED  ] 0 tests.
> > [  FAILED  ] 1 test, listed below:
> > [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest
> >
> >  1 FAILED TEST
> >   YOU HAVE 24 DISABLED TESTS
> >
> >
> > 
> > Testing: 0 .
> > FAIL: AddressSanitizer-Unit :: Asan-i386-with-calls-Test/Addr
> essSanitizer.ReallocFreedPointerTest (2233 of 30204)

[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.

2016-12-22 Thread Pekka Jääskeläinen via Phabricator via cfe-commits
pekka.jaaskelainen accepted this revision.
pekka.jaaskelainen added a reviewer: pekka.jaaskelainen.
pekka.jaaskelainen added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D28048#629569, @Anastasia wrote:

> @pekka.jaaskelainen, I think you are using x86 target in the POCL toolchain, 
> do you envision any issues with this change?


No, this should simplify things actually.


https://reviews.llvm.org/D28048



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


[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D28047#629922, @chandlerc wrote:

> In https://reviews.llvm.org/D28047#629892, @mehdi_amini wrote:
>
> > LGTM, but please wait for @rnk  to confirm :)
> >  Also any opinion about a driver option -emit-raw-llvm as @joerg suggested?
>
>
> I like the idea generally, but I don't have an immediate use case and would 
> defer that to a subsequent patch authored by someone who wants this? I really 
> only wanted to remove the divergence between two flags here.


Sure, even @joerg mentioned it was fine to leave for later in his suggestion, I 
just didn't want it to go unnoticed.


https://reviews.llvm.org/D28047



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


r290390 - Fix example: byref struct's init was incorrect, and the block literal's holder should point to it.

2016-12-22 Thread Jonathan Roelofs via cfe-commits
Author: jroelofs
Date: Thu Dec 22 17:48:23 2016
New Revision: 290390

URL: http://llvm.org/viewvc/llvm-project?rev=290390&view=rev
Log:
Fix example: byref struct's init was incorrect, and the block literal's holder 
should point to it.

Modified:
cfe/trunk/docs/Block-ABI-Apple.rst

Modified: cfe/trunk/docs/Block-ABI-Apple.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Block-ABI-Apple.rst?rev=290390&r1=290389&r2=290390&view=diff
==
--- cfe/trunk/docs/Block-ABI-Apple.rst (original)
+++ cfe/trunk/docs/Block-ABI-Apple.rst Thu Dec 22 17:48:23 2016
@@ -530,13 +530,13 @@ and:
 
 .. code-block:: c
 
-struct _block_byref_i i = {( .forwarding=&i, .flags=0, .size=sizeof(struct 
_block_byref_i) )};
+struct _block_byref_i i = {( .isa=NULL, .forwarding=&i, .flags=0, 
.size=sizeof(struct _block_byref_i), .captured_i=2 )};
 struct __block_literal_5 _block_literal = {
   &_NSConcreteStackBlock,
   (1<<25)|(1<<29), ,
   __block_invoke_5,
   &__block_descriptor_5,
-  2,
+  &i,
 };
 
 Importing ``__attribute__((NSObject))`` ``__block`` variables


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


r290392 - Make '-disable-llvm-optzns' an alias for '-disable-llvm-passes'.

2016-12-22 Thread Chandler Carruth via cfe-commits
Author: chandlerc
Date: Thu Dec 22 18:23:01 2016
New Revision: 290392

URL: http://llvm.org/viewvc/llvm-project?rev=290392&view=rev
Log:
Make '-disable-llvm-optzns' an alias for '-disable-llvm-passes'.

Much to my surprise, '-disable-llvm-optzns' which I thought was the
magical flag I wanted to get at the raw LLVM IR coming out of Clang
deosn't do that. It still runs some passes over the IR. I don't want
that, I really want the *raw* IR coming out of Clang and I strongly
suspect everyone else using it is in the same camp.

There is actually a flag that does what I want that I didn't know about
called '-disable-llvm-passes'. I suspect many others don't know about it
either. It both does what I want and is much simpler.

This removes the confusing version and makes that spelling of the flag
an alias for '-disable-llvm-passes'. I've also moved everything in Clang
to use the 'passes' spelling as it seems both more accurate (*all* LLVM
passes are disabled, not just optimizations) and much easier to remember
and spell correctly.

This is part of simplifying how Clang drives LLVM to make it cleaner to
wire up to the new pass manager.

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

Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CXX/drs/dr158.cpp
cfe/trunk/test/CXX/temp/temp.spec/temp.explicit/p9-linkage.cpp
cfe/trunk/test/CodeGen/always_inline.c
cfe/trunk/test/CodeGen/attr-minsize.cpp
cfe/trunk/test/CodeGen/bool_test.c
cfe/trunk/test/CodeGen/builtin-expect.c
cfe/trunk/test/CodeGen/builtin-unpredictable.c
cfe/trunk/test/CodeGen/fixup-depth-overflow.c
cfe/trunk/test/CodeGen/function-attributes.c
cfe/trunk/test/CodeGen/inline.c
cfe/trunk/test/CodeGen/may-alias.c
cfe/trunk/test/CodeGen/tbaa-class.cpp
cfe/trunk/test/CodeGen/tbaa-ms-abi.cpp
cfe/trunk/test/CodeGen/tbaa.cpp
cfe/trunk/test/CodeGenCXX/PR26569.cpp
cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
cfe/trunk/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
cfe/trunk/test/CodeGenCXX/destructors.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp
cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp
cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp
cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp
cfe/trunk/test/CodeGenCXX/inline-hint.cpp
cfe/trunk/test/CodeGenCXX/invariant.group-for-vptrs.cpp
cfe/trunk/test/CodeGenCXX/linkage.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp
cfe/trunk/test/CodeGenCXX/pr24097.cpp
cfe/trunk/test/CodeGenCXX/sanitize-dtor-bit-field.cpp
cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp
cfe/trunk/test/CodeGenCXX/sanitize-dtor-trivial.cpp
cfe/trunk/test/CodeGenCXX/sanitize-dtor-vtable.cpp
cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
cfe/trunk/test/CodeGenCXX/strict-vtable-pointers.cpp
cfe/trunk/test/CodeGenCXX/template-instantiation.cpp
cfe/trunk/test/CodeGenCXX/thunks.cpp
cfe/trunk/test/CodeGenCXX/virtual-destructor-calls.cpp
cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp
cfe/trunk/test/CodeGenCXX/vtable-assume-load.cpp
cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp
cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
cfe/trunk/test/CodeGenObjC/arc-blocks.m
cfe/trunk/test/CodeGenObjC/arc-bridged-cast.m
cfe/trunk/test/CodeGenObjC/arc-literals.m
cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m
cfe/trunk/test/CodeGenObjC/arc-precise-lifetime.m
cfe/trunk/test/CodeGenObjC/arc-ternary-op.m
cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
cfe/trunk/test/CodeGenObjC/arc.m
cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios-arc.m
cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios.m
cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac-arc.m
cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac.m
cfe/trunk/test/CodeGenObjCXX/arc-globals.mm
cfe/trunk/test/CodeGenObjCXX/arc-move.mm
cfe/trunk/test/CodeGenObjCXX/arc-new-delete.mm
cfe/trunk/test/CodeGenObjCXX/arc-references.mm
cfe/trunk/test/CodeGenObjCXX/arc.mm
cfe/trunk/test/CodeGenObjCXX/destroy.mm
cfe/trunk/test/CodeGenObjCXX/literals.mm
cfe/trunk/test/Driver/cc1-response-files.c
cfe/trunk/test/Driver/cl-options.c
cfe/trunk/test/Modules/cxx-irgen.cpp
cfe/trunk/test/OpenMP/declare_reduction_codegen.c
cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp
cfe/trunk/test/Profile/f

[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290392: Make '-disable-llvm-optzns' an alias for 
'-disable-llvm-passes'. (authored by chandlerc).

Changed prior to commit:
  https://reviews.llvm.org/D28047?vs=82313&id=82383#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28047

Files:
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CXX/drs/dr158.cpp
  cfe/trunk/test/CXX/temp/temp.spec/temp.explicit/p9-linkage.cpp
  cfe/trunk/test/CodeGen/always_inline.c
  cfe/trunk/test/CodeGen/attr-minsize.cpp
  cfe/trunk/test/CodeGen/bool_test.c
  cfe/trunk/test/CodeGen/builtin-expect.c
  cfe/trunk/test/CodeGen/builtin-unpredictable.c
  cfe/trunk/test/CodeGen/fixup-depth-overflow.c
  cfe/trunk/test/CodeGen/function-attributes.c
  cfe/trunk/test/CodeGen/inline.c
  cfe/trunk/test/CodeGen/may-alias.c
  cfe/trunk/test/CodeGen/tbaa-class.cpp
  cfe/trunk/test/CodeGen/tbaa-ms-abi.cpp
  cfe/trunk/test/CodeGen/tbaa.cpp
  cfe/trunk/test/CodeGenCXX/PR26569.cpp
  cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
  cfe/trunk/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
  cfe/trunk/test/CodeGenCXX/destructors.cpp
  cfe/trunk/test/CodeGenCXX/dllexport.cpp
  cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp
  cfe/trunk/test/CodeGenCXX/dllimport.cpp
  cfe/trunk/test/CodeGenCXX/exceptions-seh.cpp
  cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp
  cfe/trunk/test/CodeGenCXX/inline-hint.cpp
  cfe/trunk/test/CodeGenCXX/invariant.group-for-vptrs.cpp
  cfe/trunk/test/CodeGenCXX/linkage.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-catch.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-structors-alias.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp
  cfe/trunk/test/CodeGenCXX/pr24097.cpp
  cfe/trunk/test/CodeGenCXX/sanitize-dtor-bit-field.cpp
  cfe/trunk/test/CodeGenCXX/sanitize-dtor-derived-class.cpp
  cfe/trunk/test/CodeGenCXX/sanitize-dtor-tail-call.cpp
  cfe/trunk/test/CodeGenCXX/sanitize-dtor-trivial.cpp
  cfe/trunk/test/CodeGenCXX/sanitize-dtor-vtable.cpp
  cfe/trunk/test/CodeGenCXX/stack-reuse-miscompile.cpp
  cfe/trunk/test/CodeGenCXX/strict-vtable-pointers.cpp
  cfe/trunk/test/CodeGenCXX/template-instantiation.cpp
  cfe/trunk/test/CodeGenCXX/thunks.cpp
  cfe/trunk/test/CodeGenCXX/virtual-destructor-calls.cpp
  cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp
  cfe/trunk/test/CodeGenCXX/vtable-assume-load.cpp
  cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp
  cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
  cfe/trunk/test/CodeGenObjC/arc-blocks.m
  cfe/trunk/test/CodeGenObjC/arc-bridged-cast.m
  cfe/trunk/test/CodeGenObjC/arc-literals.m
  cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m
  cfe/trunk/test/CodeGenObjC/arc-precise-lifetime.m
  cfe/trunk/test/CodeGenObjC/arc-ternary-op.m
  cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m
  cfe/trunk/test/CodeGenObjC/arc.m
  cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios-arc.m
  cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-ios.m
  cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac-arc.m
  cfe/trunk/test/CodeGenObjC/nsvalue-objc-boxable-mac.m
  cfe/trunk/test/CodeGenObjCXX/arc-globals.mm
  cfe/trunk/test/CodeGenObjCXX/arc-move.mm
  cfe/trunk/test/CodeGenObjCXX/arc-new-delete.mm
  cfe/trunk/test/CodeGenObjCXX/arc-references.mm
  cfe/trunk/test/CodeGenObjCXX/arc.mm
  cfe/trunk/test/CodeGenObjCXX/destroy.mm
  cfe/trunk/test/CodeGenObjCXX/literals.mm
  cfe/trunk/test/Driver/cc1-response-files.c
  cfe/trunk/test/Driver/cl-options.c
  cfe/trunk/test/Modules/cxx-irgen.cpp
  cfe/trunk/test/OpenMP/declare_reduction_codegen.c
  cfe/trunk/test/OpenMP/declare_reduction_codegen.cpp
  cfe/trunk/test/Profile/func-entry.c
  cfe/trunk/test/Profile/gcc-flag-compatibility.c
  cfe/trunk/test/Profile/profile-summary.c

Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -160,13 +160,13 @@
   HelpText<"Disable implicit builtin knowledge of math functions">;
 }
 
-def disable_llvm_optzns : Flag<["-"], "disable-llvm-optzns">,
-  HelpText<"Don't run LLVM optimization passes">;
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,
   HelpText<"Don't run the LLVM IR verifier pass">;
 def disable_llvm_passes : Flag<["-"], "disable-llvm-passes">,
   HelpText<"Use together with -emit-llvm to get pristine LLVM IR from the "
"frontend by not running any LLVM passes at all">;
+def disable_llvm_optzns : Flag<["-"], "disable-llvm-optzns">,
+  Alias;
 def disable_red_zone : Flag<["-"], "disable-red-zone">,
   HelpText<"Do not emit code that uses 

Re: r290169 - Revert r290149: Add the alloc_size attribute to clang.

2016-12-22 Thread George Burgess IV via cfe-commits
It looks like the root of this is that we're treating calls to `allocsize`
functions as AllocLike (e.g. any allocation function type except realloc)
functions, which caused us to perform invalid optimizations. For example,
in ReallocFreedPointerTest, EarlyCSE DCE'd the realloc because
llvm::isInstructionTriviallyDead calls llvm::isAllocLikeFn, and
isAllocLikeFn would return true if it saw the allocsize attribute. It
really shouldn't do that.

r290397 should fix this behavior by making allocsize alone insufficient to
consider a function an allocation function.

Thanks for your help!

On Thu, Dec 22, 2016 at 1:10 PM, George Burgess IV <
george.burgess...@gmail.com> wrote:

> Okay, I'm seeing this failure now if I tag my system's `realloc`
> declaration with `alloc_size`. (Which FreeBSD seems to do in their
> headers). Because all that clang does with `alloc_size` is use it to answer
> `__builtin_object_size` queries and lower it to LLVM's `allocsize`
> attribute, this is presumably a latent bug in LLVM's `allocsize` attribute.
>
> Let me mess around for a bit and see what I can dig up. :)
>
> On Thu, Dec 22, 2016 at 11:59 AM, Dimitry Andric 
> wrote:
>
>> This is when running "ninja check-all", in a tree with llvm, clang and
>> compiler-rt checked out.  The first program that shows a failure is
>> projects/compiler-rt/lib/asan/tests/default/Asan-i386-inline-Test:
>>
>> [==] Running 92 tests from 3 test cases.
>> [--] Global test environment set-up.
>> [--] 14 tests from AddressSanitizerInterface
>> ...
>> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
>> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
>> Failure
>> Death test: ptr = realloc(ptr, 77)
>> Result: failed to die.
>>  Error msg:
>> [  DEATH   ]
>> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (48 ms)
>>
>> A similar failure shows when running projects/compiler-rt/lib/asan/
>> tests/default/Asan-i386-with-calls-Test:
>>
>> [==] Running 92 tests from 3 test cases.
>> [--] Global test environment set-up.
>> [--] 14 tests from AddressSanitizerInterface
>> ...
>> [ RUN  ] AddressSanitizer.ReallocFreedPointerTest
>> /share/dim/src/llvm/trunk/projects/compiler-rt/lib/asan/tests/asan_test.cc:377:
>> Failure
>> Death test: ptr = realloc(ptr, 77)
>> Result: failed to die.
>>  Error msg:
>> [  DEATH   ]
>> [  FAILED  ] AddressSanitizer.ReallocFreedPointerTest (55 ms)
>>
>> Interestingly, the Asan-i386-inline-Noinst-Test and
>> Asan-i386-with-calls-Noinst-Test do not show this particular failure.
>>
>> The other test that fails is projects/compiler-rt/test/asan
>> /I386FreeBSDConfig/TestCases/Posix/Output/free_hook_realloc.cc.tmp,
>> which simply returns 1 without printing any output. Debugging the program
>> shows that it seems to be skipping completely over the realloc() call, and
>> jumping directly to the _exit(1), but this may be due to optimization.
>>
>> -Dimitry
>>
>> > On 22 Dec 2016, at 20:27, George Burgess IV <
>> george.burgess...@gmail.com> wrote:
>> >
>> > Yes, this was reapplied in r290297 with fixes for the msan issue we
>> caught; these asan unit test failures are news to me. Can you give me the
>> command that you're using to run these tests, please?
>> >
>> > On Thu, Dec 22, 2016 at 11:10 AM, Dimitry Andric 
>> wrote:
>> > On 20 Dec 2016, at 09:28, Chandler Carruth via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> > > Author: chandlerc
>> > > Date: Tue Dec 20 02:28:19 2016
>> > > New Revision: 290169
>> > >
>> > > URL: http://llvm.org/viewvc/llvm-project?rev=290169&view=rev
>> > > Log:
>> > > Revert r290149: Add the alloc_size attribute to clang.
>> > >
>> > > This commit fails MSan when running test/CodeGen/object-size.c in
>> > > a confusing way. After some discussion with George, it isn't really
>> > > clear what is going on here. We can make the MSan failure go away by
>> > > testing for the invalid bit, but *why* things are invalid isn't clear.
>> > > And yet, other code in the surrounding area is doing precisely this
>> and
>> > > testing for invalid.
>> > >
>> > > George is going to take a closer look at this to better understand the
>> > > nature of the failure and recommit it, for now backing it out to clean
>> > > up MSan builds.
>> >
>> > Hmm, was this reapplied later on?  I'm still getting the following
>> AddressSanitizer failures on FreeBSD, and bisecting has pointed to r290149
>> as the cause:
>> >
>> > FAIL: AddressSanitizer-Unit :: Asan-i386-inline-Test/AddressS
>> anitizer.ReallocFreedPointerTest (2124 of 30204)
>> >  TEST 'AddressSanitizer-Unit ::
>> Asan-i386-inline-Test/AddressSanitizer.ReallocFreedPointerTest' FAILED
>> 
>> > Note: Google Test filter = AddressSanitizer.ReallocFreedPointerTest
>> > [==] Running 1 test from 1 test case.
>> > [--] Global test environment set-up.
>> > [--] 1 test from AddressSanitizer
>> > [ RUN  ] AddressSa

r290398 - Cleanup the handling of noinline function attributes, -fno-inline,

2016-12-22 Thread Chandler Carruth via cfe-commits
Author: chandlerc
Date: Thu Dec 22 19:24:49 2016
New Revision: 290398

URL: http://llvm.org/viewvc/llvm-project?rev=290398&view=rev
Log:
Cleanup the handling of noinline function attributes, -fno-inline,
-fno-inline-functions, -O0, and optnone.

These were really, really tangled together:
- We used the noinline LLVM attribute for -fno-inline
  - But not for -fno-inline-functions (breaking LTO)
  - But we did use it for -finline-hint-functions (yay, LTO is happy!)
  - But we didn't for -O0 (LTO is sad yet again...)
- We had weird structuring of CodeGenOpts with both an inlining
  enumeration and a boolean. They interacted in weird ways and
  needlessly.
- A *lot* of set smashing went on with setting these, and then got worse
  when we considered optnone and other inlining-effecting attributes.
- A bunch of inline affecting attributes were managed in a completely
  different place from -fno-inline.
- Even with -fno-inline we failed to put the LLVM noinline attribute
  onto many generated function definitions because they didn't show up
  as AST-level functions.
- If you passed -O0 but -finline-functions we would run the normal
  inliner pass in LLVM despite it being in the O0 pipeline, which really
  doesn't make much sense.
- Lastly, we used things like '-fno-inline' to manipulate the pass
  pipeline which forced the pass pipeline to be much more
  parameterizable than it really needs to be. Instead we can *just* use
  the optimization level to select a pipeline and control the rest via
  attributes.

Sadly, this causes a bunch of churn in tests because we don't run the
optimizer in the tests and check the contents of attribute sets. It
would be awesome if attribute sets were a bit more FileCheck friendly,
but oh well.

I think this is a significant improvement and should remove the semantic
need to change what inliner pass we run in order to comply with the
requested inlining semantics by relying completely on attributes. It
also cleans up tho optnone and related handling a bit.

One unfortunate aspect of this is that for generating alwaysinline
routines like those in OpenMP we end up removing noinline and then
adding alwaysinline. I tried a bunch of other approaches, but because we
recompute function attributes from scratch and don't have a declaration
here I couldn't find anything substantially cleaner than this.

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

Modified:
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Frontend/CodeGenOptions.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CXX/special/class.dtor/p3-0x.cpp
cfe/trunk/test/CodeGen/2008-04-08-NoExceptions.c
cfe/trunk/test/CodeGen/address-safety-attr-kasan.cpp
cfe/trunk/test/CodeGen/address-safety-attr.cpp
cfe/trunk/test/CodeGen/address-space-field1.c
cfe/trunk/test/CodeGen/alias.c
cfe/trunk/test/CodeGen/attr-minsize.cpp
cfe/trunk/test/CodeGen/attributes.c
cfe/trunk/test/CodeGen/incomplete-function-type-2.c
cfe/trunk/test/CodeGen/inline-optim.c
cfe/trunk/test/CodeGen/mips16-attr.c
cfe/trunk/test/CodeGen/mrtd.c
cfe/trunk/test/CodeGen/ms-declspecs.c
cfe/trunk/test/CodeGen/ppc64-complex-parms.c
cfe/trunk/test/CodeGen/ppc64-complex-return.c
cfe/trunk/test/CodeGen/ppc64-extend.c
cfe/trunk/test/CodeGen/sanitize-thread-attr.cpp
cfe/trunk/test/CodeGen/sanitize-thread-no-checking-at-run-time.m
cfe/trunk/test/CodeGen/unwind-attr.c
cfe/trunk/test/CodeGenCXX/attr.cpp
cfe/trunk/test/CodeGenCXX/cxx11-exception-spec.cpp
cfe/trunk/test/CodeGenCXX/cxx11-noreturn.cpp
cfe/trunk/test/CodeGenCXX/derived-to-base.cpp
cfe/trunk/test/CodeGenCXX/global-dtor-no-atexit.cpp
cfe/trunk/test/CodeGenCXX/global-init.cpp
cfe/trunk/test/CodeGenCXX/inline-hint.cpp
cfe/trunk/test/CodeGenCXX/main-norecurse.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-array-cookies.cpp
cfe/trunk/test/CodeGenCXX/no-exceptions.cpp
cfe/trunk/test/CodeGenCXX/optnone-class-members.cpp
cfe/trunk/test/CodeGenCXX/optnone-def-decl.cpp
cfe/trunk/test/CodeGenCXX/reference-cast.cpp
cfe/trunk/test/CodeGenCXX/threadsafe-statics.cpp
cfe/trunk/test/CodeGenCXX/thunks.cpp
cfe/trunk/test/CodeGenCXX/virtual-base-cast.cpp
cfe/trunk/test/CodeGenObjC/gnu-exceptions.m
cfe/trunk/test/CodeGenObjC/objc-literal-tests.m
cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl
cfe/trunk/test/Driver/darwin-iphone-defaults.m
cfe/trunk/test/PCH/objc_container.m

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=290398&r1=290397&r2=290398&v

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

2016-12-22 Thread Chandler Carruth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290398: Cleanup the handling of noinline function 
attributes, -fno-inline, (authored by chandlerc).

Changed prior to commit:
  https://reviews.llvm.org/D28053?vs=82356&id=82390#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28053

Files:
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/include/clang/Frontend/CodeGenOptions.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CXX/special/class.dtor/p3-0x.cpp
  cfe/trunk/test/CodeGen/2008-04-08-NoExceptions.c
  cfe/trunk/test/CodeGen/address-safety-attr-kasan.cpp
  cfe/trunk/test/CodeGen/address-safety-attr.cpp
  cfe/trunk/test/CodeGen/address-space-field1.c
  cfe/trunk/test/CodeGen/alias.c
  cfe/trunk/test/CodeGen/attr-minsize.cpp
  cfe/trunk/test/CodeGen/attributes.c
  cfe/trunk/test/CodeGen/incomplete-function-type-2.c
  cfe/trunk/test/CodeGen/inline-optim.c
  cfe/trunk/test/CodeGen/mips16-attr.c
  cfe/trunk/test/CodeGen/mrtd.c
  cfe/trunk/test/CodeGen/ms-declspecs.c
  cfe/trunk/test/CodeGen/ppc64-complex-parms.c
  cfe/trunk/test/CodeGen/ppc64-complex-return.c
  cfe/trunk/test/CodeGen/ppc64-extend.c
  cfe/trunk/test/CodeGen/sanitize-thread-attr.cpp
  cfe/trunk/test/CodeGen/sanitize-thread-no-checking-at-run-time.m
  cfe/trunk/test/CodeGen/unwind-attr.c
  cfe/trunk/test/CodeGenCXX/attr.cpp
  cfe/trunk/test/CodeGenCXX/cxx11-exception-spec.cpp
  cfe/trunk/test/CodeGenCXX/cxx11-noreturn.cpp
  cfe/trunk/test/CodeGenCXX/derived-to-base.cpp
  cfe/trunk/test/CodeGenCXX/global-dtor-no-atexit.cpp
  cfe/trunk/test/CodeGenCXX/global-init.cpp
  cfe/trunk/test/CodeGenCXX/inline-hint.cpp
  cfe/trunk/test/CodeGenCXX/main-norecurse.cpp
  cfe/trunk/test/CodeGenCXX/microsoft-abi-array-cookies.cpp
  cfe/trunk/test/CodeGenCXX/no-exceptions.cpp
  cfe/trunk/test/CodeGenCXX/optnone-class-members.cpp
  cfe/trunk/test/CodeGenCXX/optnone-def-decl.cpp
  cfe/trunk/test/CodeGenCXX/reference-cast.cpp
  cfe/trunk/test/CodeGenCXX/threadsafe-statics.cpp
  cfe/trunk/test/CodeGenCXX/thunks.cpp
  cfe/trunk/test/CodeGenCXX/virtual-base-cast.cpp
  cfe/trunk/test/CodeGenObjC/gnu-exceptions.m
  cfe/trunk/test/CodeGenObjC/objc-literal-tests.m
  cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
  cfe/trunk/test/CodeGenOpenCL/amdgpu-attrs.cl
  cfe/trunk/test/Driver/darwin-iphone-defaults.m
  cfe/trunk/test/PCH/objc_container.m

Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -104,8 +104,6 @@
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP zero
 CODEGENOPT(ReciprocalMath, 1, 0) ///< Allow FP divisions to be reassociated.
 CODEGENOPT(NoTrappingMath, 1, 0) ///< Set when -fno-trapping-math is enabled.
-CODEGENOPT(NoInline  , 1, 0) ///< Set when -fno-inline is enabled.
- ///< Disables use of the inline keyword.
 CODEGENOPT(NoNaNsFPMath  , 1, 0) ///< Assume FP arguments, results not NaN.
 CODEGENOPT(FlushDenorm   , 1, 0) ///< Allow FP denorm numbers to be flushed to zero
 CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt
@@ -233,7 +231,7 @@
 CODEGENOPT(EmitCodeView, 1, 0)
 
 /// The kind of inlining to perform.
-ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NoInlining)
+ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NormalInlining)
 
 // Vector functions library to use.
 ENUM_CODEGENOPT(VecLib, VectorLibrary, 2, NoLibrary)
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.h
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.h
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.h
@@ -44,7 +44,6 @@
 class CodeGenOptions : public CodeGenOptionsBase {
 public:
   enum InliningMethod {
-NoInlining, // Perform no inlining whatsoever.
 NormalInlining, // Use the standard function inlining pass.
 OnlyHintInlining,   // Inline only (implicitly) hinted functions.
 OnlyAlwaysInlining  // Only run the always inlining pass.
Index: cfe/trunk/test/Driver/darwin-iphone-defaults.m
===
--- cfe/trunk/test/Driver/darwin-iphone-defaults.m
+++ cfe/trunk/test/Driver/darwin-iphone-defaults.m
@@ -26,4 +26,4 @@
   [I1 alloc];
 }
 
-// CHECK: attributes [[F0]] = { ssp{{.*}} }
+// CHECK: attributes [[F0]] = { noinline ssp{{.*}} }
Index: cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
===
--- cfe/trunk/test/CodeGenObjCXX/lambda-expressions.mm
+++

r290399 - When merging two deduced non-type template arguments for the same parameter,

2016-12-22 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Dec 22 19:30:39 2016
New Revision: 290399

URL: http://llvm.org/viewvc/llvm-project?rev=290399&view=rev
Log:
When merging two deduced non-type template arguments for the same parameter,
fail the merge if the arguments have different types (except if one of them was
deduced from an array bound, in which case take the type from the other).

This is correct because (except in the array bound case) the type of the
template argument in each deduction must match the type of the parameter, so at
least one of the two deduced arguments must have a mismatched type.

This is necessary because we would otherwise lose the type information for the
discarded template argument in the merge, and fail to diagnose the mismatch.

In order to power this, we now properly retain the type of a deduced non-type
template argument deduced from a declaration, rather than giving it the type of
the template parameter; we'll convert it to the template parameter type when
checking the deduced arguments.

Modified:
cfe/trunk/include/clang/AST/TemplateBase.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/TemplateBase.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/test/CXX/temp/temp.fct.spec/temp.deduct/p9.cpp
cfe/trunk/test/SemaTemplate/deduction.cpp

Modified: cfe/trunk/include/clang/AST/TemplateBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TemplateBase.h?rev=290399&r1=290398&r2=290399&view=diff
==
--- cfe/trunk/include/clang/AST/TemplateBase.h (original)
+++ cfe/trunk/include/clang/AST/TemplateBase.h Thu Dec 22 19:30:39 2016
@@ -301,6 +301,10 @@ public:
 Integer.Type = T.getAsOpaquePtr();
   }
 
+  /// \brief If this is a non-type template argument, get its type. Otherwise,
+  /// returns a null QualType.
+  QualType getNonTypeTemplateArgumentType() const;
+
   /// \brief Retrieve the template argument as an expression.
   Expr *getAsExpr() const {
 assert(getKind() == Expression && "Unexpected kind");

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290399&r1=290398&r2=290399&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22 19:30:39 
2016
@@ -3343,6 +3343,10 @@ def note_ovl_candidate_incomplete_deduct
 def note_ovl_candidate_inconsistent_deduction : Note<
 "candidate template ignored: deduced conflicting %select{types|values|"
 "templates}0 for parameter %1%diff{ ($ vs. $)|}2,3">;
+def note_ovl_candidate_inconsistent_deduction_types : Note<
+"candidate template ignored: deduced values %diff{"
+"of conflicting types for parameter %0 (%1 of type $ vs. %3 of type $)|"
+"%1 and %3 of conflicting types for parameter %0|}2,4">;
 def note_ovl_candidate_explicit_arg_mismatch_named : Note<
 "candidate template ignored: invalid explicitly-specified argument "
 "for template parameter %0">;
@@ -3840,7 +3844,7 @@ def err_non_type_template_parm_type_dedu
   "non-type template parameter %0 with type %1 has incompatible initializer of 
type %2">;
 def err_deduced_non_type_template_arg_type_mismatch : Error<
   "deduced non-type template argument does not have the same type as the "
-  "its corresponding template parameter%diff{ ($ vs $)|}0,1">;
+  "corresponding template parameter%diff{ ($ vs $)|}0,1">;
 def err_non_type_template_arg_subobject : Error<
   "non-type template argument refers to subobject '%0'">;
 def err_non_type_template_arg_addr_label_diff : Error<

Modified: cfe/trunk/lib/AST/TemplateBase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TemplateBase.cpp?rev=290399&r1=290398&r2=290399&view=diff
==
--- cfe/trunk/lib/AST/TemplateBase.cpp (original)
+++ cfe/trunk/lib/AST/TemplateBase.cpp Thu Dec 22 19:30:39 2016
@@ -243,6 +243,31 @@ Optional TemplateArgument::get
   return None; 
 }
 
+QualType TemplateArgument::getNonTypeTemplateArgumentType() const {
+  switch (getKind()) {
+  case TemplateArgument::Null:
+  case TemplateArgument::Type:
+  case TemplateArgument::Template:
+  case TemplateArgument::TemplateExpansion:
+  case TemplateArgument::Pack:
+return QualType();
+
+  case TemplateArgument::Integral:
+return getIntegralType();
+
+  case TemplateArgument::Expression:
+return getAsExpr()->getType();
+
+  case TemplateArgument::Declaration:
+return getParamTypeForDecl();
+
+  case TemplateArgument::NullPtr:
+return getNullPtrType();
+  }
+
+  llvm_unreachable("Invalid TemplateArgument Kind!");
+}
+
 void TemplateArgument::Profile(llvm::FoldingSetNodeID &ID,
 

r290403 - Only substitute into type of non-type template parameter once, rather than

2016-12-22 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Dec 22 20:00:24 2016
New Revision: 290403

URL: http://llvm.org/viewvc/llvm-project?rev=290403&view=rev
Log:
Only substitute into type of non-type template parameter once, rather than
twice, in finalization of template argument deduction.

This is a re-commit of r290310 (reverted in r290329); the bug found by the
buildbots was fixed in r290399 (we would sometimes build a deduced template
argument with a bogus type).

Modified:
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=290403&r1=290402&r2=290403&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Thu Dec 22 20:00:24 2016
@@ -2072,7 +2072,8 @@ static bool isSameTemplateArg(ASTContext
 ///
 /// \param NTTPType For a declaration template argument, the type of
 /// the non-type template parameter that corresponds to this template
-/// argument.
+/// argument. Can be null if no type sugar is available to add to the
+/// type from the template argument.
 ///
 /// \param Loc The source location to use for the resulting template
 /// argument.
@@ -2088,12 +2089,16 @@ Sema::getTrivialTemplateArgumentLoc(cons
 Arg, Context.getTrivialTypeSourceInfo(Arg.getAsType(), Loc));
 
   case TemplateArgument::Declaration: {
+if (NTTPType.isNull())
+  NTTPType = Arg.getParamTypeForDecl();
 Expr *E = BuildExpressionFromDeclTemplateArgument(Arg, NTTPType, Loc)
   .getAs();
 return TemplateArgumentLoc(TemplateArgument(E), E);
   }
 
   case TemplateArgument::NullPtr: {
+if (NTTPType.isNull())
+  NTTPType = Arg.getNullPtrType();
 Expr *E = BuildExpressionFromDeclTemplateArgument(Arg, NTTPType, Loc)
   .getAs();
 return TemplateArgumentLoc(TemplateArgument(NTTPType, /*isNullPtr*/true),
@@ -2144,31 +2149,13 @@ ConvertDeducedTemplateArgument(Sema &S,
TemplateDeductionInfo &Info,
bool InFunctionTemplate,
SmallVectorImpl &Output) {
-  // First, for a non-type template parameter type that is
-  // initialized by a declaration, we need the type of the
-  // corresponding non-type template parameter.
-  QualType NTTPType;
-  if (NonTypeTemplateParmDecl *NTTP =
-  dyn_cast(Param)) {
-NTTPType = NTTP->getType();
-if (NTTPType->isDependentType()) {
-  TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack, Output);
-  NTTPType = S.SubstType(NTTPType,
- MultiLevelTemplateArgumentList(TemplateArgs),
- NTTP->getLocation(),
- NTTP->getDeclName());
-  if (NTTPType.isNull())
-return true;
-}
-  }
-
   auto ConvertArg = [&](DeducedTemplateArgument Arg,
 unsigned ArgumentPackIndex) {
 // Convert the deduced template argument into a template
 // argument that we can check, almost as if the user had written
 // the template argument explicitly.
 TemplateArgumentLoc ArgLoc =
-S.getTrivialTemplateArgumentLoc(Arg, NTTPType, Info.getLocation());
+S.getTrivialTemplateArgumentLoc(Arg, QualType(), Info.getLocation());
 
 // Check the template argument, converting it as necessary.
 return S.CheckTemplateArgument(
@@ -2200,22 +2187,28 @@ ConvertDeducedTemplateArgument(Sema &S,
 }
 
 // If the pack is empty, we still need to substitute into the parameter
-// itself, in case that substitution fails. For non-type parameters, we did
-// this above. For type parameters, no substitution is ever required.
-auto *TTP = dyn_cast(Param);
-if (TTP && PackedArgsBuilder.empty()) {
-  // Set up a template instantiation context.
+// itself, in case that substitution fails.
+if (PackedArgsBuilder.empty()) {
   LocalInstantiationScope Scope(S);
-  Sema::InstantiatingTemplate Inst(S, Template->getLocation(), Template,
-   TTP, Output,
-   Template->getSourceRange());
-  if (Inst.isInvalid())
-return true;
-
   TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack, Output);
-  if (!S.SubstDecl(TTP, S.CurContext,
-   MultiLevelTemplateArgumentList(TemplateArgs)))
-return true;
+  MultiLevelTemplateArgumentList Args(TemplateArgs);
+
+  if (auto *NTTP = dyn_cast(Param)) {
+Sema::InstantiatingTemplate Inst(S, Template->getLocation(), Template,
+ NTTP, Output,
+ Template->getSourceRange());
+if (Inst.isInvalid() || 
+S.SubstType(NTTP->getType(), Args, NTTP->getLocation(),
+ 

r290405 - Move generation of injected template arguments for a template parameter list

2016-12-22 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Dec 22 20:10:11 2016
New Revision: 290405

URL: http://llvm.org/viewvc/llvm-project?rev=290405&view=rev
Log:
Move generation of injected template arguments for a template parameter list
out of an internal function and into ASTContext; this is needed in template
argument deduction for P0522R0.

Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/DeclTemplate.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=290405&r1=290404&r2=290405&view=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Thu Dec 22 20:10:11 2016
@@ -1355,6 +1355,12 @@ public:
   ElaboratedTypeKeyword Keyword, NestedNameSpecifier *NNS,
   const IdentifierInfo *Name, ArrayRef Args) const;
 
+  /// Get a template argument list with one argument per template parameter
+  /// in a template parameter list, such as for the injected class name of
+  /// a class template.
+  void getInjectedTemplateArgs(const TemplateParameterList *Params,
+   SmallVectorImpl &Args);
+
   QualType getPackExpansionType(QualType Pattern,
 Optional NumExpansions);
 

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=290405&r1=290404&r2=290405&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Dec 22 20:10:11 2016
@@ -3872,6 +3872,44 @@ ASTContext::getDependentTemplateSpeciali
   return QualType(T, 0);
 }
 
+void
+ASTContext::getInjectedTemplateArgs(const TemplateParameterList *Params,
+SmallVectorImpl &Args) {
+  Args.reserve(Args.size() + Params->size());
+
+  for (NamedDecl *Param : *Params) {
+TemplateArgument Arg;
+if (auto *TTP = dyn_cast(Param)) {
+  QualType ArgType = getTypeDeclType(TTP);
+  if (TTP->isParameterPack())
+ArgType = getPackExpansionType(ArgType, None);
+
+  Arg = TemplateArgument(ArgType);
+} else if (auto *NTTP = dyn_cast(Param)) {
+  Expr *E = new (*this) DeclRefExpr(
+  NTTP, /*enclosing*/false,
+  NTTP->getType().getNonLValueExprType(*this),
+  Expr::getValueKindForType(NTTP->getType()), NTTP->getLocation());
+
+  if (NTTP->isParameterPack())
+E = new (*this) PackExpansionExpr(DependentTy, E, NTTP->getLocation(),
+  None);
+  Arg = TemplateArgument(E);
+} else {
+  auto *TTP = cast(Param);
+  if (TTP->isParameterPack())
+Arg = TemplateArgument(TemplateName(TTP), Optional());
+  else
+Arg = TemplateArgument(TemplateName(TTP));
+}
+
+if (Param->isTemplateParameterPack())
+  Arg = TemplateArgument::CreatePackCopy(*this, Arg);
+
+Args.push_back(Arg);
+  }
+}
+
 QualType ASTContext::getPackExpansionType(QualType Pattern,
   Optional NumExpansions) {
   llvm::FoldingSetNodeID ID;

Modified: cfe/trunk/lib/AST/DeclTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=290405&r1=290404&r2=290405&view=diff
==
--- cfe/trunk/lib/AST/DeclTemplate.cpp (original)
+++ cfe/trunk/lib/AST/DeclTemplate.cpp Thu Dec 22 20:10:11 2016
@@ -204,44 +204,6 @@ void RedeclarableTemplateDecl::addSpecia
   SETraits::getDecl(Entry));
 }
 
-/// \brief Generate the injected template arguments for the given template
-/// parameter list, e.g., for the injected-class-name of a class template.
-static void GenerateInjectedTemplateArgs(ASTContext &Context,
- TemplateParameterList *Params,
- TemplateArgument *Args) {
-  for (NamedDecl *Param : *Params) {
-TemplateArgument Arg;
-if (auto *TTP = dyn_cast(Param)) {
-  QualType ArgType = Context.getTypeDeclType(TTP);
-  if (TTP->isParameterPack())
-ArgType = Context.getPackExpansionType(ArgType, None);
-
-  Arg = TemplateArgument(ArgType);
-} else if (auto *NTTP = dyn_cast(Param)) {
-  Expr *E = new (Context) DeclRefExpr(NTTP, /*enclosing*/ false,
-  
NTTP->getType().getNonLValueExprType(Context),
-  Expr::getValueKindForType(NTTP->getType()),
-  NTTP->getLocation());
-
-  if (NTTP->isParameterPack())
-E = new (Context) PackExpansionExpr(Context.DependentTy, E,
-NTTP->getLocation(), None);
-  Arg = TemplateAr

[PATCH] D25556: [Sema] Add variable captured by a block to the enclosing lambda's potential capture list

2016-12-22 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 82394.
ahatanak added a comment.

Rebase.

r286584 changed getCurLambda to optionally skip CapturedRegionScopeInfos. This 
patch changes it to skip BlockScopeInfos too.


https://reviews.llvm.org/D25556

Files:
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjCXX/blocks.mm


Index: test/SemaObjCXX/blocks.mm
===
--- test/SemaObjCXX/blocks.mm
+++ test/SemaObjCXX/blocks.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class 
-std=c++14 %s
 @protocol NSObject;
 
 void bar(id(^)(void));
@@ -144,3 +144,14 @@
 
   template void f(X);
 }
+
+namespace GenericLambdaCapture {
+  int test(int outerp) {
+auto lambda =[&](auto p) {
+  return ^{
+return p + outerp;
+  }();
+};
+return lambda(1);
+  }
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14126,8 +14126,9 @@
 (SemaRef.CurContext != Var->getDeclContext() &&
  Var->getDeclContext()->isFunctionOrMethod() && 
Var->hasLocalStorage());
 if (RefersToEnclosingScope) {
-  if (LambdaScopeInfo *const LSI =
-  SemaRef.getCurLambda(/*IgnoreCapturedRegions=*/true)) {
+  LambdaScopeInfo *const LSI =
+  SemaRef.getCurLambda(/*IgnoreNonLambdaCapturingScope=*/true);
+  if (LSI && !LSI->CallOperator->Encloses(Var->getDeclContext())) {
 // If a variable could potentially be odr-used, defer marking it so
 // until we finish analyzing the full expression for any
 // lvalue-to-rvalue
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1243,14 +1243,14 @@
   return CurBSI;
 }
 
-LambdaScopeInfo *Sema::getCurLambda(bool IgnoreCapturedRegions) {
+LambdaScopeInfo *Sema::getCurLambda(bool IgnoreNonLambdaCapturingScope) {
   if (FunctionScopes.empty())
 return nullptr;
 
   auto I = FunctionScopes.rbegin();
-  if (IgnoreCapturedRegions) {
+  if (IgnoreNonLambdaCapturingScope) {
 auto E = FunctionScopes.rend();
-while (I != E && isa(*I))
+while (I != E && isa(*I) && !isa(*I))
   ++I;
 if (I == E)
   return nullptr;
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1226,9 +1226,11 @@
   sema::BlockScopeInfo *getCurBlock();
 
   /// Retrieve the current lambda scope info, if any.
-  /// \param IgnoreCapturedRegions true if should find the top-most lambda 
scope
-  /// info ignoring all inner captured regions scope infos.
-  sema::LambdaScopeInfo *getCurLambda(bool IgnoreCapturedRegions = false);
+  /// \param IgnoreNonLambdaCapturingScope true if should find the top-most
+  /// lambda scope info ignoring all inner capturing scopes that are not
+  /// lambda scopes.
+  sema::LambdaScopeInfo *
+  getCurLambda(bool IgnoreNonLambdaCapturingScope = false);
 
   /// \brief Retrieve the current generic lambda info, if any.
   sema::LambdaScopeInfo *getCurGenericLambda();


Index: test/SemaObjCXX/blocks.mm
===
--- test/SemaObjCXX/blocks.mm
+++ test/SemaObjCXX/blocks.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class -std=c++14 %s
 @protocol NSObject;
 
 void bar(id(^)(void));
@@ -144,3 +144,14 @@
 
   template void f(X);
 }
+
+namespace GenericLambdaCapture {
+  int test(int outerp) {
+auto lambda =[&](auto p) {
+  return ^{
+return p + outerp;
+  }();
+};
+return lambda(1);
+  }
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14126,8 +14126,9 @@
 (SemaRef.CurContext != Var->getDeclContext() &&
  Var->getDeclContext()->isFunctionOrMethod() && Var->hasLocalStorage());
 if (RefersToEnclosingScope) {
-  if (LambdaScopeInfo *const LSI =
-  SemaRef.getCurLambda(/*IgnoreCapturedRegions=*/true)) {
+  LambdaScopeInfo *const LSI =
+  SemaRef.getCurLambda(/*IgnoreNonLambdaCapturingScope=*/true);
+  if (LSI && !LSI->CallOperator->Encloses(Var->getDeclContext())) {
 // If a variable could potentially be odr-used, defer marking it so
 // until we finish analyzing the full expression for any
 // lvalue-to-rvalue
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1243,14 +1243,14 @@
   return CurBSI;
 }
 
-LambdaScopeInfo *Sema::getCurLambda(bool IgnoreCapturedRe

[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration

2016-12-22 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D27680#629182, @ahatanak wrote:

> In https://reviews.llvm.org/D27680#628272, @rjmccall wrote:
>
> > Wouldn't it be simpler to just record an insertion point for the beginning 
> > of the current lexical scope and insert the lifetime.begin there instead of 
> > at the current IP?
>
>
> I'm not sure I understood your comment, but it seems to me that simply moving 
> the lifetime.start intrinsics to the current lexical scope wouldn't work in a 
> case like this:


I'm suggesting that, instead of moving instructions retroactively when you see 
a goto, you just insert the lifetime.start intrinsics at the start of the 
current lexical scope when you're emitting the variable.  That more exactly 
models the C language rule, I think, and shouldn't have any significant 
negative impact on optimization.


https://reviews.llvm.org/D27680



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


r290413 - Fix for PR15623. The patch eliminates unwanted ProgramState checker data propagation from an operand of the logical operation to operation result.

2016-12-22 Thread Anton Yartsev via cfe-commits
Author: ayartsev
Date: Thu Dec 22 21:31:00 2016
New Revision: 290413

URL: http://llvm.org/viewvc/llvm-project?rev=290413&view=rev
Log:
Fix for PR15623. The patch eliminates unwanted ProgramState checker data 
propagation from an operand of the logical operation to operation result.
The patch also simplifies an assume of a constraint of the form: "(exp 
comparison_op expr) != 0" to true into an assume of "exp comparison_op expr" to 
true. (And similarly, an assume of the form "(exp comparison_op expr) == 0" to 
true as an assume of exp comparison_op expr to false.) which improves precision 
overall.
https://reviews.llvm.org/D22862

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
cfe/trunk/test/Analysis/malloc.c
cfe/trunk/test/Analysis/misc-ps.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=290413&r1=290412&r2=290413&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Thu Dec 22 21:31:00 2016
@@ -665,23 +665,13 @@ void ExprEngine::VisitLogicalExpr(const
 if (RHSVal.isUndef()) {
   X = RHSVal;
 } else {
-  DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs();
-  ProgramStateRef StTrue, StFalse;
-  std::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS);
-  if (StTrue) {
-if (StFalse) {
-  // We can't constrain the value to 0 or 1.
-  // The best we can do is a cast.
-  X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType());
-} else {
-  // The value is known to be true.
-  X = getSValBuilder().makeIntVal(1, B->getType());
-}
-  } else {
-// The value is known to be false.
-assert(StFalse && "Infeasible path!");
-X = getSValBuilder().makeIntVal(0, B->getType());
-  }
+  // We evaluate "RHSVal != 0" expression which result in 0 if the value is
+  // known to be false, 1 if the value is known to be true and a new symbol
+  // when the assumption is unknown.
+  nonloc::ConcreteInt Zero(getBasicVals().getValue(0, B->getType()));
+  X = evalBinOp(N->getState(), BO_NE, 
+svalBuilder.evalCast(RHSVal, B->getType(), RHS->getType()),
+Zero, B->getType());
 }
   }
   Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), 
X));

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp?rev=290413&r1=290412&r2=290413&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp Thu Dec 22 
21:31:00 2016
@@ -254,6 +254,21 @@ ProgramStateRef SimpleConstraintManager:
   assert(BinaryOperator::isComparisonOp(Op) &&
  "Non-comparison ops should be rewritten as comparisons to zero.");
 
+  SymbolRef Sym = LHS;
+
+  // Simplification: translate an assume of a constraint of the form
+  // "(exp comparison_op expr) != 0" to true into an assume of 
+  // "exp comparison_op expr" to true. (And similarly, an assume of the form
+  // "(exp comparison_op expr) == 0" to true into an assume of
+  // "exp comparison_op expr" to false.)
+  if (Int == 0 && (Op == BO_EQ || Op == BO_NE)) {
+if (const BinarySymExpr *SE = dyn_cast(Sym)) {
+  BinaryOperator::Opcode Op = SE->getOpcode();
+  if (BinaryOperator::isComparisonOp(Op))
+return assume(State, nonloc::SymbolVal(Sym), (Op == BO_NE ? true : 
false));
+}
+  }
+
   // Get the type used for calculating wraparound.
   BasicValueFactory &BVF = getBasicVals();
   APSIntType WraparoundType = BVF.getAPSIntType(LHS->getType());
@@ -265,7 +280,6 @@ ProgramStateRef SimpleConstraintManager:
   // x < 4 has the solution [0, 3]. x+2 < 4 has the solution [0-2, 3-2], which
   // in modular arithmetic is [0, 1] U [UINT_MAX-1, UINT_MAX]. It's up to
   // the subclasses of SimpleConstraintManager to handle the adjustment.
-  SymbolRef Sym = LHS;
   llvm::APSInt Adjustment = WraparoundType.getZeroValue();
   computeAdjustment(Sym, Adjustment);
 

Modified: cfe/trunk/test/Analysis/malloc.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=290413&r1=290412&r2=290413&view=diff
==
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Thu Dec 22 21:31:00 2016
@@ -1763,6 +1763,17 @@ void testConstEscapeThroughAnotherField(
   constEscape(&(s.x)); // could free s->p!
 } // no-warning
 
+// PR15623
+in

r290415 - Revert changes made by r290413 until regression is fixed.

2016-12-22 Thread Anton Yartsev via cfe-commits
Author: ayartsev
Date: Thu Dec 22 22:09:18 2016
New Revision: 290415

URL: http://llvm.org/viewvc/llvm-project?rev=290415&view=rev
Log:
Revert changes made by r290413 until regression is fixed.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
cfe/trunk/test/Analysis/malloc.c
cfe/trunk/test/Analysis/misc-ps.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=290415&r1=290414&r2=290415&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Thu Dec 22 22:09:18 2016
@@ -665,13 +665,23 @@ void ExprEngine::VisitLogicalExpr(const
 if (RHSVal.isUndef()) {
   X = RHSVal;
 } else {
-  // We evaluate "RHSVal != 0" expression which result in 0 if the value is
-  // known to be false, 1 if the value is known to be true and a new symbol
-  // when the assumption is unknown.
-  nonloc::ConcreteInt Zero(getBasicVals().getValue(0, B->getType()));
-  X = evalBinOp(N->getState(), BO_NE, 
-svalBuilder.evalCast(RHSVal, B->getType(), RHS->getType()),
-Zero, B->getType());
+  DefinedOrUnknownSVal DefinedRHS = RHSVal.castAs();
+  ProgramStateRef StTrue, StFalse;
+  std::tie(StTrue, StFalse) = N->getState()->assume(DefinedRHS);
+  if (StTrue) {
+if (StFalse) {
+  // We can't constrain the value to 0 or 1.
+  // The best we can do is a cast.
+  X = getSValBuilder().evalCast(RHSVal, B->getType(), RHS->getType());
+} else {
+  // The value is known to be true.
+  X = getSValBuilder().makeIntVal(1, B->getType());
+}
+  } else {
+// The value is known to be false.
+assert(StFalse && "Infeasible path!");
+X = getSValBuilder().makeIntVal(0, B->getType());
+  }
 }
   }
   Bldr.generateNode(B, Pred, state->BindExpr(B, Pred->getLocationContext(), 
X));

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp?rev=290415&r1=290414&r2=290415&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp Thu Dec 22 
22:09:18 2016
@@ -254,21 +254,6 @@ ProgramStateRef SimpleConstraintManager:
   assert(BinaryOperator::isComparisonOp(Op) &&
  "Non-comparison ops should be rewritten as comparisons to zero.");
 
-  SymbolRef Sym = LHS;
-
-  // Simplification: translate an assume of a constraint of the form
-  // "(exp comparison_op expr) != 0" to true into an assume of 
-  // "exp comparison_op expr" to true. (And similarly, an assume of the form
-  // "(exp comparison_op expr) == 0" to true into an assume of
-  // "exp comparison_op expr" to false.)
-  if (Int == 0 && (Op == BO_EQ || Op == BO_NE)) {
-if (const BinarySymExpr *SE = dyn_cast(Sym)) {
-  BinaryOperator::Opcode Op = SE->getOpcode();
-  if (BinaryOperator::isComparisonOp(Op))
-return assume(State, nonloc::SymbolVal(Sym), (Op == BO_NE ? true : 
false));
-}
-  }
-
   // Get the type used for calculating wraparound.
   BasicValueFactory &BVF = getBasicVals();
   APSIntType WraparoundType = BVF.getAPSIntType(LHS->getType());
@@ -280,6 +265,7 @@ ProgramStateRef SimpleConstraintManager:
   // x < 4 has the solution [0, 3]. x+2 < 4 has the solution [0-2, 3-2], which
   // in modular arithmetic is [0, 1] U [UINT_MAX-1, UINT_MAX]. It's up to
   // the subclasses of SimpleConstraintManager to handle the adjustment.
+  SymbolRef Sym = LHS;
   llvm::APSInt Adjustment = WraparoundType.getZeroValue();
   computeAdjustment(Sym, Adjustment);
 

Modified: cfe/trunk/test/Analysis/malloc.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=290415&r1=290414&r2=290415&view=diff
==
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Thu Dec 22 22:09:18 2016
@@ -1763,17 +1763,6 @@ void testConstEscapeThroughAnotherField(
   constEscape(&(s.x)); // could free s->p!
 } // no-warning
 
-// PR15623
-int testNoCheckerDataPropogationFromLogicalOpOperandToOpResult(void) {
-   char *param = malloc(10);
-   char *value = malloc(10);
-   int ok = (param && value);
-   free(param);
-   free(value);
-   // Previously we ended up with 'Use of memory after it is freed' on return.
-   return ok; // no warning
-}
-
 // 
 // False negatives.
 

Modified: cfe/trunk/test/Analysi

[PATCH] D27680: [CodeGen] Move lifetime.start of a variable when goto jumps back past its declaration

2016-12-22 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Ah, good idea. That sounds like a much simpler and less invasive approach. I 
agree that the performance impact would probably be small, and if it turns out 
to have a significant impact, we can reduce the number of times we move the 
lifetime.start (without moving it retroactively) by checking whether a label 
was seen.

I'll see if I can come with a new patch.


https://reviews.llvm.org/D27680



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


r290417 - Add an assert to catch improperly constructed %diff sequences in

2016-12-22 Thread Chandler Carruth via cfe-commits
Author: chandlerc
Date: Thu Dec 22 23:19:47 2016
New Revision: 290417

URL: http://llvm.org/viewvc/llvm-project?rev=290417&view=rev
Log:
Add an assert to catch improperly constructed %diff sequences in
diagnostics and fix one such diagnostic.

Sadly, this assert doesn't catch this bug because we have no tests that
emit this diagnostic! Doh! I'm following up on the commit that
introduces it to get that fixed. Then this assert will help in a more
direct way.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Basic/Diagnostic.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290417&r1=290416&r2=290417&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22 23:19:47 
2016
@@ -3346,7 +3346,7 @@ def note_ovl_candidate_inconsistent_dedu
 def note_ovl_candidate_inconsistent_deduction_types : Note<
 "candidate template ignored: deduced values %diff{"
 "of conflicting types for parameter %0 (%1 of type $ vs. %3 of type $)|"
-"%1 and %3 of conflicting types for parameter %0|}2,4">;
+"%1 and %3 of conflicting types for parameter %0}2,4">;
 def note_ovl_candidate_explicit_arg_mismatch_named : Note<
 "candidate template ignored: invalid explicitly-specified argument "
 "for template parameter %0">;

Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=290417&r1=290416&r2=290417&view=diff
==
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Thu Dec 22 23:19:47 2016
@@ -742,7 +742,10 @@ FormatDiagnostic(const char *DiagStr, co
 //   "%diff{compare $ to $|other text}1,2"
 // treat it as:
 //   "compare %1 to %2"
-const char *Pipe = ScanFormat(Argument, Argument + ArgumentLen, '|');
+const char *ArgumentEnd = Argument + ArgumentLen;
+const char *Pipe = ScanFormat(Argument, ArgumentEnd, '|');
+assert(ScanFormat(Pipe + 1, ArgumentEnd, '|') == ArgumentEnd &&
+   "Found too many '|'s in a %diff modifier!");
 const char *FirstDollar = ScanFormat(Argument, Pipe, '$');
 const char *SecondDollar = ScanFormat(FirstDollar + 1, Pipe, '$');
 const char ArgStr1[] = { '%', static_cast('0' + ArgNo) };


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


Re: r290399 - When merging two deduced non-type template arguments for the same parameter,

2016-12-22 Thread Chandler Carruth via cfe-commits
On Thu, Dec 22, 2016 at 5:41 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Dec 22
> 19:30:39 2016
> @@ -3343,6 +3343,10 @@ def note_ovl_candidate_incomplete_deduct
>  def note_ovl_candidate_inconsistent_deduction : Note<
>  "candidate template ignored: deduced conflicting
> %select{types|values|"
>  "templates}0 for parameter %1%diff{ ($ vs. $)|}2,3">;
> +def note_ovl_candidate_inconsistent_deduction_types : Note<
> +"candidate template ignored: deduced values %diff{"
> +"of conflicting types for parameter %0 (%1 of type $ vs. %3 of type
> $)|"
> +"%1 and %3 of conflicting types for parameter %0|}2,4">;
>

So, this new diagnostic isn't actually covered by any test. Not just is the
wording not covered, literally it isn't emitted. There is a bug in the
format that we didn't defend against -- having too many '|'s in the %diff
alternatives. I added an assert to try to catch this in r290417 and it
didn't trigger here because we never process this format.

Could you add a test covering this?

I went ahead and fixed the %diff format syntax by inspection in r290417.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits