[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

I see. Then it makes sense to do it this way. Thanks for clarifying!


https://reviews.llvm.org/D39396



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

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.

Lifting from Bob Wilson's notes: The hash value that we compute and store in
PGO profile data to detect out-of-date profiles does not include enough
information. This means that many significant changes to the source will not
cause compiler warnings about the profile being out of date, and worse, we may
continue to use the outdated profile data to make bad optimization decisions.
There is some tension here because some source changes won't affect PGO and we
don't want to invalidate the profile unnecessary.

This patch adds a new version of the PGO hash which is sensitive to 'goto',
'break', 'continue', and 'return'. It also draws a distinction between 'if'
statements with/without an 'else' block.

I chose these types of statements because their presence or absence
significantly alter control flow (to the point where any existing
profile data for the function becomes suspect). I picked these types of
statements after scanning through the subclasses of Expr and Stmt --
these seemed like the most important ones to handle. If there's
something missing, it will be easy to update the hash version again.

Profiles which use the old version of the PGO hash remain valid and can
be used without issue (there are already tests in tree for this).

rdar://17068282


https://reviews.llvm.org/D39446

Files:
  lib/CodeGen/CodeGenPGO.cpp
  test/Profile/Inputs/c-counter-overflows.proftext
  test/Profile/Inputs/c-general.proftext
  test/Profile/Inputs/cxx-throws.proftext

Index: test/Profile/Inputs/cxx-throws.proftext
===
--- test/Profile/Inputs/cxx-throws.proftext
+++ test/Profile/Inputs/cxx-throws.proftext
@@ -1,5 +1,5 @@
 _Z6throwsv
-18359008150154
+18371893052042
 9
 1
 100
Index: test/Profile/Inputs/c-general.proftext
===
--- test/Profile/Inputs/c-general.proftext
+++ test/Profile/Inputs/c-general.proftext
@@ -7,7 +7,7 @@
 75
 
 conditionals
-74917022372782735
+78295546727031439
 11
 1
 100
@@ -22,7 +22,7 @@
 100
 
 early_exits
-44128811889290
+44128811890058
 9
 1
 0
Index: test/Profile/Inputs/c-counter-overflows.proftext
===
--- test/Profile/Inputs/c-counter-overflows.proftext
+++ test/Profile/Inputs/c-counter-overflows.proftext
@@ -1,5 +1,5 @@
 main
-285734896137
+298619798025
 8
 1
 68719476720
Index: lib/CodeGen/CodeGenPGO.cpp
===
--- lib/CodeGen/CodeGenPGO.cpp
+++ lib/CodeGen/CodeGenPGO.cpp
@@ -47,6 +47,15 @@
   llvm::createPGOFuncNameMetadata(*Fn, FuncName);
 }
 
+/// The version of the PGO hash algorithm.
+enum PGOHashVersion : unsigned {
+  PGO_HASH_V1,
+  PGO_HASH_V2,
+
+  // Keep this set to the latest hash version.
+  PGO_HASH_LATEST = PGO_HASH_V2
+};
+
 namespace {
 /// \brief Stable hasher for PGO region counters.
 ///
@@ -61,6 +70,7 @@
 class PGOHash {
   uint64_t Working;
   unsigned Count;
+  PGOHashVersion HashVersion;
   llvm::MD5 MD5;
 
   static const int NumBitsPerType = 6;
@@ -93,22 +103,39 @@
 BinaryOperatorLAnd,
 BinaryOperatorLOr,
 BinaryConditionalOperator,
+// The preceding values are available with PGO_HASH_V1.
+
+GotoStmt,
+IndirectGotoStmt,
+BreakStmt,
+ContinueStmt,
+ReturnStmt,
+IfElseStmt,
+// The preceding values are available with PGO_HASH_V2.
 
 // Keep this last.  It's for the static assert that follows.
 LastHashType
   };
   static_assert(LastHashType <= TooBig, "Too many types in HashType");
 
-  // TODO: When this format changes, take in a version number here, and use the
-  // old hash calculation for file formats that used the old hash.
-  PGOHash() : Working(0), Count(0) {}
+  PGOHash(PGOHashVersion HashVersion)
+  : Working(0), Count(0), HashVersion(HashVersion), MD5() {}
   void combine(HashType Type);
   uint64_t finalize();
+  PGOHashVersion getHashVersion() const { return HashVersion; }
 };
 const int PGOHash::NumBitsPerType;
 const unsigned PGOHash::NumTypesPerWord;
 const unsigned PGOHash::TooBig;
 
+/// Get the PGO hash version used in the given indexed profile.
+static PGOHashVersion getPGOHashVersion(llvm::IndexedInstrProfReader *PGOReader,
+CodeGenModule &CGM) {
+  if (PGOReader->getVersion() <= 4)
+return PGO_HASH_V1;
+  return PGO_HASH_V2;
+}
+
 /// A RecursiveASTVisitor that fills a map of statements to PGO counters.
 struct MapRegionCounters : public RecursiveASTVisitor {
   /// The next counter value to assign.
@@ -118,8 +145,9 @@
   /// The map of statements to counters.
   llvm::DenseMap &CounterMap;
 
-  MapRegionCounters(llvm::DenseMap &CounterMap)
-  : NextCounter(0), CounterMap(CounterMap) {}
+  MapRegionCounters(PGOHashVersion HashVersion,
+llvm::DenseMap &CounterMap)
+  : NextCounter(0), Hash(HashVersion), CounterMap(CounterMap) {}
 
   // Blocks an

[PATCH] D39445: [clang-fuzzer] Fix incremental builds of the fuzzer

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
Herald added a subscriber: mgorny.

Don't use BUILD_IN_SOURCE keep git checkout clean
Don't forward CMAKE_GENERATOR as ExternalProject_Add should do it already
Reset UPDATE_COMMAND to avoid git checkout updates on each build


https://reviews.llvm.org/D39445

Files:
  clang/cmake/modules/ProtobufMutator.cmake


Index: clang/cmake/modules/ProtobufMutator.cmake
===
--- clang/cmake/modules/ProtobufMutator.cmake
+++ clang/cmake/modules/ProtobufMutator.cmake
@@ -1,23 +1,18 @@
 set(PBM_PREFIX protobuf_mutator)
 set(PBM_PATH ${CMAKE_CURRENT_BINARY_DIR}/${PBM_PREFIX}/src/${PBM_PREFIX})
-set(PBM_LIB_PATH ${PBM_PATH}/src/libprotobuf-mutator.a)
-set(PBM_FUZZ_LIB_PATH 
${PBM_PATH}/src/libfuzzer/libprotobuf-mutator-libfuzzer.a)
+set(PBM_LIB_PATH ${PBM_PATH}-build/src/libprotobuf-mutator.a)
+set(PBM_FUZZ_LIB_PATH 
${PBM_PATH}-build/src/libfuzzer/libprotobuf-mutator-libfuzzer.a)
 
 ExternalProject_Add(${PBM_PREFIX}
   PREFIX ${PBM_PREFIX}
   GIT_REPOSITORY https://github.com/google/libprotobuf-mutator.git
   GIT_TAG master
-  CONFIGURE_COMMAND ${CMAKE_COMMAND} -G${CMAKE_GENERATOR}
--DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
--DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
--DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-  BUILD_COMMAND ${CMAKE_MAKE_PROGRAM}
+  CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+  CMAKE_CACHE_ARGS -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER}
+   -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER}
   BUILD_BYPRODUCTS ${PBM_LIB_PATH} ${PBM_FUZZ_LIB_PATH}
-  BUILD_IN_SOURCE 1
+  UPDATE_COMMAND ""
   INSTALL_COMMAND ""
-  LOG_DOWNLOAD 1
-  LOG_CONFIGURE 1
-  LOG_BUILD 1
   )
 
 set(ProtobufMutator_INCLUDE_DIRS ${PBM_PATH})


Index: clang/cmake/modules/ProtobufMutator.cmake
===
--- clang/cmake/modules/ProtobufMutator.cmake
+++ clang/cmake/modules/ProtobufMutator.cmake
@@ -1,23 +1,18 @@
 set(PBM_PREFIX protobuf_mutator)
 set(PBM_PATH ${CMAKE_CURRENT_BINARY_DIR}/${PBM_PREFIX}/src/${PBM_PREFIX})
-set(PBM_LIB_PATH ${PBM_PATH}/src/libprotobuf-mutator.a)
-set(PBM_FUZZ_LIB_PATH ${PBM_PATH}/src/libfuzzer/libprotobuf-mutator-libfuzzer.a)
+set(PBM_LIB_PATH ${PBM_PATH}-build/src/libprotobuf-mutator.a)
+set(PBM_FUZZ_LIB_PATH ${PBM_PATH}-build/src/libfuzzer/libprotobuf-mutator-libfuzzer.a)
 
 ExternalProject_Add(${PBM_PREFIX}
   PREFIX ${PBM_PREFIX}
   GIT_REPOSITORY https://github.com/google/libprotobuf-mutator.git
   GIT_TAG master
-  CONFIGURE_COMMAND ${CMAKE_COMMAND} -G${CMAKE_GENERATOR}
--DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
--DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
--DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-  BUILD_COMMAND ${CMAKE_MAKE_PROGRAM}
+  CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+  CMAKE_CACHE_ARGS -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER}
+   -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER}
   BUILD_BYPRODUCTS ${PBM_LIB_PATH} ${PBM_FUZZ_LIB_PATH}
-  BUILD_IN_SOURCE 1
+  UPDATE_COMMAND ""
   INSTALL_COMMAND ""
-  LOG_DOWNLOAD 1
-  LOG_CONFIGURE 1
-  LOG_BUILD 1
   )
 
 set(ProtobufMutator_INCLUDE_DIRS ${PBM_PATH})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39284: Allow conditions to be decomposed with structured bindings

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 120922.
lichray marked an inline comment as done.
lichray added a comment.

Tweak coding style


https://reviews.llvm.org/D39284

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/DeclSpec.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/Misc/warning-flags.c
  test/Parser/cxx1z-decomposition.cpp
  test/Parser/decomposed-condition.cpp
  test/SemaCXX/decomposed-condition.cpp

Index: test/SemaCXX/decomposed-condition.cpp
===
--- /dev/null
+++ test/SemaCXX/decomposed-condition.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -std=c++1z -w -verify %s
+
+struct X {
+  bool flag;
+  int data;
+  constexpr explicit operator bool() const {
+return flag;
+  }
+  constexpr operator int() const {
+return data;
+  }
+};
+
+namespace CondInIf {
+constexpr int f(X x) {
+  if (auto [ok, d] = x)
+return d + int(ok);
+  else
+return d * int(ok);
+  ok = {}; // expected-error {{use of undeclared identifier 'ok'}}
+  d = {};  // expected-error {{use of undeclared identifier 'd'}}
+}
+
+static_assert(f({true, 2}) == 3);
+static_assert(f({false, 2}) == 0);
+
+constexpr char g(char const (&x)[2]) {
+  if (auto &[a, b] = x)
+return a;
+  else
+return b;
+
+  if (auto [a, b] = x) // expected-error {{an array type is not allowed here}}
+;
+}
+
+static_assert(g("x") == 'x');
+} // namespace CondInIf
+
+namespace CondInSwitch {
+constexpr int f(int n) {
+  switch (X s = {true, n}; auto [ok, d] = s) {
+s = {};
+  case 0:
+return int(ok);
+  case 1:
+return d * 10;
+  case 2:
+return d * 40;
+  default:
+return 0;
+  }
+  ok = {}; // expected-error {{use of undeclared identifier 'ok'}}
+  d = {};  // expected-error {{use of undeclared identifier 'd'}}
+  s = {};  // expected-error {{use of undeclared identifier 's'}}
+}
+
+static_assert(f(0) == 1);
+static_assert(f(1) == 10);
+static_assert(f(2) == 80);
+} // namespace CondInSwitch
+
+namespace CondInWhile {
+constexpr int f(int n) {
+  int m = 1;
+  while (auto [ok, d] = X{n > 1, n}) {
+m *= d;
+--n;
+  }
+  return m;
+  return ok; // expected-error {{use of undeclared identifier 'ok'}}
+}
+
+static_assert(f(0) == 1);
+static_assert(f(1) == 1);
+static_assert(f(4) == 24);
+} // namespace CondInWhile
+
+namespace CondInFor {
+constexpr int f(int n) {
+  int a = 1, b = 1;
+  for (X x = {true, n}; auto &[ok, d] = x; --d) {
+if (d < 2)
+  ok = false;
+else {
+  int x = b;
+  b += a;
+  a = x;
+}
+  }
+  return b;
+  return d; // expected-error {{use of undeclared identifier 'd'}}
+}
+
+static_assert(f(0) == 1);
+static_assert(f(1) == 1);
+static_assert(f(2) == 2);
+static_assert(f(5) == 8);
+} // namespace CondInFor
Index: test/Parser/decomposed-condition.cpp
===
--- /dev/null
+++ test/Parser/decomposed-condition.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -std=c++1z %s -verify
+
+struct Na {
+  bool flag;
+  float data;
+};
+
+struct Rst {
+  bool flag;
+  float data;
+  explicit operator bool() const {
+return flag;
+  }
+};
+
+Rst f();
+Na g();
+
+namespace CondInIf {
+void h() {
+  if (auto [ok, d] = f()) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  if (auto [ok, d] = g()) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
+;
+}
+} // namespace CondInIf
+
+namespace CondInWhile {
+void h() {
+  while (auto [ok, d] = f()) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  while (auto [ok, d] = g()) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
+;
+}
+} // namespace CondInWhile
+
+namespace CondInFor {
+void h() {
+  for (; auto [ok, d] = f();) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  for (; auto [ok, d] = g();) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
+;
+}
+} // namespace CondInFor
+
+struct IntegerLike {
+  bool flag;
+  float data;
+  operator int() const {
+return int(data);
+  }
+};
+
+namespace CondInSwitch {
+void h(IntegerLike x) {
+  switch (auto [ok, d] = x) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  switch (auto [ok, d] = g()) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{statement requires expression of integer type ('Na' invalid)}}
+;
+}
+} // namespace CondInSwitch
Index: test/Parser/cxx1z-decomposition.cpp
===
--- test/Parser/cxx1z-decomposition.cpp
+++ test/Parser/cxx1z-decomposition.cpp
@@ -32,13 +32,8 @@
   void f(auto [a, b, c]); // expected-error {{'auto'

[PATCH] D39441: [refactor][extract] insert semicolons into extracted/inserted code when needed

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
Herald added a subscriber: mgorny.

This patch implements the semicolon insertion logic into the extract 
refactoring.
The following rules are taken:

- extracting expression: add terminating ';' to the extracted function.
- extracting statements that don't require terminating ';' (e.g. switch): add 
terminating ';' to the callee.
- extracting statements with ';':  move (if possible) the original ';' from the 
callee and add terminating ';'.
- otherwise, add ';' to both places.


Repository:
  rL LLVM

https://reviews.llvm.org/D39441

Files:
  include/clang/Lex/Lexer.h
  lib/Lex/Lexer.cpp
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/Extract.cpp
  lib/Tooling/Refactoring/SourceExtraction.cpp
  lib/Tooling/Refactoring/SourceExtraction.h
  test/Refactor/Extract/ExtractExprIntoFunction.cpp
  test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
  test/Refactor/Extract/ExtractionSemicolonPolicy.m

Index: test/Refactor/Extract/ExtractionSemicolonPolicy.m
===
--- /dev/null
+++ test/Refactor/Extract/ExtractionSemicolonPolicy.m
@@ -0,0 +1,56 @@
+// RUN: clang-refactor extract -selection=test:%s %s -- 2>&1 | grep -v CHECK | FileCheck %s
+
+@interface NSArray
++ (id)arrayWithObjects:(const id [])objects count:(unsigned long)cnt;
+@end
+
+void extractStatementNoSemiObjCFor(NSArray *array) {
+  /*range astmt=->+2:4*/for (id i in array) {
+int x = 0;
+  }
+}
+// CHECK: 1 'astmt' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: for (id i in array) {
+// CHECK-NEXT: int x = 0;
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+
+void extractStatementNoSemiSync() {
+  id lock;
+  /*range bstmt=->+2:4*/@synchronized(lock) {
+int x = 0;
+  }
+}
+// CHECK: 1 'bstmt' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: @synchronized(lock) {
+// CHECK-NEXT: int x = 0;
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+
+void extractStatementNoSemiAutorel() {
+  /*range cstmt=->+2:4*/@autoreleasepool {
+int x = 0;
+  }
+}
+// CHECK: 1 'cstmt' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: @autoreleasepool {
+// CHECK-NEXT: int x = 0;
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+
+void extractStatementNoSemiTryFinally() {
+  /*range dstmt=->+3:4*/@try {
+int x = 0;
+  } @finally {
+  }
+}
+// CHECK: 1 'dstmt' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: @try {
+// CHECK-NEXT: int x = 0;
+// CHECK-NEXT: } @finally {
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
Index: test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
===
--- /dev/null
+++ test/Refactor/Extract/ExtractionSemicolonPolicy.cpp
@@ -0,0 +1,192 @@
+// RUN: clang-refactor extract -selection=test:%s %s -- -std=c++11 2>&1 | grep -v CHECK | FileCheck %s
+
+struct Rectangle { int width, height; };
+
+void extractStatement(const Rectangle &r) {
+  /*range adeclstmt=->+0:59*/int area = r.width * r.height;
+}
+// CHECK: 1 'adeclstmt' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: int area = r.width * r.height;{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void extractStatement(const Rectangle &r) {
+// CHECK-NEXT:   /*range adeclstmt=->+0:59*/extracted();{{$}}
+// CHECK-NEXT: }
+
+void extractStatementNoSemiIf(const Rectangle &r) {
+  /*range bextractif=->+2:4*/if (r.width) {
+int x = r.height;
+  }
+}
+// CHECK: 1 'bextractif' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: if (r.width) {
+// CHECK-NEXT: int x = r.height;
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void extractStatementNoSemiIf(const Rectangle &r) {
+// CHECK-NEXT:   /*range bextractif=->+2:4*/extracted();{{$}}
+// CHECK-NEXT: }
+
+void extractStatementDontExtraneousSemi(const Rectangle &r) {
+  /*range cextractif=->+2:4*/if (r.width) {
+int x = r.height;
+  } ;
+} //^ This semicolon shouldn't be extracted.
+// CHECK: 1 'cextractif' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: if (r.width) {
+// CHECK-NEXT: int x = r.height;
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void extractStatementDontExtraneousSemi(const Rectangle &r) {
+// CHECK-NEXT: extracted(); ;{{$}}
+// CHECK-NEXT: }
+
+void extractStatementNotSemiSwitch() {
+  /*range dextract=->+5:4*/switch (2) {
+  case 1:
+break;
+  case 2:
+break;
+  }
+}
+// CHECK: 1 'dextract' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: switch (2) {
+// CHECK-NEXT: case 1:
+// CHECK-NEXT:   break;
+// CHECK-NEXT: case 2:
+// CHECK-NEXT:   break;
+// CHECK-NEXT: }{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void extractStatementNotSemiSwitch() {
+// CHECK-NEXT: extracted();{{$}}
+// CHECK-NEXT: }
+
+void extractStatementNotSemiWhile() {
+  /*range eextract=->+2:4*/while (true) {
+   

r316971 - [refactor] select the entire DeclStmt if one ifs decls is selected

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Oct 30 18:28:17 2017
New Revision: 316971

URL: http://llvm.org/viewvc/llvm-project?rev=316971&view=rev
Log:
[refactor] select the entire DeclStmt if one ifs decls is selected

Modified:
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp?rev=316971&r1=316970&r2=316971&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp Mon Oct 30 18:28:17 2017
@@ -279,11 +279,23 @@ static void findDeepestWithKind(
 llvm::SmallVectorImpl &MatchingNodes,
 SourceSelectionKind Kind,
 llvm::SmallVectorImpl &ParentStack) {
-  if (!hasAnyDirectChildrenWithKind(ASTSelection, Kind)) {
-// This node is the bottom-most.
-MatchingNodes.push_back(SelectedNodeWithParents{
-std::cref(ASTSelection), {ParentStack.begin(), ParentStack.end()}});
-return;
+  if (ASTSelection.Node.get()) {
+// Select the entire decl stmt when any of its child declarations is the
+// bottom-most.
+for (const auto &Child : ASTSelection.Children) {
+  if (!hasAnyDirectChildrenWithKind(Child, Kind)) {
+MatchingNodes.push_back(SelectedNodeWithParents{
+std::cref(ASTSelection), {ParentStack.begin(), 
ParentStack.end()}});
+return;
+  }
+}
+  } else {
+if (!hasAnyDirectChildrenWithKind(ASTSelection, Kind)) {
+  // This node is the bottom-most.
+  MatchingNodes.push_back(SelectedNodeWithParents{
+  std::cref(ASTSelection), {ParentStack.begin(), ParentStack.end()}});
+  return;
+}
   }
   // Search in the children.
   ParentStack.push_back(std::cref(ASTSelection));

Modified: cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp?rev=316971&r1=316970&r2=316971&view=diff
==
--- cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ASTSelectionTest.cpp Mon Oct 30 18:28:17 2017
@@ -840,4 +840,60 @@ void f() {
   });
 }
 
+TEST(ASTSelectionFinder, SelectEntireDeclStmtRange) {
+  StringRef Source = R"(
+void f(int x, int y) {
+   int a = x * y;
+}
+)";
+  // 'int a = x * y'
+  findSelectedASTNodesWithRange(
+  Source, {3, 4}, FileRange{{3, 4}, {3, 17}},
+  [](SourceRange SelectionRange, Optional Node) {
+EXPECT_TRUE(Node);
+Optional SelectedCode =
+CodeRangeASTSelection::create(SelectionRange, std::move(*Node));
+EXPECT_TRUE(SelectedCode);
+EXPECT_EQ(SelectedCode->size(), 1u);
+EXPECT_TRUE(isa((*SelectedCode)[0]));
+ArrayRef Parents =
+SelectedCode->getParents();
+EXPECT_EQ(Parents.size(), 3u);
+EXPECT_TRUE(
+isa(Parents[0].get().Node.get()));
+// Function 'f' definition.
+EXPECT_TRUE(isa(Parents[1].get().Node.get()));
+// Function body of function 'F'.
+EXPECT_TRUE(isa(Parents[2].get().Node.get()));
+  });
+}
+
+TEST(ASTSelectionFinder, SelectEntireDeclStmtRangeWithMultipleDecls) {
+  StringRef Source = R"(
+void f(int x, int y) {
+   int a = x * y, b = x - y;
+}
+)";
+  // 'b = x - y'
+  findSelectedASTNodesWithRange(
+  Source, {3, 19}, FileRange{{3, 19}, {3, 28}},
+  [](SourceRange SelectionRange, Optional Node) {
+EXPECT_TRUE(Node);
+Optional SelectedCode =
+CodeRangeASTSelection::create(SelectionRange, std::move(*Node));
+EXPECT_TRUE(SelectedCode);
+EXPECT_EQ(SelectedCode->size(), 1u);
+EXPECT_TRUE(isa((*SelectedCode)[0]));
+ArrayRef Parents =
+SelectedCode->getParents();
+EXPECT_EQ(Parents.size(), 3u);
+EXPECT_TRUE(
+isa(Parents[0].get().Node.get()));
+// Function 'f' definition.
+EXPECT_TRUE(isa(Parents[1].get().Node.get()));
+// Function body of function 'F'.
+EXPECT_TRUE(isa(Parents[2].get().Node.get()));
+  });
+}
+
 } // end anonymous namespace


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

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Wolfgang Pieb via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D39396#911306, @aprantl wrote:

> This works for me, but as I said previously, perhaps we can get by with just 
> not having any variables described in the thunk to further simplify the code.


I remember, but this "thunk" isn't really a thunk in the sense that it makes 
some adjustments and then branches to (or calls) the real function. It 
effectively becomes the real function (hence the cloning), so if you drop the 
variables you lose the ability to debug it.


https://reviews.llvm.org/D39396



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

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> I was going to include the builtins too, but that exposes another bug (marked 
> here with FIXME) - the builtins are all defined 'const'.

Probably just need to change c->e in Builtins.def?

> This does make me curious about the use-case of libm-equivalent builtins. If 
> they are exactly identical to libm (including errno behavior), then why are 
> they needed in the first place?

It gets treated differently under -fno-builtin/-std=c89.


https://reviews.llvm.org/D39204



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

[PATCH] D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/string-with-signedness.c:1
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+

We do have a warning for this.


https://reviews.llvm.org/D39422



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

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I thought on something like this, but I still do not like my phrasing:

"Addition operator is applied to the argument of strlen(). instead of its 
result; move the '+ 1' outside of the call. (Or, if it is intentional then 
surround the addition subexpression with parentheses to silence this warning)."

Any better ideas?


https://reviews.llvm.org/D39121



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

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp marked 5 inline comments as done.
wolfgangp added inline comments.



Comment at: lib/CodeGen/CGVTables.cpp:143
+  for (llvm::Function::iterator BB = Fn->begin(), E = Fn->end(); BB != E;
+   ++BB) {
+for (llvm::Instruction &I : *BB) {

aprantl wrote:
> does this work?
> `for (auto &BB : Fn->getBasicBlockList())`
Yep, thanks. I wasn't aware of this.


https://reviews.llvm.org/D39396



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

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp updated this revision to Diff 120876.
wolfgangp added a comment.

Incorporated review comments.


https://reviews.llvm.org/D39396

Files:
  include/llvm/IR/Metadata.h
  lib/CodeGen/CGVTables.cpp
  test/CodeGenCXX/tmp-md-nodes1.cpp
  test/CodeGenCXX/tmp-md-nodes2.cpp

Index: test/CodeGenCXX/tmp-md-nodes2.cpp
===
--- test/CodeGenCXX/tmp-md-nodes2.cpp
+++ test/CodeGenCXX/tmp-md-nodes2.cpp
@@ -0,0 +1,33 @@
+// REQUIRES: asserts
+// RUN: %clang_cc1 -O0 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm %s -o - | \
+// RUN: FileCheck %s
+
+// This test simply checks that the varargs thunk is created. The failing test
+// case asserts.
+
+typedef signed char __int8_t;
+typedef int BOOL;
+class CMsgAgent;
+
+class CFs {
+public:
+  typedef enum {} CACHE_HINT;
+  virtual BOOL ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ) ;
+};
+
+typedef struct {} _Lldiv_t;
+
+class CBdVfs {
+public:
+  virtual ~CBdVfs( ) {}
+};
+
+class CBdVfsImpl : public CBdVfs, public CFs {
+  BOOL ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... );
+};
+
+BOOL CBdVfsImpl::ReqCacheHint( CMsgAgent* p_ma, CACHE_HINT hint, ... ) {
+  return true;
+}
+
+// CHECK: define {{.*}} @_ZThn8_N10CBdVfsImpl12ReqCacheHintEP9CMsgAgentN3CFs10CACHE_HINTEz(
Index: test/CodeGenCXX/tmp-md-nodes1.cpp
===
--- test/CodeGenCXX/tmp-md-nodes1.cpp
+++ test/CodeGenCXX/tmp-md-nodes1.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: asserts
+// RUN: %clang_cc1 -O0 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm %s -o - | \
+// RUN: FileCheck %s
+
+// This test simply checks that the varargs thunk is created. The failing test
+// case asserts.
+
+struct Alpha {
+  virtual void bravo(...);
+};
+struct Charlie {
+  virtual ~Charlie() {}
+};
+struct CharlieImpl : Charlie, Alpha {
+  void bravo(...) {}
+} delta;
+
+// CHECK: define {{.*}} void @_ZThn8_N11CharlieImpl5bravoEz(
Index: lib/CodeGen/CGVTables.cpp
===
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -14,11 +14,12 @@
 #include "CGCXXABI.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
-#include "clang/CodeGen/ConstantInitBuilder.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "clang/CodeGen/ConstantInitBuilder.h"
 #include "clang/Frontend/CodeGenOptions.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include 
@@ -122,6 +123,33 @@
   return RValue::get(ReturnValue);
 }
 
+/// This function clones a function's DISubprogram node and enters it into 
+/// a value map with the intent that the map can be utilized by the cloner
+/// to short-circuit Metadata node mapping.
+/// Furthermore, the function resolves any DILocalVariable nodes referenced
+/// by dbg.value intrinsics so they can be properly mapped during cloning.
+static void resolveTopLevelMetadata(llvm::Function *Fn,
+llvm::ValueToValueMapTy &VMap) {
+  // Clone the DISubprogram node and put it into the Value map.
+  auto *DIS = Fn->getSubprogram();
+  if (!DIS)
+return;
+  auto *NewDIS = DIS->replaceWithDistinct(DIS->clone());
+  VMap.MD()[DIS].reset(NewDIS);
+
+  // Find all llvm.dbg.declare intrinsics and resolve the DILocalVariable nodes
+  // they are referencing.
+  for (auto &BB : Fn->getBasicBlockList()) {
+for (auto &I : BB) {
+  if (auto *DII = dyn_cast(&I)) {
+auto *DILocal = DII->getVariable();
+if (!DILocal->isResolved())
+  DILocal->resolve();
+  }
+}
+  }
+}
+
 // This function does roughly the same thing as GenerateThunk, but in a
 // very different way, so that va_start and va_end work correctly.
 // FIXME: This function assumes "this" is the first non-sret LLVM argument of
@@ -154,6 +182,10 @@
 
   // Clone to thunk.
   llvm::ValueToValueMapTy VMap;
+
+  // We are cloning a function while some Metadata nodes are still unresolved.
+  // Ensure that the value mapper does not encounter any of them.
+  resolveTopLevelMetadata(BaseFn, VMap);
   llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap);
   Fn->replaceAllUsesWith(NewFn);
   NewFn->takeName(Fn);
Index: include/llvm/IR/Metadata.h
===
--- include/llvm/IR/Metadata.h
+++ include/llvm/IR/Metadata.h
@@ -958,6 +958,9 @@
   /// \pre No operands (or operands' operands, etc.) have \a isTemporary().
   void resolveCycles();
 
+  /// Resolve a unique, unresolved node.
+  void resolve();
+
   /// \brief Replace a temporary node with a permanent one.
   ///
   /// Try to create a uniqued version of \c N -- in place, if possible -- and
@@ -1009,9 +1012,6 @@
 private:
   void handleChangedOperand(void *Ref, Metadata *New);
 
-  /// Resolve a unique, unresolve

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Adrian Prantl via Phabricator via cfe-commits]

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

This works for me, but as I said previously, perhaps we can get by with just 
not having any variables described in the thunk to further simplify the code.


https://reviews.llvm.org/D39396



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

[libcxx] r316970 - Fix broken links; update more issues.

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Oct 30 17:19:47 2017
New Revision: 316970

URL: http://llvm.org/viewvc/llvm-project?rev=316970&view=rev
Log:
Fix broken links; update more issues.


Modified:
libcxx/trunk/www/upcoming_meeting.html

Modified: libcxx/trunk/www/upcoming_meeting.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/upcoming_meeting.html?rev=316970&r1=316969&r2=316970&view=diff
==
--- libcxx/trunk/www/upcoming_meeting.html (original)
+++ libcxx/trunk/www/upcoming_meeting.html Mon Oct 30 17:19:47 2017
@@ -50,7 +50,7 @@
   
Paper #GroupPaper 
NameMeetingStatusFirst released version
 
 
   
@@ -59,38 +59,38 @@
   
Issue #Issue 
NameMeetingStatus
 
-   https://wg21.link/2779";>2779[networking.ts] Relax 
requirements on buffer sequence iteratorsAlbuquerque
-   https://wg21.link/2870";>2870Default value 
of parameter theta of polar should be 
dependentAlbuquerque
-   https://wg21.link/2935";>2935What should 
create_directories do when p already exists but is not a 
directory?Albuquerque
-   https://wg21.link/2941";>2941§[thread.req.timing] wording 
should apply to both member and namespace-level 
functionsAlbuquerque
-   https://wg21.link/2944";>2944LWG 2905 
accidentally removed requirement that construction of the deleter doesn't throw 
an exceptionAlbuquerque
-   https://wg21.link/2945";>2945Order of 
template parameters in optional 
comparisonsAlbuquerque
-   https://wg21.link/2948";>2948unique_ptr 
does not define operator<< for stream 
outputAlbuquerque
-   https://wg21.link/2950";>2950std::byte 
operations are misspecifiedAlbuquerquePatch Ready
-   https://wg21.link/2952";>2952iterator_traits should work for 
pointers to cv TAlbuquerque
-   https://wg21.link/2953";>2953LWG 2853 
should apply to deque::erase tooAlbuquerque
-   https://wg21.link/2964";>2964Apparently 
redundant requirement for 
dynamic_pointer_castAlbuquerque
-   https://wg21.link/2965";>2965Non-existing 
path::native_string() in filesystem_error::what() 
specificationAlbuquerque
-   https://wg21.link/2972";>2972What is 
is_trivially_destructible_v?Albuquerque
-   https://wg21.link/2976";>2976Dangling 
uses_allocator specialization for 
packaged_taskAlbuquerque
-   https://wg21.link/2977";>2977unordered_meow::merge() has 
incorrect Throws: clauseAlbuquerque
-   https://wg21.link/2978";>2978Hash support 
for pmr::string and friendsAlbuquerque
-   https://wg21.link/2979";>2979aligned_union 
should require complete object typesAlbuquerque
-   https://wg21.link/2980";>2980Cannot 
compare_exchange empty pointersAlbuquerque
-   https://wg21.link/2981";>2981Remove 
redundant deduction guides from standard 
libraryAlbuquerque
-   https://wg21.link/2982";>2982Making 
size_type consistent in associative container deduction 
guidesAlbuquerque
-   https://wg21.link/2988";>2988Clause 32 
cleanup missed one typenameAlbuquerque
-   https://wg21.link/2993";>2993reference_wrapper conversion 
from T&&Albuquerque
-   https://wg21.link/2998";>2998Requirements 
on function objects passed to {forward_,}list-specific 
algorithmsAlbuquerque
-   https://wg21.link/3001";>3001weak_ptr::element_type needs 
remove_extent_tAlbuquerque
-   https://wg21.link/3024";>3024variant's 
copies must be deleted instead of disabled via 
SFINAEAlbuquerque
+   https://wg21.link/LWG2779";>2779[networking.ts] Relax 
requirements on buffer sequence iteratorsAlbuquerque
+   https://wg21.link/LWG2870";>2870Default 
value of parameter theta of polar should be 
dependentAlbuquerque
+   https://wg21.link/LWG2935";>2935What 
should create_directories do when p already exists but is not a 
directory?Albuquerque
+   https://wg21.link/LWG2941";>2941§[thread.req.timing] wording 
should apply to both member and namespace-level 
functionsAlbuquerque
+   https://wg21.link/LWG2944";>2944LWG 2905 
accidentally removed requirement that construction of the deleter doesn't throw 
an exceptionAlbuquerque
+   https://wg21.link/LWG2945";>2945Order of 
template parameters in optional 
comparisonsAlbuquerque
+   https://wg21.link/LWG2948";>2948unique_ptr 
does not define operator<< for stream 
outputAlbuquerque
+   https://wg21.link/LWG2950";>2950std::byte 
operations are misspecifiedAlbuquerquePatch Ready
+   https://wg21.link/LWG2952";>2952iterator_traits should work 
for pointers to cv TAlbuquerque
+   https://wg21.link/LWG2953";>2953LWG 2853 
should apply to deque::erase tooAlbuquerque
+   https://wg21.link/LWG2964";>2964Apparently 
redundant requirement for 
dynamic_pointer_castAlbuquerque
+   https://wg21.link/LWG2965";>2965Non-existing 
path::native_string() in filesystem_error::what() 
specificationAlbuquerque
+   https://wg21.link/LWG2972";>2972What is 
is_trivially_destructible_v?AlbuquerqueComplete
+   https://wg21.link/LWG2976";>297

[PATCH] D39284: Allow conditions to be decomposed with structured bindings

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:696-698
+  // a for-range-declaration, or a condition in C++2a, but we parse it in more
+  // cases than that.
+  if (!D.mayHaveDecompositionDeclarator(getLangOpts())) {

lichray wrote:
> rsmith wrote:
> > Again, please don't guess what's going to be in C++2a. I would actually 
> > expect that we'll vote this in as a DR against C++17. It seems like a 
> > wording oversight to me.
> I would say, use DR with parsimony... But OK.
> 
> OT: I'm writing a paper on `auto(x)` and `auto{x}` while implementing it.  Do 
> you expect this to land as a DR?  In which form you expect it to appear in 
> Clang?
For that one, I don't know. The status quo is inconsistent (particularly 
comparing `auto` to class template argument deduction), but I don't think the 
inconsistency is a wording oversight. If this were to land in Clang prior to 
being voted into C++2a, I'd expect it to produce an `ExtWarn` that's not tied 
to any particular C++ version.


https://reviews.llvm.org/D39284



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

[libcxx] r316969 - Add a fail test for aligned_union of an incomplete type. See LWG#2979. NFC

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Oct 30 17:05:17 2017
New Revision: 316969

URL: http://llvm.org/viewvc/llvm-project?rev=316969&view=rev
Log:
Add a fail test for aligned_union of an incomplete type. See LWG#2979. NFC

Added:

libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_union.fail.cpp

Added: 
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_union.fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_union.fail.cpp?rev=316969&view=auto
==
--- 
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_union.fail.cpp
 (added)
+++ 
libcxx/trunk/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_union.fail.cpp
 Mon Oct 30 17:05:17 2017
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// type_traits
+
+// aligned_union
+
+#include 
+
+class A; // Incomplete
+
+int main()
+{
+typedef std::aligned_union<10, A>::type T1;
+}


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

[PATCH] D39284: Allow conditions to be decomposed with structured bindings

[Zhihao Yuan via Phabricator via cfe-commits]

lichray marked 3 inline comments as done.
lichray added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:696-698
+  // a for-range-declaration, or a condition in C++2a, but we parse it in more
+  // cases than that.
+  if (!D.mayHaveDecompositionDeclarator(getLangOpts())) {

rsmith wrote:
> Again, please don't guess what's going to be in C++2a. I would actually 
> expect that we'll vote this in as a DR against C++17. It seems like a wording 
> oversight to me.
I would say, use DR with parsimony... But OK.

OT: I'm writing a paper on `auto(x)` and `auto{x}` while implementing it.  Do 
you expect this to land as a DR?  In which form you expect it to appear in 
Clang?


https://reviews.llvm.org/D39284



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

[PATCH] D39284: [c++2a] Decomposed _condition_

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 120913.
lichray added a comment.

Make the feature unconditional with an `ExtWarn`


https://reviews.llvm.org/D39284

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/DeclSpec.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/Misc/warning-flags.c
  test/Parser/cxx1z-decomposition.cpp
  test/Parser/decomposed-condition.cpp
  test/SemaCXX/decomposed-condition.cpp

Index: test/SemaCXX/decomposed-condition.cpp
===
--- /dev/null
+++ test/SemaCXX/decomposed-condition.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -std=c++1z -w -verify %s
+
+struct X {
+  bool flag;
+  int data;
+  constexpr explicit operator bool() const {
+return flag;
+  }
+  constexpr operator int() const {
+return data;
+  }
+};
+
+namespace CondInIf {
+constexpr int f(X x) {
+  if (auto [ok, d] = x)
+return d + int(ok);
+  else
+return d * int(ok);
+  ok = {}; // expected-error {{use of undeclared identifier 'ok'}}
+  d = {};  // expected-error {{use of undeclared identifier 'd'}}
+}
+
+static_assert(f({true, 2}) == 3);
+static_assert(f({false, 2}) == 0);
+
+constexpr char g(char const (&x)[2]) {
+  if (auto &[a, b] = x)
+return a;
+  else
+return b;
+
+  if (auto [a, b] = x) // expected-error {{an array type is not allowed here}}
+;
+}
+
+static_assert(g("x") == 'x');
+} // namespace CondInIf
+
+namespace CondInSwitch {
+constexpr int f(int n) {
+  switch (X s = {true, n}; auto [ok, d] = s) {
+s = {};
+  case 0:
+return int(ok);
+  case 1:
+return d * 10;
+  case 2:
+return d * 40;
+  default:
+return 0;
+  }
+  ok = {}; // expected-error {{use of undeclared identifier 'ok'}}
+  d = {};  // expected-error {{use of undeclared identifier 'd'}}
+  s = {};  // expected-error {{use of undeclared identifier 's'}}
+}
+
+static_assert(f(0) == 1);
+static_assert(f(1) == 10);
+static_assert(f(2) == 80);
+} // namespace CondInSwitch
+
+namespace CondInWhile {
+constexpr int f(int n) {
+  int m = 1;
+  while (auto [ok, d] = X{n > 1, n}) {
+m *= d;
+--n;
+  }
+  return m;
+  return ok; // expected-error {{use of undeclared identifier 'ok'}}
+}
+
+static_assert(f(0) == 1);
+static_assert(f(1) == 1);
+static_assert(f(4) == 24);
+} // namespace CondInWhile
+
+namespace CondInFor {
+constexpr int f(int n) {
+  int a = 1, b = 1;
+  for (X x = {true, n}; auto &[ok, d] = x; --d) {
+if (d < 2)
+  ok = false;
+else {
+  int x = b;
+  b += a;
+  a = x;
+}
+  }
+  return b;
+  return d; // expected-error {{use of undeclared identifier 'd'}}
+}
+
+static_assert(f(0) == 1);
+static_assert(f(1) == 1);
+static_assert(f(2) == 2);
+static_assert(f(5) == 8);
+} // namespace CondInFor
Index: test/Parser/decomposed-condition.cpp
===
--- /dev/null
+++ test/Parser/decomposed-condition.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -std=c++1z %s -verify
+
+struct Na {
+  bool flag;
+  float data;
+};
+
+struct Rst {
+  bool flag;
+  float data;
+  explicit operator bool() const {
+return flag;
+  }
+};
+
+Rst f();
+Na g();
+
+namespace CondInIf {
+void h() {
+  if (auto [ok, d] = f()) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  if (auto [ok, d] = g()) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
+;
+}
+} // namespace CondInIf
+
+namespace CondInWhile {
+void h() {
+  while (auto [ok, d] = f()) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  while (auto [ok, d] = g()) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
+;
+}
+} // namespace CondInWhile
+
+namespace CondInFor {
+void h() {
+  for (; auto [ok, d] = f();) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  for (; auto [ok, d] = g();) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
+;
+}
+} // namespace CondInFor
+
+struct IntegerLike {
+  bool flag;
+  float data;
+  operator int() const {
+return int(data);
+  }
+};
+
+namespace CondInSwitch {
+void h(IntegerLike x) {
+  switch (auto [ok, d] = x) // expected-warning {{decomposed condition is a Clang extension}}
+;
+  switch (auto [ok, d] = g()) // expected-warning {{decomposed condition is a Clang extension}} expected-error {{statement requires expression of integer type ('Na' invalid)}}
+;
+}
+} // namespace CondInSwitch
Index: test/Parser/cxx1z-decomposition.cpp
===
--- test/Parser/cxx1z-decomposition.cpp
+++ test/Parser/cxx1z-decomposition.cpp
@@ -32,13 +32,8 @@
   void f(auto [a, b, c]); // expected-error {{'auto' not allowed

r316966 - Typo correct the condition of 'do-while' before exiting its scope

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Oct 30 15:55:11 2017
New Revision: 316966

URL: http://llvm.org/viewvc/llvm-project?rev=316966&view=rev
Log:
Typo correct the condition of 'do-while' before exiting its scope

rdar://35172419

Modified:
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/test/SemaObjCXX/typo-correction.mm

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=316966&r1=316965&r2=316966&view=diff
==
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Mon Oct 30 15:55:11 2017
@@ -1479,6 +1479,9 @@ StmtResult Parser::ParseDoStatement() {
   DiagnoseAndSkipCXX11Attributes();
 
   ExprResult Cond = ParseExpression();
+  // Correct the typos in condition before closing the scope.
+  if (Cond.isUsable())
+Cond = Actions.CorrectDelayedTyposInExpr(Cond);
   T.consumeClose();
   DoScope.Exit();
 

Modified: cfe/trunk/test/SemaObjCXX/typo-correction.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/typo-correction.mm?rev=316966&r1=316965&r2=316966&view=diff
==
--- cfe/trunk/test/SemaObjCXX/typo-correction.mm (original)
+++ cfe/trunk/test/SemaObjCXX/typo-correction.mm Mon Oct 30 15:55:11 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Wno-objc-root-class
 
 class ClassA {};
 
@@ -55,3 +55,35 @@ void invalidNameInIvarAndPropertyBase()
   typoCandidate.x = 0; // expected-error {{use of undeclared identifier 
'typoCandidate'; did you mean '_typoCandidate'?}}
 }
 @end
+
+// rdar://35172419
+// The scope of 'do-while' ends before typo-correction takes place.
+
+struct Mat2 { int rows; };
+
+@implementation ImplNoInt // expected-warning {{cannot find interface 
declaration for 'ImplNoInt'}}
+
+- (void)typoCorrentInDoWhile {
+  Mat2 tlMat1; // expected-note {{'tlMat1' declared here}}
+  // Create many scopes to exhaust the cache.
+  do {
+for (int index = 0; index < 2; index++) {
+  if (true) {
+for (int specialTileType = 1; specialTileType < 5; specialTileType++) {
+  for (int i = 0; i < 10; i++) {
+for (double scale = 0.95; scale <= 1.055; scale += 0.05) {
+  for (int j = 0; j < 10; j++) {
+if (1 > 0.9) {
+for (int sptile = 1; sptile < 5; sptile++) {
+}
+}
+  }
+}
+  }
+}
+  }
+}
+  } while (tlMat.rows); // expected-error {{use of undeclared identifier 
'tlMat'; did you mean 'tlMat1'}}
+}
+
+@end


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

[PATCH] D39438: [analyzer] Diagnose stack leaks via block captures

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap updated this revision to Diff 120908.
alexshap added a comment.

Add positive tests


Repository:
  rL LLVM

https://reviews.llvm.org/D39438

Files:
  lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  test/Analysis/stack-async-leak.m

Index: test/Analysis/stack-async-leak.m
===
--- test/Analysis/stack-async-leak.m
+++ test/Analysis/stack-async-leak.m
@@ -0,0 +1,97 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core -analyzer-store=region -fblocks -verify %s
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef void (^dispatch_block_t)(void);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+typedef long dispatch_once_t;
+void dispatch_once(dispatch_once_t *predicate, dispatch_block_t block);
+typedef long dispatch_time_t;
+void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
+
+extern dispatch_queue_t queue;
+extern dispatch_once_t *predicate;
+extern dispatch_time_t when;
+
+void test_block_expr_async() {
+  int x = 123;
+  int *p = &x;
+
+  dispatch_async(queue, ^{
+*p = 321;
+  });
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_expr_once() {
+  int x = 123;
+  int *p = &x;
+  dispatch_once(predicate, ^{
+*p = 321;
+  });
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_expr_after() {
+  int x = 123;
+  int *p = &x;
+  dispatch_after(when, queue, ^{
+*p = 321;
+  });
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_expr_async_no_leak() {
+  int x = 123;
+  int *p = &x;
+
+  dispatch_async(queue, ^{
+int y = x;
+++y;
+  });
+}
+
+void test_block_var_async() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+*p = 1; 
+  };
+  dispatch_async(queue, b);
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_var_once() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+*p = 1; 
+  };
+  dispatch_once(predicate, b);
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_var_after() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+*p = 1; 
+  };
+  dispatch_after(when, queue, b);
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_var_async_no_leak() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+int y = x;
+++y; 
+  };
+  dispatch_async(queue, b);
+}
Index: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -18,20 +18,24 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 using namespace ento;
 
 namespace {
-class StackAddrEscapeChecker : public Checker< check::PreStmt,
+class StackAddrEscapeChecker : public Checker< check::PreCall,
+   check::PreStmt,
check::EndFunction > {
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
+  mutable std::unique_ptr BT_capturestackleak;
 
 public:
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
   void checkEndFunction(CheckerContext &Ctx) const;
 private:
@@ -92,8 +96,9 @@
   return range;
 }
 
-void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, const MemRegion *R,
-  const Expr *RetE) const {
+void StackAddrEscapeChecker::EmitStackError(CheckerContext &C,
+const MemRegion *R,
+const Expr *RetE) const {
   ExplodedNode *N = C.generateErrorNode();
 
   if (!N)
@@ -116,6 +121,47 @@
   C.em

Re: r315402 - [modules] Only take visible using-directives into account during name lookup.

[Vassil Vassilev via cfe-commits]

On 30/10/17 23:40, Richard Smith wrote:
On 30 October 2017 at 15:12, Vassil Vassilev via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:


On 11/10/17 03:19, Richard Smith via cfe-commits wrote:

Author: rsmith
Date: Tue Oct 10 18:19:11 2017
New Revision: 315402

URL: http://llvm.org/viewvc/llvm-project?rev=315402&view=rev

Log:
[modules] Only take visible using-directives into account
during name lookup.

Added:
     cfe/trunk/test/Modules/using-directive.cpp
Modified:
     cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=315402&r1=315401&r2=315402&view=diff



==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Oct 10 18:19:11 2017
@@ -88,13 +88,15 @@ namespace {
    /// A collection of using directives, as used by C++
unqualified
    /// lookup.
    class UnqualUsingDirectiveSet {
+    Sema &SemaRef;
+
      typedef SmallVector ListTy;
        ListTy list;
      llvm::SmallPtrSet visited;
      public:
-    UnqualUsingDirectiveSet() {}
+    UnqualUsingDirectiveSet(Sema &SemaRef) : SemaRef(SemaRef) {}
        void visitScopeChain(Scope *S, Scope
*InnermostFileScope) {
        // C++ [namespace.udir]p1:
@@ -113,7 +115,8 @@ namespace {
            visit(Ctx, Ctx);
          } else if (!Ctx || Ctx->isFunctionOrMethod()) {
            for (auto *I : S->using_directives())
-            visit(I, InnermostFileDC);
+            if (SemaRef.isVisible(I))
+              visit(I, InnermostFileDC);
          }
        }
      }
@@ -152,7 +155,7 @@ namespace {
        while (true) {
          for (auto UD : DC->using_directives()) {
            DeclContext *NS = UD->getNominatedNamespace();
-          if (visited.insert(NS).second) {
+          if (visited.insert(NS).second &&
SemaRef.isVisible(UD)) {

  It looks like this change breaks examples such as:

// RUN: rm -fr %t
// RUN %clang_cc1 -fmodules -fmodules-local-submodule-visibility
-fmodules-cache-path=%t -verify %s
// expected-no-diagnostics
#pragma clang module build M
module M { module TDFNodes {} module TDFInterface {} }
#pragma clang module contents
  // TDFNodes
  #pragma clang module begin M.TDFNodes
  namespace Detail {
 namespace TDF {
    class TLoopManager {};
 }
  }
  namespace Internal {
 namespace TDF {
    using namespace Detail::TDF;
 }
  }
  #pragma clang module end

  // TDFInterface
  #pragma clang module begin M.TDFInterface
    #pragma clang module import M.TDFNodes
  namespace Internal {
    namespace TDF {
  using namespace Detail::TDF;
    }
  }
  #pragma clang module end

#pragma clang module endbuild

#pragma clang module import M.TDFNodes
namespace Internal {
  namespace TDF {
    TLoopManager * use;
  }
}

  I suspect that it triggers a preexisting bug in failing to make
M.TDFNodes visible...


This code in NamedDecl::declarationReplaces is wrong, we even have a 
FIXME for the bug:


  // UsingDirectiveDecl's are not really NamedDecl's, and all have 
same name.

  // They can be replaced if they nominate the same namespace.
  // FIXME: Is this true even if they have different module visibility?
  if (auto *UD = dyn_cast(this))
    return UD->getNominatedNamespace()->getOriginalNamespace() ==
 cast(OldD)->getNominatedNamespace()
               ->getOriginalNamespace();

Fixed in r316965 by deleting the above code :)
You ninjaed me I just understood that the getOriginalNamespace was wrong 
and hunting that down :D


Thanks for the quick fix!


This might introduce a performance regression in the presence of large 
numbers of duplicated using-directives. We can rectify that, if 
needed, by making using-directives redeclarable, but I'm not expecting 
that to be a problem outside of pathological cases.


              addUsingDirective(UD, EffectiveDC);
              queue.push_back(NS);
            }
@@ -1085,7 +1088,7 @@ bool Sema::CppLookupName(LookupResult &R
    //   }
    // }
    //
-  UnqualUsingDirectiveSet UDirs;
+  UnqualUsingDirectiveSet UDirs(*th

[PATCH] D39438: [analyzer] Diagnose stack leaks via block captures

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap created this revision.
Herald added subscribers: szepet, xazax.hun.

This diff extends StackAddrEscapeChecker to catch stack addr leaks via block 
captures if the block is executed
asynchronously. 
Test plan: make check-all


Repository:
  rL LLVM

https://reviews.llvm.org/D39438

Files:
  lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  test/Analysis/stack-async-leak.m

Index: test/Analysis/stack-async-leak.m
===
--- test/Analysis/stack-async-leak.m
+++ test/Analysis/stack-async-leak.m
@@ -0,0 +1,77 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core -analyzer-store=region -fblocks -verify %s
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef void (^dispatch_block_t)(void);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+typedef long dispatch_once_t;
+void dispatch_once(dispatch_once_t *predicate, dispatch_block_t block);
+typedef long dispatch_time_t;
+void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
+
+extern dispatch_queue_t queue;
+extern dispatch_once_t *predicate;
+extern dispatch_time_t when;
+
+void test_block_expr_async() {
+  int x = 123;
+  int *p = &x;
+
+  dispatch_async(queue, ^{
+*p = 321;
+  });
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_expr_once() {
+  int x = 123;
+  int *p = &x;
+  dispatch_once(predicate, ^{
+*p = 321;
+  });
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_expr_after() {
+  int x = 123;
+  int *p = &x;
+  dispatch_after(when, queue, ^{
+*p = 321;
+  });
+  // expected-warning@-3 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_var_async() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+*p = 1; 
+  };
+  dispatch_async(queue, b);
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_var_once() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+*p = 1; 
+  };
+  dispatch_once(predicate, b);
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
+
+void test_block_var_after() {
+  int x = 123;
+  int *p = &x;
+  void (^b)(void) = ^void(void) {
+*p = 1; 
+  };
+  dispatch_after(when, queue, b);
+  // expected-warning@-1 {{Address of stack memory associated with local variable 'x' was captured by the block \
+which will be executed asynchronously}}
+}
Index: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -18,20 +18,24 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 using namespace ento;
 
 namespace {
-class StackAddrEscapeChecker : public Checker< check::PreStmt,
+class StackAddrEscapeChecker : public Checker< check::PreCall,
+   check::PreStmt,
check::EndFunction > {
   mutable std::unique_ptr BT_stackleak;
   mutable std::unique_ptr BT_returnstack;
+  mutable std::unique_ptr BT_capturestackleak;
 
 public:
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
   void checkEndFunction(CheckerContext &Ctx) const;
 private:
@@ -92,8 +96,9 @@
   return range;
 }
 
-void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, const MemRegion *R,
-  const Expr *RetE) const {
+void StackAddrEscapeChecker::EmitStackError(CheckerContext &C,
+const MemRegion *R,
+const Expr *RetE) const {
   ExplodedNode *N = C.generateErrorNode();
 
   if (!N)
@@ -116,6 +121,47 @@
   C.emitReport(std::move(report));
 }
 
+void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
+  CheckerContext &C) const {
+  

Re: r315402 - [modules] Only take visible using-directives into account during name lookup.

[Richard Smith via cfe-commits]

On 30 October 2017 at 15:12, Vassil Vassilev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On 11/10/17 03:19, Richard Smith via cfe-commits wrote:
>
>> Author: rsmith
>> Date: Tue Oct 10 18:19:11 2017
>> New Revision: 315402
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315402&view=rev
>> Log:
>> [modules] Only take visible using-directives into account during name
>> lookup.
>>
>> Added:
>>  cfe/trunk/test/Modules/using-directive.cpp
>> Modified:
>>  cfe/trunk/lib/Sema/SemaLookup.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaL
>> ookup.cpp?rev=315402&r1=315401&r2=315402&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Oct 10 18:19:11 2017
>> @@ -88,13 +88,15 @@ namespace {
>> /// A collection of using directives, as used by C++ unqualified
>> /// lookup.
>> class UnqualUsingDirectiveSet {
>> +Sema &SemaRef;
>> +
>>   typedef SmallVector ListTy;
>> ListTy list;
>>   llvm::SmallPtrSet visited;
>>   public:
>> -UnqualUsingDirectiveSet() {}
>> +UnqualUsingDirectiveSet(Sema &SemaRef) : SemaRef(SemaRef) {}
>> void visitScopeChain(Scope *S, Scope *InnermostFileScope) {
>> // C++ [namespace.udir]p1:
>> @@ -113,7 +115,8 @@ namespace {
>> visit(Ctx, Ctx);
>>   } else if (!Ctx || Ctx->isFunctionOrMethod()) {
>> for (auto *I : S->using_directives())
>> -visit(I, InnermostFileDC);
>> +if (SemaRef.isVisible(I))
>> +  visit(I, InnermostFileDC);
>>   }
>> }
>>   }
>> @@ -152,7 +155,7 @@ namespace {
>> while (true) {
>>   for (auto UD : DC->using_directives()) {
>> DeclContext *NS = UD->getNominatedNamespace();
>> -  if (visited.insert(NS).second) {
>> +  if (visited.insert(NS).second && SemaRef.isVisible(UD)) {
>>
>   It looks like this change breaks examples such as:
>
> // RUN: rm -fr %t
> // RUN %clang_cc1 -fmodules -fmodules-local-submodule-visibility
> -fmodules-cache-path=%t -verify %s
> // expected-no-diagnostics
> #pragma clang module build M
> module M { module TDFNodes {} module TDFInterface {} }
> #pragma clang module contents
>   // TDFNodes
>   #pragma clang module begin M.TDFNodes
>   namespace Detail {
>  namespace TDF {
> class TLoopManager {};
>  }
>   }
>   namespace Internal {
>  namespace TDF {
> using namespace Detail::TDF;
>  }
>   }
>   #pragma clang module end
>
>   // TDFInterface
>   #pragma clang module begin M.TDFInterface
> #pragma clang module import M.TDFNodes
>   namespace Internal {
> namespace TDF {
>   using namespace Detail::TDF;
> }
>   }
>   #pragma clang module end
>
> #pragma clang module endbuild
>
> #pragma clang module import M.TDFNodes
> namespace Internal {
>   namespace TDF {
> TLoopManager * use;
>   }
> }
>
>   I suspect that it triggers a preexisting bug in failing to make
> M.TDFNodes visible...


This code in NamedDecl::declarationReplaces is wrong, we even have a FIXME
for the bug:

  // UsingDirectiveDecl's are not really NamedDecl's, and all have same
name.
  // They can be replaced if they nominate the same namespace.
  // FIXME: Is this true even if they have different module visibility?
  if (auto *UD = dyn_cast(this))
return UD->getNominatedNamespace()->getOriginalNamespace() ==
   cast(OldD)->getNominatedNamespace()
   ->getOriginalNamespace();

Fixed in r316965 by deleting the above code :)

This might introduce a performance regression in the presence of large
numbers of duplicated using-directives. We can rectify that, if needed, by
making using-directives redeclarable, but I'm not expecting that to be a
problem outside of pathological cases.


>   addUsingDirective(UD, EffectiveDC);
>>   queue.push_back(NS);
>> }
>> @@ -1085,7 +1088,7 @@ bool Sema::CppLookupName(LookupResult &R
>> //   }
>> // }
>> //
>> -  UnqualUsingDirectiveSet UDirs;
>> +  UnqualUsingDirectiveSet UDirs(*this);
>> bool VisitedUsingDirectives = false;
>> bool LeftStartingScope = false;
>> DeclContext *OutsideOfTemplateParamDC = nullptr;
>> @@ -1868,22 +1871,19 @@ static bool LookupQualifiedNameInUsingDi
>>DeclContext *StartDC) {
>> assert(StartDC->isFileContext() && "start context is not a file
>> context");
>>   -  DeclContext::udir_range UsingDirectives =
>> StartDC->using_directives();
>> -  if (UsingDirectives.begin() == UsingDirectives.end()) return false;
>> +  // We have not yet looked into these namespaces, much less added
>> +  // their "using-children" to the queue.
>> +  SmallVector Queue;
>>   // We have at least added all these contexts to 

Re: r316965 - [modules] Retain multiple using-directives in the same scope even if they name the same namespace.

[Richard Smith via cfe-commits]

I should add: the test is courtesy of Vassil Vassilev. Thanks!

On 30 October 2017 at 15:38, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Mon Oct 30 15:38:20 2017
> New Revision: 316965
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316965&view=rev
> Log:
> [modules] Retain multiple using-directives in the same scope even if they
> name the same namespace.
>
> They might have different visibility, and thus discarding all but one of
> them
> can result in rejecting valid code. Also fix name lookup to cope with
> multiple
> using-directives being found that denote the same namespace, where some
> are not
> visible -- don't cache an "already visited" state for a using-directive
> that we
> didn't visit because it was hidden.
>
> Added:
> cfe/trunk/test/Modules/using-directive-redecl.cpp
> Modified:
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/Sema/SemaLookup.cpp
>
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> Decl.cpp?rev=316965&r1=316964&r2=316965&view=diff
> 
> ==
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Mon Oct 30 15:38:20 2017
> @@ -1597,14 +1597,6 @@ bool NamedDecl::declarationReplaces(Name
>  cast
> (OldD)->getQualifier());
>}
>
> -  // UsingDirectiveDecl's are not really NamedDecl's, and all have same
> name.
> -  // They can be replaced if they nominate the same namespace.
> -  // FIXME: Is this true even if they have different module visibility?
> -  if (auto *UD = dyn_cast(this))
> -return UD->getNominatedNamespace()->getOriginalNamespace() ==
> -   cast(OldD)->getNominatedNamespace()
> -   ->getOriginalNamespace();
> -
>if (isRedeclarable(getKind())) {
>  if (getCanonicalDecl() != OldD->getCanonicalDecl())
>return false;
>
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaLookup.cpp?rev=316965&r1=316964&r2=316965&view=diff
> 
> ==
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Oct 30 15:38:20 2017
> @@ -155,7 +155,7 @@ namespace {
>while (true) {
>  for (auto UD : DC->using_directives()) {
>DeclContext *NS = UD->getNominatedNamespace();
> -  if (visited.insert(NS).second && SemaRef.isVisible(UD)) {
> +  if (SemaRef.isVisible(UD) && visited.insert(NS).second) {
>  addUsingDirective(UD, EffectiveDC);
>  queue.push_back(NS);
>}
> @@ -1883,7 +1883,7 @@ static bool LookupQualifiedNameInUsingDi
>// with its using-children.
>for (auto *I : StartDC->using_directives()) {
>  NamespaceDecl *ND = I->getNominatedNamespace()->
> getOriginalNamespace();
> -if (Visited.insert(ND).second && S.isVisible(I))
> +if (S.isVisible(I) && Visited.insert(ND).second)
>Queue.push_back(ND);
>}
>
> @@ -1931,7 +1931,7 @@ static bool LookupQualifiedNameInUsingDi
>
>  for (auto I : ND->using_directives()) {
>NamespaceDecl *Nom = I->getNominatedNamespace();
> -  if (Visited.insert(Nom).second && S.isVisible(I))
> +  if (S.isVisible(I) && Visited.insert(Nom).second)
>  Queue.push_back(Nom);
>  }
>}
>
> Added: cfe/trunk/test/Modules/using-directive-redecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/using-directive-redecl.cpp?rev=316965&view=auto
> 
> ==
> --- cfe/trunk/test/Modules/using-directive-redecl.cpp (added)
> +++ cfe/trunk/test/Modules/using-directive-redecl.cpp Mon Oct 30 15:38:20
> 2017
> @@ -0,0 +1,37 @@
> +// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility
> -verify %s
> +// expected-no-diagnostics
> +#pragma clang module build M
> +module M { module TDFNodes {} module TDFInterface {} }
> +#pragma clang module contents
> +  // TDFNodes
> +  #pragma clang module begin M.TDFNodes
> +  namespace Detail {
> + namespace TDF {
> +class TLoopManager {};
> + }
> +  }
> +  namespace Internal {
> + namespace TDF {
> +using namespace Detail::TDF;
> + }
> +  }
> +  #pragma clang module end
> +
> +  // TDFInterface
> +  #pragma clang module begin M.TDFInterface
> +#pragma clang module import M.TDFNodes
> +  namespace Internal {
> +namespace TDF {
> +  using namespace Detail::TDF;
> +}
> +  }
> +  #pragma clang module end
> +
> +#pragma clang module endbuild
> +
> +#pragma clang module import M.TDFNodes
> +namespace Internal {
> +  namespace TDF {
> +TLoopManager * use;
> +  }
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/

r316965 - [modules] Retain multiple using-directives in the same scope even if they name the same namespace.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Oct 30 15:38:20 2017
New Revision: 316965

URL: http://llvm.org/viewvc/llvm-project?rev=316965&view=rev
Log:
[modules] Retain multiple using-directives in the same scope even if they name 
the same namespace.

They might have different visibility, and thus discarding all but one of them
can result in rejecting valid code. Also fix name lookup to cope with multiple
using-directives being found that denote the same namespace, where some are not
visible -- don't cache an "already visited" state for a using-directive that we
didn't visit because it was hidden.

Added:
cfe/trunk/test/Modules/using-directive-redecl.cpp
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=316965&r1=316964&r2=316965&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Oct 30 15:38:20 2017
@@ -1597,14 +1597,6 @@ bool NamedDecl::declarationReplaces(Name
 cast(OldD)->getQualifier());
   }
 
-  // UsingDirectiveDecl's are not really NamedDecl's, and all have same name.
-  // They can be replaced if they nominate the same namespace.
-  // FIXME: Is this true even if they have different module visibility?
-  if (auto *UD = dyn_cast(this))
-return UD->getNominatedNamespace()->getOriginalNamespace() ==
-   cast(OldD)->getNominatedNamespace()
-   ->getOriginalNamespace();
-
   if (isRedeclarable(getKind())) {
 if (getCanonicalDecl() != OldD->getCanonicalDecl())
   return false;

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=316965&r1=316964&r2=316965&view=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Oct 30 15:38:20 2017
@@ -155,7 +155,7 @@ namespace {
   while (true) {
 for (auto UD : DC->using_directives()) {
   DeclContext *NS = UD->getNominatedNamespace();
-  if (visited.insert(NS).second && SemaRef.isVisible(UD)) {
+  if (SemaRef.isVisible(UD) && visited.insert(NS).second) {
 addUsingDirective(UD, EffectiveDC);
 queue.push_back(NS);
   }
@@ -1883,7 +1883,7 @@ static bool LookupQualifiedNameInUsingDi
   // with its using-children.
   for (auto *I : StartDC->using_directives()) {
 NamespaceDecl *ND = I->getNominatedNamespace()->getOriginalNamespace();
-if (Visited.insert(ND).second && S.isVisible(I))
+if (S.isVisible(I) && Visited.insert(ND).second)
   Queue.push_back(ND);
   }
 
@@ -1931,7 +1931,7 @@ static bool LookupQualifiedNameInUsingDi
 
 for (auto I : ND->using_directives()) {
   NamespaceDecl *Nom = I->getNominatedNamespace();
-  if (Visited.insert(Nom).second && S.isVisible(I))
+  if (S.isVisible(I) && Visited.insert(Nom).second)
 Queue.push_back(Nom);
 }
   }

Added: cfe/trunk/test/Modules/using-directive-redecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/using-directive-redecl.cpp?rev=316965&view=auto
==
--- cfe/trunk/test/Modules/using-directive-redecl.cpp (added)
+++ cfe/trunk/test/Modules/using-directive-redecl.cpp Mon Oct 30 15:38:20 2017
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -verify %s
+// expected-no-diagnostics
+#pragma clang module build M
+module M { module TDFNodes {} module TDFInterface {} }
+#pragma clang module contents
+  // TDFNodes
+  #pragma clang module begin M.TDFNodes
+  namespace Detail {
+ namespace TDF {
+class TLoopManager {};
+ }
+  }
+  namespace Internal {
+ namespace TDF {
+using namespace Detail::TDF;
+ }
+  }
+  #pragma clang module end
+
+  // TDFInterface
+  #pragma clang module begin M.TDFInterface
+#pragma clang module import M.TDFNodes
+  namespace Internal {
+namespace TDF {
+  using namespace Detail::TDF;
+}
+  }
+  #pragma clang module end
+
+#pragma clang module endbuild
+
+#pragma clang module import M.TDFNodes
+namespace Internal {
+  namespace TDF {
+TLoopManager * use;
+  }
+}


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

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

[Sanjay Patel via Phabricator via cfe-commits]

spatel updated this revision to Diff 120902.
spatel added a comment.

Patch updated:
As suggested, I've morphed this patch to just handle sqrt libcalls based on the 
updated LLVM intrinsic definition.

I was going to include the builtins too, but that exposes another bug (marked 
here with FIXME) - the builtins are all defined 'const'. Therefore, they can 
never set errno (unless I'm still misunderstanding). So I think we are wrongly 
currently turning those into libcalls marked 'readnone'.

We could wrongly turn those into intrinsics in this patch if that seems better? 
:)

This does make me curious about the use-case of libm-equivalent builtins. If 
they are exactly identical to libm (including errno behavior), then why are 
they needed in the first place?


https://reviews.llvm.org/D39204

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/2005-07-20-SqrtNoErrno.c
  test/CodeGen/builtin-sqrt.c
  test/CodeGen/libcalls.c

Index: test/CodeGen/libcalls.c
===
--- test/CodeGen/libcalls.c
+++ test/CodeGen/libcalls.c
@@ -6,29 +6,28 @@
 // CHECK-NO-LABEL: define void @test_sqrt
 // CHECK-FAST-LABEL: define void @test_sqrt
 void test_sqrt(float a0, double a1, long double a2) {
-  // Following llvm-gcc's lead, we never emit these as intrinsics;
-  // no-math-errno isn't good enough.  We could probably use intrinsics
-  // with appropriate guards if it proves worthwhile.
-
   // CHECK-YES: call float @sqrtf
-  // CHECK-NO: call float @sqrtf
+  // CHECK-NO: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
-  // CHECK-NO: call double @sqrt
+  // CHECK-NO: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);
 
   // CHECK-YES: call x86_fp80 @sqrtl
-  // CHECK-NO: call x86_fp80 @sqrtl
+  // CHECK-NO: call x86_fp80 @llvm.sqrt.f80(x86_fp80
+  // CHECK-FAST: call x86_fp80 @llvm.sqrt.f80(x86_fp80
   long double l2 = sqrtl(a2);
 }
 
 // CHECK-YES: declare float @sqrtf(float)
 // CHECK-YES: declare double @sqrt(double)
 // CHECK-YES: declare x86_fp80 @sqrtl(x86_fp80)
-// CHECK-NO: declare float @sqrtf(float) [[NUW_RN:#[0-9]+]]
-// CHECK-NO: declare double @sqrt(double) [[NUW_RN]]
-// CHECK-NO: declare x86_fp80 @sqrtl(x86_fp80) [[NUW_RN]]
+// CHECK-NO: declare float @llvm.sqrt.f32(float)
+// CHECK-NO: declare double @llvm.sqrt.f64(double)
+// CHECK-NO: declare x86_fp80 @llvm.sqrt.f80(x86_fp80)
 // CHECK-FAST: declare float @llvm.sqrt.f32(float)
 // CHECK-FAST: declare double @llvm.sqrt.f64(double)
 // CHECK-FAST: declare x86_fp80 @llvm.sqrt.f80(x86_fp80)
@@ -86,7 +85,7 @@
   double atan_ = atan(d);
   long double atanl_ = atanl(ld);
   float atanf_ = atanf(f);
-// CHECK-NO: declare double @atan(double) [[NUW_RN]]
+// CHECK-NO: declare double @atan(double) [[NUW_RN:#[0-9]+]]
 // CHECK-NO: declare x86_fp80 @atanl(x86_fp80) [[NUW_RN]]
 // CHECK-NO: declare float @atanf(float) [[NUW_RN]]
 // CHECK-YES-NOT: declare double @atan(double) [[NUW_RN]]
@@ -126,5 +125,5 @@
 
 // CHECK-YES: attributes [[NUW_RN]] = { nounwind readnone speculatable }
 
-// CHECK-NO: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
-// CHECK-NO: attributes [[NUW_RNI]] = { nounwind readnone speculatable }
+// CHECK-NO-DAG: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
+// CHECK-NO-DAG: attributes [[NUW_RNI]] = { nounwind readnone speculatable }
Index: test/CodeGen/builtin-sqrt.c
===
--- test/CodeGen/builtin-sqrt.c
+++ test/CodeGen/builtin-sqrt.c
@@ -1,10 +1,19 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s
-// llvm.sqrt has undefined behavior on negative inputs, so it is
-// inappropriate to translate C/C++ sqrt to this.
-float sqrtf(float x);
+// RUN: %clang_cc1 -fmath-errno -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s --check-prefix=HAS_ERRNO
+// RUN: %clang_cc1  -triple x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s --check-prefix=NO_ERRNO
+
+// FIXME: If a builtin is supposed to have identical semantics to its libm twin, then it
+// should not be marked "constant" in Builtins.def because that means it can't set errno.
+// Note that both runs have 'readnone' on the libcall here.
+
 float foo(float X) {
-  // CHECK: foo
-  // CHECK: call float @sqrtf(float %
-  // Check that this is marked readonly when errno is ignored.
-  return sqrtf(X);
+  // HAS_ERRNO: call float @sqrtf(float
+  // NO_ERRNO: call float @sqrtf(float
+  return __builtin_sqrtf(X);
 }
+
+// HAS_ERRNO: declare float @sqrtf(float) [[ATTR:#[0-9]+]]
+// HAS_ERRNO: attributes [[ATTR]] = { nounwind readnone {{.*}}}
+
+// NO_ERRNO: declare float @sqrtf(float) [[ATTR:#[0-9]+]]
+// NO_ERRNO: attributes [[ATTR]] = { nounwind readnone {{.*}}}
+
Index: test/CodeGen/2005-07-20-SqrtNoErrno.c

[PATCH] D39430: [clangd] formatting: don't ignore style

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

For testing you could do something similar to how clang-format does it 
.


https://reviews.llvm.org/D39430



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

r316963 - [analyzer] Use the same filename for the header and the implementation of BugReporterVisitor

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Oct 30 15:31:57 2017
New Revision: 316963

URL: http://llvm.org/viewvc/llvm-project?rev=316963&view=rev
Log:
[analyzer] Use the same filename for the header and the implementation of 
BugReporterVisitor

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

Added:

cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  - copied, changed from r316948, 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
Removed:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=316963&r1=316962&r2=316963&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Mon 
Oct 30 15:31:57 2017
@@ -17,7 +17,7 @@
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
-#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"

Removed: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h?rev=316962&view=auto
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h 
(removed)
@@ -1,379 +0,0 @@
-//===---  BugReporterVisitor.h - Generate PathDiagnostics ---*- C++ 
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-//  This file declares BugReporterVisitors, which are used to generate enhanced
-//  diagnostic traces.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITOR_H
-#define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITOR_H
-
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
-#include "llvm/ADT/FoldingSet.h"
-
-namespace clang {
-class CFGBlock;
-
-namespace ento {
-
-class BugReport;
-class BugReporterContext;
-class ExplodedNode;
-class MemRegion;
-class PathDiagnosticPiece;
-
-/// \brief BugReporterVisitors are used to add custom diagnostics along a path.
-///
-/// Custom visitors should subclass the BugReporterVisitorImpl class for a
-/// default implementation of the clone() method.
-/// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the
-/// default implementation of clone() will NOT do the right thing, and you
-/// will have to provide your own implementation.)
-class BugReporterVisitor : public llvm::FoldingSetNode {
-public:
-  BugReporterVisitor() = default;
-  BugReporterVisitor(const BugReporterVisitor &) = default;
-  BugReporterVisitor(BugReporterVisitor &&) {}
-  virtual ~BugReporterVisitor();
-
-  /// \brief Returns a copy of this BugReporter.
-  ///
-  /// Custom BugReporterVisitors should not override this method directly.
-  /// Instead, they should inherit from BugReporterVisitorImpl and provide
-  /// a protected or public copy constructor.
-  ///
-  /// (Warning: if you have a deep subclass of BugReporterVisitorImpl, the
-  /// default implementation of clone() will NOT do the right thing, and you
-  /// will have to provide your own implementation.)
-  virtual std::unique_ptr clone() const = 0;
-
-  /// \brief Return a diagnostic piece which should be associated with the
-  /// given node.
-  ///
-  /// The last parameter can be used to register a new visitor with the given
-  /// BugReport while processing a node.
-  virtual std::shared_ptr
-  VisitNode(const ExplodedNode *Succ, const ExplodedNode *Pred,
-BugReporterContext &BRC, BugReport &BR) = 0;
-
-  /// \brief Provide custom definition for the final diagnostic piece on the
-  /// path - the piece, which is displayed before the path is expanded.
-  ///
-  /// If returns NULL the default implementation will be used.
-  /// Also note that at most one visitor of a BugReport shou

Re: r315402 - [modules] Only take visible using-directives into account during name lookup.

[Vassil Vassilev via cfe-commits]

On 11/10/17 03:19, Richard Smith via cfe-commits wrote:

Author: rsmith
Date: Tue Oct 10 18:19:11 2017
New Revision: 315402

URL: http://llvm.org/viewvc/llvm-project?rev=315402&view=rev
Log:
[modules] Only take visible using-directives into account during name lookup.

Added:
 cfe/trunk/test/Modules/using-directive.cpp
Modified:
 cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=315402&r1=315401&r2=315402&view=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue Oct 10 18:19:11 2017
@@ -88,13 +88,15 @@ namespace {
/// A collection of using directives, as used by C++ unqualified
/// lookup.
class UnqualUsingDirectiveSet {
+Sema &SemaRef;
+
  typedef SmallVector ListTy;
  
  ListTy list;

  llvm::SmallPtrSet visited;
  
public:

-UnqualUsingDirectiveSet() {}
+UnqualUsingDirectiveSet(Sema &SemaRef) : SemaRef(SemaRef) {}
  
  void visitScopeChain(Scope *S, Scope *InnermostFileScope) {

// C++ [namespace.udir]p1:
@@ -113,7 +115,8 @@ namespace {
visit(Ctx, Ctx);
  } else if (!Ctx || Ctx->isFunctionOrMethod()) {
for (auto *I : S->using_directives())
-visit(I, InnermostFileDC);
+if (SemaRef.isVisible(I))
+  visit(I, InnermostFileDC);
  }
}
  }
@@ -152,7 +155,7 @@ namespace {
while (true) {
  for (auto UD : DC->using_directives()) {
DeclContext *NS = UD->getNominatedNamespace();
-  if (visited.insert(NS).second) {
+  if (visited.insert(NS).second && SemaRef.isVisible(UD)) {

  It looks like this change breaks examples such as:

// RUN: rm -fr %t
// RUN %clang_cc1 -fmodules -fmodules-local-submodule-visibility 
-fmodules-cache-path=%t -verify %s

// expected-no-diagnostics
#pragma clang module build M
module M { module TDFNodes {} module TDFInterface {} }
#pragma clang module contents
  // TDFNodes
  #pragma clang module begin M.TDFNodes
  namespace Detail {
 namespace TDF {
    class TLoopManager {};
 }
  }
  namespace Internal {
 namespace TDF {
    using namespace Detail::TDF;
 }
  }
  #pragma clang module end

  // TDFInterface
  #pragma clang module begin M.TDFInterface
    #pragma clang module import M.TDFNodes
  namespace Internal {
    namespace TDF {
  using namespace Detail::TDF;
    }
  }
  #pragma clang module end

#pragma clang module endbuild

#pragma clang module import M.TDFNodes
namespace Internal {
  namespace TDF {
    TLoopManager * use;
  }
}

  I suspect that it triggers a preexisting bug in failing to make 
M.TDFNodes visible...

  addUsingDirective(UD, EffectiveDC);
  queue.push_back(NS);
}
@@ -1085,7 +1088,7 @@ bool Sema::CppLookupName(LookupResult &R
//   }
// }
//
-  UnqualUsingDirectiveSet UDirs;
+  UnqualUsingDirectiveSet UDirs(*this);
bool VisitedUsingDirectives = false;
bool LeftStartingScope = false;
DeclContext *OutsideOfTemplateParamDC = nullptr;
@@ -1868,22 +1871,19 @@ static bool LookupQualifiedNameInUsingDi
   DeclContext *StartDC) {
assert(StartDC->isFileContext() && "start context is not a file context");
  
-  DeclContext::udir_range UsingDirectives = StartDC->using_directives();

-  if (UsingDirectives.begin() == UsingDirectives.end()) return false;
+  // We have not yet looked into these namespaces, much less added
+  // their "using-children" to the queue.
+  SmallVector Queue;
  
// We have at least added all these contexts to the queue.

llvm::SmallPtrSet Visited;
Visited.insert(StartDC);
  
-  // We have not yet looked into these namespaces, much less added

-  // their "using-children" to the queue.
-  SmallVector Queue;
-
// We have already looked into the initial namespace; seed the queue
// with its using-children.
-  for (auto *I : UsingDirectives) {
+  for (auto *I : StartDC->using_directives()) {
  NamespaceDecl *ND = I->getNominatedNamespace()->getOriginalNamespace();
-if (Visited.insert(ND).second)
+if (Visited.insert(ND).second && S.isVisible(I))
Queue.push_back(ND);
}
  
@@ -1931,7 +1931,7 @@ static bool LookupQualifiedNameInUsingDi
  
  for (auto I : ND->using_directives()) {

NamespaceDecl *Nom = I->getNominatedNamespace();
-  if (Visited.insert(Nom).second)
+  if (Visited.insert(Nom).second && S.isVisible(I))
  Queue.push_back(Nom);
  }
}
@@ -3540,6 +3540,8 @@ static void LookupVisibleDecls(DeclConte
if (QualifiedNameLookup) {
  ShadowContextRAII Shadow(Visited);
  for (auto I : Ctx->using_directives()) {
+  if (!Result.getSema().isVisible(I))
+continue;
LookupVisibleDecls(I

[PATCH] D35894: [clangd] Code hover for Clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth added a comment.

I also have a quick patch that supports displaying comments preceding the 
declaration of a function. Once the review comments have been addressed for 
this revision I will submit it.


https://reviews.llvm.org/D35894



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

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 120890.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

Ping.

Rebased.
Silenced (via the `-w` flag in the `RUN` line) the `Fix conflicts with existing 
fix!` assertion since this differential does not actually cause it, but the 
testcase just happens to trigger it...
Reported it as PR35136  with 
minimal reproducer.

In https://reviews.llvm.org/D36836#889375, @aaron.ballman wrote:

> Adding @dberlin for licensing discussion questions.


@dberlin ping? I'm wondering if you had the chance to look at this? :)


Repository:
  rL LLVM

https://reviews.llvm.org/D36836

Files:
  LICENSE.TXT
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt
  clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang-tidy/readability/FunctionCognitiveComplexityCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
  test/clang-tidy/readability-function-cognitive-complexity.cpp

Index: test/clang-tidy/readability-function-cognitive-complexity.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-function-cognitive-complexity.cpp
@@ -0,0 +1,954 @@
+// RUN: %check_clang_tidy %s readability-function-cognitive-complexity %t -- -config='{CheckOptions: [{key: readability-function-cognitive-complexity.Threshold, value: 0}]}' -- -std=c++11 -w
+
+// any function should be checked.
+
+extern int ext_func(int x = 0);
+
+int some_func(int x = 0);
+
+static int some_other_func(int x = 0) {}
+
+template void some_templ_func(T x = 0) {}
+
+class SomeClass {
+public:
+  int *begin(int x = 0);
+  int *end(int x = 0);
+  static int func(int x = 0);
+  template void some_templ_func(T x = 0) {}
+  SomeClass() = default;
+  SomeClass(SomeClass&) = delete;
+};
+
+// nothing ever decreases cognitive complexity, so we can check all the things
+// in one go. none of the following should increase cognitive complexity:
+void unittest_false() {
+  {};
+  ext_func();
+  some_func();
+  some_other_func();
+  some_templ_func();
+  some_templ_func();
+  SomeClass::func();
+  SomeClass C;
+  C.some_templ_func();
+  C.some_templ_func();
+  C.func();
+  C.end();
+  int i = some_func();
+  i = i;
+  i++;
+  --i;
+  i < 0;
+  int j = 0 ?: 1;
+  auto k = new int;
+  delete k;
+  throw i;
+  {
+throw i;
+  }
+end:
+  return;
+}
+
+#if 1
+#define CC100
+#else
+// this macro has cognitive complexity of 100.
+// it is needed to be able to compare the testcases with the
+// reference Sonar implementation. please place it right after the first
+// CHECK-NOTES in each function
+#define CC100 if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){if(1){}if(1){}
+#endif
+
+////
+//-- B1. Increments --//
+////
+// Check that every thing listed in B1 of the specification does indeed   //
+// recieve the base increment, and that not-body does not increase nesting//
+////
+
+// break does not increase cognitive complexity.
+// only  break LABEL  does, but it is unavaliable in C or C++
+
+// continue does not increase cognitive complexity.
+// only  continue LABEL  does, but it is unavaliable in C or C++
+
+void unittest_b1_00() {
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: function 'unittest_b1_00' has cognitive complexity of 33 (threshold 0) [readability-function-cognitive-complexity]
+  CC100;
+
+  if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:3: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:9: note: +1, including nesting penalty of 0, nesting level increased to 1{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:11: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+} else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:12: note: +1, nesting level increased to 2{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:18: note: +3, including nesting penalty of 2, nesting level increased to 3{{$}}
+} else {
+// CHECK-NOTES: :[[@LINE-1]]:7: note: +1, nesting level increased to 2{{$}}
+}
+  } else if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:10: note: +1, nesting level increased to 1{{$}}
+// CHECK-NOTES: :[[@LINE-2]]:16: note: +2, including nesting penalty of 1, nesting level increased to 2{{$}}
+
+if (1 ? 1 : 0) {
+// CHECK-NOTES: :[[@LINE-1]]:5: note: +2, including nesting penalty of 1, nesting

[PATCH] D39269: [Analyzer] [Tests] Do not discard output from CmpRuns.py when running integration tests

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

@zaks.anna prints "XXX added / XXX removed" for every report which has appeared 
or disappeared.


https://reviews.llvm.org/D39269



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

[PATCH] D35894: [clangd] Code hover for Clangd

[William Enright via Phabricator via cfe-commits]

Nebiroth updated this revision to Diff 120894.
Nebiroth added a comment.

Code hover now displays declaration of symbol instead of source code by default.
Code hover now displays context information such as namespace and class name.
Array of MarkedString objects is now sent as response in JSON.


https://reviews.llvm.org/D35894

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  test/clangd/hover.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -5,16 +5,17 @@
 Content-Length: 143
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 535
+# CHECK: Content-Length: 568
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
 # CHECK:   "documentRangeFormattingProvider": true,
 # CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
 # CHECK:   "codeActionProvider": true,
 # CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
 # CHECK:   "signatureHelpProvider": {"triggerCharacters": ["(",","]},
-# CHECK:   "definitionProvider": true
+# CHECK:   "definitionProvider": true,
+# CHECK:   "hoverProvider": true
 # CHECK: }}}
 #
 Content-Length: 44
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -5,16 +5,17 @@
 Content-Length: 142
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":"","rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 535
+# CHECK: Content-Length: 568
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
 # CHECK:   "documentRangeFormattingProvider": true,
 # CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
 # CHECK:   "codeActionProvider": true,
 # CHECK:   "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
 # CHECK:   "signatureHelpProvider": {"triggerCharacters": ["(",","]},
-# CHECK:   "definitionProvider": true
+# CHECK:   "definitionProvider": true,
+# CHECK:   "hoverProvider": true
 # CHECK: }}}
 #
 Content-Length: 44
Index: test/clangd/hover.test
===
--- /dev/null
+++ test/clangd/hover.test
@@ -0,0 +1,52 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 407
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define MACRO 1\nnamespace ns1 {\nint test = 5;\nstruct MyClass {\nint xasd;\nvoid anotherOperation() {\n}\nstatic int foo(MyClass*) {\nreturn 0;\n}\n\n};}\nint main() {\nint a;\na;\nint b = ns1::test;\nns1::MyClass* Params;\nParams->anotherOperation();\nMACRO;}\n"}}}
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":0,"character":12}}}
+# Go to local variable
+# CHECK: {"jsonrpc":"2.0","id":1,"result":{"contents": ["#define statement",{"language": "C++", "value": "MACRO 1"}], "range": {"start": {"line": 0, "character": 8}, "end": {"line": 0, "character": 15
+
+Content-Length: 144
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":14,"character":1}}}
+# Go to local variable
+# CHECK: {"jsonrpc":"2.0","id":1,"result":{"contents": [{"language": "C++", "value": "int a"}], "range": {"start": {"line": 13, "character": 0}, "end": {"line": 13, "character": 5
+
+Content-Length: 145
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":15,"character":15}}}
+# Go to local variable
+# CHECK: {"jsonrpc":"2.0","id":1,"result":{"contents": ["In namespace named ns1",{"language": "C++", "value": "int test = 5"}], "range": {"start": {"line": 2, "character": 0}, "end": {"line": 2, "character": 12
+
+Content-Length: 145
+
+{"jsonrp

[PATCH] D39430: [clangd] formatting: don't ignore style

[Raoul Wols via Phabricator via cfe-commits]

rwols added a comment.

Does anyone have an idea how I would write a test for this?

Also, I should have commit access after approval now. So that's less work for 
you guys ;)


https://reviews.llvm.org/D39430



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

[PATCH] D39430: [clangd] formatting: don't ignore style

[Raoul Wols via Phabricator via cfe-commits]

rwols created this revision.

Look up in parent directories for a .clang-format file. Use "LLVM" as fallback 
style.


https://reviews.llvm.org/D39430

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h

Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -285,11 +285,15 @@
   llvm::Optional switchSourceHeader(PathRef Path);
 
   /// Run formatting for \p Rng inside \p File.
-  std::vector formatRange(PathRef File, Range Rng);
+  llvm::Expected> formatRange(PathRef File,
+Range Rng);
+
   /// Run formatting for the whole \p File.
-  std::vector formatFile(PathRef File);
+  llvm::Expected> formatFile(PathRef File);
+
   /// Run formatting after a character was typed at \p Pos in \p File.
-  std::vector formatOnType(PathRef File, Position Pos);
+  llvm::Expected> formatOnType(PathRef File,
+ Position Pos);
 
   /// Gets current document contents for \p File. \p File must point to a
   /// currently tracked file.
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -34,13 +34,14 @@
   std::promise &Promise;
 };
 
-std::vector formatCode(StringRef Code, StringRef Filename,
- ArrayRef Ranges) {
+llvm::Expected>
+formatCode(StringRef Code, StringRef Filename,
+   ArrayRef Ranges) {
   // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  auto Result = format::reformat(Style, Code, Ranges, Filename);
-
+  auto StyleOrError = format::getStyle("file", Filename, "LLVM");
+  if (!StyleOrError)
+return StyleOrError.takeError();
+  auto Result = format::reformat(StyleOrError.get(), Code, Ranges, Filename);
   return std::vector(Result.begin(), Result.end());
 }
 
@@ -301,23 +302,24 @@
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
-std::vector ClangdServer::formatRange(PathRef File,
-Range Rng) {
+llvm::Expected>
+ClangdServer::formatRange(PathRef File, Range Rng) {
   std::string Code = getDocument(File);
 
   size_t Begin = positionToOffset(Code, Rng.start);
   size_t Len = positionToOffset(Code, Rng.end) - Begin;
   return formatCode(Code, File, {tooling::Range(Begin, Len)});
 }
 
-std::vector ClangdServer::formatFile(PathRef File) {
+llvm::Expected>
+ClangdServer::formatFile(PathRef File) {
   // Format everything.
   std::string Code = getDocument(File);
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-std::vector ClangdServer::formatOnType(PathRef File,
- Position Pos) {
+llvm::Expected>
+ClangdServer::formatOnType(PathRef File, Position Pos) {
   // Look for the previous opening brace from the character position and
   // format starting from there.
   std::string Code = getDocument(File);
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -73,6 +73,12 @@
   std::vector
   getFixIts(StringRef File, const clangd::Diagnostic &D);
 
+  void replyWithTextEditsOrError(
+  Ctx C, std::string Code,
+  llvm::Expected>
+  ReplacementsOrError,
+  llvm::StringRef MessageInCaseError) const;
+
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// Language Server client.
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,6 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
+#include 
 
 using namespace clang::clangd;
 using namespace clang;
@@ -89,30 +90,40 @@
   Server.removeDocument(Params.textDocument.uri.file);
 }
 
+void ClangdLSPServer::replyWithTextEditsOrError(
+Ctx C, std::string Code,
+llvm::Expected>
+ReplacementsOrError,
+llvm::StringRef MessageInCaseError) const {
+  if (!ReplacementsOrError) {
+C.replyError(/*UnknownErrorCode*/ -32001, MessageInCaseError);
+  } else {
+C.reply("[" + replacementsToEdits(Code, ReplacementsOrError.get()) + "]");
+  }
+}
+
 void ClangdLSPServer::onDocumentOnTypeFormatting(
 Ctx C, DocumentOnTypeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  std::string Edits =
-  replacementsToEdits(Code, Server.formatOnType(File, Params.position));
-  C.reply("[" + Edits + "]");
+  const auto File = Params.textDocument.uri.file;
+  replyWithTextEditsOrError(std::move(C), Server.getDocument(File),
+   

[PATCH] D36101: Fix usage of right shift operator in fold expressions

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks!




Comment at: lib/Parse/ParseExpr.cpp:273
 static bool isFoldOperator(tok::TokenKind Kind) {
-  return isFoldOperator(getBinOpPrecedence(Kind, false, true));
+  return isFoldOperator(getBinOpPrecedence(Kind, true, true));
 }

Do we ever need to pass in the `Parser`'s `GreaterThanIsOperator` value here? 
(From a quick check, it looks like we never call `isFoldOperator` in any case 
where `>` might end a template argument list instead, but it's not completely 
obvious, because we do support parsing unparenthesized fold operators for 
better error handling in some cases.)

Seems like it would be safer to pass the correct value in here, even if it 
happens to not matter right now.


https://reviews.llvm.org/D36101



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D39149#911024, @rsmith wrote:

> In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
>
> > I'm thinking you could account for all possible type sizes in the existing 
> > (enabled by default) warning, and have a different warning for possibly 
> > tautological comparisons. E.g. if a `long` is being compared against 
> > `INT_MAX`, you know that's only tautological on some platforms, so it 
> > should go under `-Wpossible-tautological-constant-compare` (which would 
> > only be enabled by `-Weverything` and not `-Wall` or `-Wextra`); 
> > `-Wtautological-constant-compare` should be reserved for definitely 
> > tautological cases.
>
>
> This sounds like a very promising direction. (That is: if the types of the 
> values being compared are different, but are of the same size, then suppress 
> the warning or move it to a different warning group that's not part of 
> `-Wtautological-compare`.) That would also suppress warnings for cases like 
> 'ch > CHAR_MAX', when `ch` is a `char`, but we could detect and re-enable the 
> warning for such cases by, say, always warning if the constant side is an 
> integer literal.


Yeah, the type comparison implementation was what I was thinking of originally, 
though I wasn't sure about the edge cases.

When you say "always warning if the constant side is an integer literal", do 
you mean if it's a straight-up integer literal, or if it's an expression which 
evaluates to an integer literal at compile time? For example, would there be a 
difference if you were comparing to `numeric_limits::max()` vs. `INT_MAX`?


https://reviews.llvm.org/D39149



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D39149#911024, @rsmith wrote:

> In https://reviews.llvm.org/D39149#910936, @smeenai wrote:
>
> > I'm thinking you could account for all possible type sizes in the existing 
> > (enabled by default) warning, and have a different warning for possibly 
> > tautological comparisons. E.g. if a `long` is being compared against 
> > `INT_MAX`, you know that's only tautological on some platforms, so it 
> > should go under `-Wpossible-tautological-constant-compare` (which would 
> > only be enabled by `-Weverything` and not `-Wall` or `-Wextra`); 
> > `-Wtautological-constant-compare` should be reserved for definitely 
> > tautological cases.
>
>
> This sounds like a very promising direction. (




> That is: if the types of the values being compared are different, but are of 
> the same size, then suppress the warning or move it to a different warning 
> group that's not part of `-Wtautological-compare`.

Ok, now **that** makes at least //some// sense :)

> ) That would also suppress warnings for cases like 'ch > CHAR_MAX', when `ch` 
> is a `char`, but we could detect and re-enable the warning for such cases by, 
> say, always warning if the constant side is an integer literal.




https://reviews.llvm.org/D39149



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D39149#910936, @smeenai wrote:

> I'm thinking you could account for all possible type sizes in the existing 
> (enabled by default) warning, and have a different warning for possibly 
> tautological comparisons. E.g. if a `long` is being compared against 
> `INT_MAX`, you know that's only tautological on some platforms, so it should 
> go under `-Wpossible-tautological-constant-compare` (which would only be 
> enabled by `-Weverything` and not `-Wall` or `-Wextra`); 
> `-Wtautological-constant-compare` should be reserved for definitely 
> tautological cases.


This sounds like a very promising direction. (That is: if the types of the 
values being compared are different, but are of the same size, then suppress 
the warning or move it to a different warning group that's not part of 
`-Wtautological-compare`.) That would also suppress warnings for cases like 'ch 
> CHAR_MAX', when `ch` is a `char`, but we could detect and re-enable the 
warning for such cases by, say, always warning if the constant side is an 
integer literal.


https://reviews.llvm.org/D39149



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

[libcxx] r316951 - Add more fuzzing bits: partial_sort_copy, partition_copy, unique, unique_copy. No functional change to libc++; this is all test infastructure

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Oct 30 12:51:58 2017
New Revision: 316951

URL: http://llvm.org/viewvc/llvm-project?rev=316951&view=rev
Log:
Add more fuzzing bits: partial_sort_copy, partition_copy, unique, unique_copy. 
No functional change to libc++; this is all test infastructure

Added:
libcxx/trunk/test/libcxx/fuzzing/partial_sort_copy.cpp
libcxx/trunk/test/libcxx/fuzzing/partition_copy.cpp
libcxx/trunk/test/libcxx/fuzzing/unique.cpp
libcxx/trunk/test/libcxx/fuzzing/unique_copy.cpp
Modified:
libcxx/trunk/fuzzing/RoutineNames.txt
libcxx/trunk/fuzzing/fuzzing.cpp
libcxx/trunk/fuzzing/fuzzing.h

Modified: libcxx/trunk/fuzzing/RoutineNames.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/fuzzing/RoutineNames.txt?rev=316951&r1=316950&r2=316951&view=diff
==
--- libcxx/trunk/fuzzing/RoutineNames.txt (original)
+++ libcxx/trunk/fuzzing/RoutineNames.txt Mon Oct 30 12:51:58 2017
@@ -1,9 +1,13 @@
 sort
 stable_sort
 partition
+partition_copy
 stable_partition
+unique
+unique_copy
 nth_element
 partial_sort
+partial_sort_copy
 make_heap
 push_heap
 pop_heap

Modified: libcxx/trunk/fuzzing/fuzzing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/fuzzing/fuzzing.cpp?rev=316951&r1=316950&r2=316951&view=diff
==
--- libcxx/trunk/fuzzing/fuzzing.cpp (original)
+++ libcxx/trunk/fuzzing/fuzzing.cpp Mon Oct 30 12:51:58 2017
@@ -102,11 +102,13 @@ struct is_even
}
 };
 
-// == sort ==
+typedef std::vector Vec;
+typedef std::vector StableVec;
 
+// == sort ==
 int sort(const uint8_t *data, size_t size)
 {
-   std::vector working(data, data + size);
+   Vec working(data, data + size);
std::sort(working.begin(), working.end());
 
if (!std::is_sorted(working.begin(), working.end())) return 1;
@@ -116,13 +118,12 @@ int sort(const uint8_t *data, size_t siz
 
 
 // == stable_sort ==
-
 int stable_sort(const uint8_t *data, size_t size)
 {
-   std::vector input;
+   StableVec input;
for (size_t i = 0; i < size; ++i)
input.push_back(stable_test(data[i], i));
-   std::vector working = input;
+   StableVec working = input;
std::stable_sort(working.begin(), working.end(), key_less());
 
if (!std::is_sorted(working.begin(), working.end(), key_less()))   
return 1;
@@ -138,10 +139,9 @@ int stable_sort(const uint8_t *data, siz
 }
 
 // == partition ==
-
 int partition(const uint8_t *data, size_t size)
 {
-   std::vector working(data, data + size);
+   Vec working(data, data + size);
auto iter = std::partition(working.begin(), working.end(), 
is_even());
 
if (!std::all_of (working.begin(), iter, is_even())) return 1;
@@ -151,14 +151,38 @@ int partition(const uint8_t *data, size_
 }
 
 
-// == stable_partition ==
+// == partition_copy ==
+int partition_copy(const uint8_t *data, size_t size)
+{
+   Vec v1, v2;
+   auto iter = std::partition_copy(data, data + size,
+   std::back_inserter(v1), std::back_inserter(v2),
+   is_even());
+
+// The two vectors should add up to the original size
+   if (v1.size() + v2.size() != size) return 1;
+
+// All of the even values should be in the first vector, and none in the 
second
+   if (!std::all_of (v1.begin(), v1.end(), is_even())) return 2;
+   if (!std::none_of(v2.begin(), v2.end(), is_even())) return 3;
+
+// Every value in both vectors has to be in the original
+   for (auto v: v1)
+   if (std::find(data, data + size, v) == data + size) return 4;
+   
+   for (auto v: v2)
+   if (std::find(data, data + size, v) == data + size) return 5;
+   
+   return 0;
+}
 
+// == stable_partition ==
 int stable_partition (const uint8_t *data, size_t size)
 {
-   std::vector input;
+   StableVec input;
for (size_t i = 0; i < size; ++i)
input.push_back(stable_test(data[i], i));
-   std::vector working = input;
+   StableVec working = input;
auto iter = std::stable_partition(working.begin(), working.end(), 
is_even());
 
if (!std::all_of (working.begin(), iter, is_even())) 
return 1;
@@ -175,7 +199,7 @@ int nth_element (const uint8_t *data, si
 {
if (size <= 1) return 0;
const size_t partition_point = data[0] % size;  
-   std::vector working(data + 1, data + size);
+   Vec working(data + 1, data + size);
const auto partition_iter = working.begin() + partition_point;
std::nth_element(working.begin(), partition_iter, working.end());
 
@@ -203,7 +227,7 @@ int partial_sort (const uint8_t *data, s
 {
if (size <= 1) return 0;
const size_t sort_point = data[0] % size;
-   std::vector working(data + 1, data + size);
+   Vec working(data + 1, da

r316948 - [analyzer] [tests] Remove empty folders in reference results, do not store diffs.txt

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Oct 30 12:40:33 2017
New Revision: 316948

URL: http://llvm.org/viewvc/llvm-project?rev=316948&view=rev
Log:
[analyzer] [tests] Remove empty folders in reference results, do not store 
diffs.txt

Storing diffs.txt is now redundant, as we simply dump the CmpRuns output
to stdout (it is saved in CI and tends to be small).
Not generating those files enables us to remove empty folders, which
confuse git, as it would not add them with reference results.

Modified:
cfe/trunk/utils/analyzer/SATestBuild.py
cfe/trunk/utils/analyzer/SATestUpdateDiffs.py

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=316948&r1=316947&r2=316948&view=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Mon Oct 30 12:40:33 2017
@@ -120,8 +120,6 @@ BuildLogName = "run_static_analyzer.log"
 # displayed when buildbot detects a build failure.
 NumOfFailuresInSummary = 10
 FailuresSummaryFileName = "failures.txt"
-# Summary of the result diffs.
-DiffsSummaryFileName = "diffs.txt"
 
 # The scan-build result directory.
 SBOutputDirName = "ScanBuildResults"
@@ -434,6 +432,16 @@ def CleanUpEmptyPlists(SBOutputDir):
 continue
 
 
+def CleanUpEmptyFolders(SBOutputDir):
+"""
+Remove empty folders from results, as git would not store them.
+"""
+Subfolders = glob.glob(SBOutputDir + "/*")
+for Folder in Subfolders:
+if not os.listdir(Folder):
+os.removedirs(Folder)
+
+
 def checkBuild(SBOutputDir):
 """
 Given the scan-build output directory, checks if the build failed
@@ -446,6 +454,7 @@ def checkBuild(SBOutputDir):
 TotalFailed = len(Failures)
 if TotalFailed == 0:
 CleanUpEmptyPlists(SBOutputDir)
+CleanUpEmptyFolders(SBOutputDir)
 Plists = glob.glob(SBOutputDir + "/*/*.plist")
 print "Number of bug reports (non-empty plist files) produced: %d" %\
 len(Plists)
@@ -519,9 +528,8 @@ def runCmpResults(Dir, Strictness=0):
 if Verbose == 1:
 print "  Comparing Results: %s %s" % (RefDir, NewDir)
 
-DiffsPath = os.path.join(NewDir, DiffsSummaryFileName)
 PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName)
-Opts = CmpRuns.CmpOptions(DiffsPath, "", PatchedSourceDirPath)
+Opts = CmpRuns.CmpOptions(rootA="", rootB=PatchedSourceDirPath)
 # Scan the results, delete empty plist files.
 NumDiffs, ReportsInRef, ReportsInNew = \
 CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False)

Modified: cfe/trunk/utils/analyzer/SATestUpdateDiffs.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestUpdateDiffs.py?rev=316948&r1=316947&r2=316948&view=diff
==
--- cfe/trunk/utils/analyzer/SATestUpdateDiffs.py (original)
+++ cfe/trunk/utils/analyzer/SATestUpdateDiffs.py Mon Oct 30 12:40:33 2017
@@ -54,22 +54,9 @@ def updateReferenceResults(ProjName, Pro
 # Clean up the generated difference results.
 SATestBuild.cleanupReferenceResults(RefResultsPath)
 
-# Remove the created .diffs file before adding.
-removeDiffsSummaryFiles(RefResultsPath)
-
 runCmd('git add "%s"' % (RefResultsPath,))
 
 
-def removeDiffsSummaryFiles(RefResultsPath):
-"""
-Remove all auto-generated .diffs files in reference data.
-"""
-for (Dirpath, Dirnames, Filenames) in os.walk(RefResultsPath):
-if SATestBuild.DiffsSummaryFileName in Filenames:
-runCmd("rm '%s'" % os.path.join(
-Dirpath, SATestBuild.DiffsSummaryFileName))
-
-
 def main(argv):
 if len(argv) == 2 and argv[1] in ('-h', '--help'):
 print >> sys.stderr, "Update static analyzer reference results based "\


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

[PATCH] D39251: [libunwind] Fix building for ARM with dwarf exception handling

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 120863.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Avoiding duplicating the highest register number, avoiding any further casts in 
log lines.


https://reviews.llvm.org/D39251

Files:
  include/__libunwind_config.h
  include/libunwind.h
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -55,7 +55,7 @@
 # define REGISTER_KIND Registers_ppc
 #elif defined(__aarch64__)
 # define REGISTER_KIND Registers_arm64
-#elif defined(_LIBUNWIND_ARM_EHABI)
+#elif defined(__arm__)
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -583,6 +583,12 @@
   }
 #endif
 
+#if defined(_LIBUNWIND_TARGET_ARM)
+  compact_unwind_encoding_t dwarfEncoding(Registers_arm &) const {
+return 0;
+  }
+#endif
+
 #if defined (_LIBUNWIND_TARGET_OR1K)
   compact_unwind_encoding_t dwarfEncoding(Registers_or1k &) const {
 return 0;
Index: src/Registers.hpp
===
--- src/Registers.hpp
+++ src/Registers.hpp
@@ -1386,7 +1386,7 @@
   Registers_arm(const void *registers);
 
   boolvalidRegister(int num) const;
-  uint32_tgetRegister(int num);
+  uint32_tgetRegister(int num) const;
   voidsetRegister(int num, uint32_t value);
   boolvalidFloatRegister(int num) const;
   unw_fpreg_t getFloatRegister(int num);
@@ -1399,6 +1399,7 @@
 restoreSavedFloatRegisters();
 restoreCoreAndJumpTo();
   }
+  static int  lastDwarfRegNum() { return _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM; }
 
   uint32_t  getSP() const { return _registers.__sp; }
   void  setSP(uint32_t value) { _registers.__sp = value; }
@@ -1472,11 +1473,11 @@
   // Whether iWMMX data registers are saved.
   bool _saved_iwmmx;
   // Whether iWMMX control registers are saved.
-  bool _saved_iwmmx_control;
+  mutable bool _saved_iwmmx_control;
   // iWMMX registers
   unw_fpreg_t _iwmmx[16];
   // iWMMX control registers
-  uint32_t _iwmmx_control[4];
+  mutable uint32_t _iwmmx_control[4];
 #endif
 };
 
@@ -1533,7 +1534,7 @@
   return false;
 }
 
-inline uint32_t Registers_arm::getRegister(int regNum) {
+inline uint32_t Registers_arm::getRegister(int regNum) const {
   if (regNum == UNW_REG_SP || regNum == UNW_ARM_SP)
 return _registers.__sp;
 
Index: include/libunwind.h
===
--- include/libunwind.h
+++ include/libunwind.h
@@ -73,7 +73,7 @@
 
 typedef int unw_regnum_t;
 typedef uintptr_t unw_word_t;
-#if defined(_LIBUNWIND_ARM_EHABI)
+#if defined(__arm__)
 typedef uint64_t unw_fpreg_t;
 #else
 typedef double unw_fpreg_t;
Index: include/__libunwind_config.h
===
--- include/__libunwind_config.h
+++ include/__libunwind_config.h
@@ -19,7 +19,7 @@
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_X86_6432
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC   112
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM64 95
-#define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM   95
+#define _LIBUNWIND_HIGHEST_DWARF_REGISTER_ARM   287
 #define _LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K  31
 
 #if defined(_LIBUNWIND_IS_NATIVE_ONLY)
@@ -75,7 +75,7 @@
 # define _LIBUNWIND_TARGET_OR1K 1
 # define _LIBUNWIND_CONTEXT_SIZE 128
 # define _LIBUNWIND_CURSOR_SIZE 140
-# define _LIBUNWIND_HIGHEST_DWARF_REGISTER 119
+# define _LIBUNWIND_HIGHEST_DWARF_REGISTER 287
 #endif // _LIBUNWIND_IS_NATIVE_ONLY
 
 #endif // LIBUNWIND_CONFIG_H__
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov updated this revision to Diff 120861.

https://reviews.llvm.org/D39428

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/BodyFarm.h
  lib/Analysis/AnalysisDeclContext.cpp


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(MD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(MD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -304,10 +305,8 @@
   return AC.get();
 }
 
-BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
-  if (!FunctionBodyFarm)
-FunctionBodyFarm = llvm::make_unique(ASTCtx, Injector.get());
-  return FunctionBodyFarm.get();
+BodyFarm& AnalysisDeclContextManager::getBodyFarm() {
+  return FunctionBodyFarm;
 }
 
 const StackFrameContext *
Index: include/clang/Analysis/BodyFarm.h
===
--- include/clang/Analysis/BodyFarm.h
+++ include/clang/Analysis/BodyFarm.h
@@ -39,6 +39,9 @@
   /// Factory method for creating bodies for Objective-C properties.
   Stmt *getBody(const ObjCMethodDecl *D);
 
+  /// Remove copy constructor to avoid accidental copying.
+  BodyFarm(const BodyFarm& other) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -419,9 +419,9 @@
   /// declarations from external source.
   std::unique_ptr Injector;
 
-  /// Pointer to a factory for creating and caching implementations for common
+  /// A factory for creating and caching implementations for common
   /// methods during the analysis.
-  std::unique_ptr FunctionBodyFarm;
+  BodyFarm FunctionBodyFarm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
@@ -475,8 +475,8 @@
 return LocContexts.getStackFrame(getContext(D), Parent, S, Blk, Idx);
   }
 
-  /// Get and lazily create a {@code BodyFarm} instance.
-  BodyFarm *getBodyFarm();
+  /// Get a reference to {@code BodyFarm} instance.
+  BodyFarm& getBodyFarm();
 
   /// Discard all previously created AnalysisDeclContexts.
   void clear();


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm(

[PATCH] D39280: [libunwind] Use uint64_t for unw_word_t in ARM EHABI

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo abandoned this revision.
mstorsjo added a comment.

This was made obsolete by https://reviews.llvm.org/D39365.


https://reviews.llvm.org/D39280



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

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316942: Change unw_word_t to always have the same size as 
the pointer size (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D39365?vs=120562&id=120860#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39365

Files:
  libunwind/trunk/include/__libunwind_config.h
  libunwind/trunk/include/libunwind.h
  libunwind/trunk/src/Unwind-EHABI.cpp
  libunwind/trunk/src/UnwindLevel1-gcc-ext.c
  libunwind/trunk/src/UnwindLevel1.c
  libunwind/trunk/src/libunwind.cpp

Index: libunwind/trunk/src/UnwindLevel1.c
===
--- libunwind/trunk/src/UnwindLevel1.c
+++ libunwind/trunk/src/UnwindLevel1.c
@@ -76,8 +76,8 @@
   unw_word_t pc;
   unw_get_reg(cursor, UNW_REG_IP, &pc);
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase1(ex_ojb=%p): pc=0x%" PRIx64 ", start_ip=0x%" PRIx64
-  ", func=%s, lsda=0x%" PRIx64 ", personality=0x%" PRIx64 "",
+  "unwind_phase1(ex_ojb=%p): pc=0x%" PRIxPTR ", start_ip=0x%" PRIxPTR
+  ", func=%s, lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR "",
   (void *)exception_object, pc, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
@@ -170,9 +170,9 @@
  &offset) != UNW_ESUCCESS) ||
   (frameInfo.start_ip + offset > frameInfo.end_ip))
 functionName = ".anonymous.";
-  _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): start_ip=0x%" PRIx64
- ", func=%s, sp=0x%" PRIx64 ", lsda=0x%" PRIx64
- ", personality=0x%" PRIx64,
+  _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): start_ip=0x%" PRIxPTR
+ ", func=%s, sp=0x%" PRIxPTR ", lsda=0x%" PRIxPTR
+ ", personality=0x%" PRIxPTR,
  (void *)exception_object, frameInfo.start_ip,
  functionName, sp, frameInfo.lsda,
  frameInfo.handler);
@@ -213,8 +213,8 @@
   unw_get_reg(cursor, UNW_REG_IP, &pc);
   unw_get_reg(cursor, UNW_REG_SP, &sp);
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): re-entering "
- "user code with ip=0x%" PRIx64
- ", sp=0x%" PRIx64,
+ "user code with ip=0x%" PRIxPTR
+ ", sp=0x%" PRIxPTR,
  (void *)exception_object, pc, sp);
 }
 unw_resume(cursor);
@@ -262,8 +262,8 @@
   (frameInfo.start_ip + offset > frameInfo.end_ip))
 functionName = ".anonymous.";
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase2_forced(ex_ojb=%p): start_ip=0x%" PRIx64
-  ", func=%s, lsda=0x%" PRIx64 ", personality=0x%" PRIx64,
+  "unwind_phase2_forced(ex_ojb=%p): start_ip=0x%" PRIxPTR
+  ", func=%s, lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR,
   (void *)exception_object, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
@@ -467,17 +467,17 @@
   unw_cursor_t *cursor = (unw_cursor_t *)context;
   unw_word_t result;
   unw_get_reg(cursor, index, &result);
-  _LIBUNWIND_TRACE_API("_Unwind_GetGR(context=%p, reg=%d) => 0x%" PRIx64,
-   (void *)context, index, (uint64_t)result);
+  _LIBUNWIND_TRACE_API("_Unwind_GetGR(context=%p, reg=%d) => 0x%" PRIxPTR,
+   (void *)context, index, result);
   return (uintptr_t)result;
 }
 
 /// Called by personality handler during phase 2 to alter register values.
 _LIBUNWIND_EXPORT void _Unwind_SetGR(struct _Unwind_Context *context, int index,
  uintptr_t value) {
-  _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d, value=0x%0" PRIx64
+  _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d, value=0x%0" PRIxPTR
")",
-   (void *)context, index, (uint64_t)value);
+   (void *)context, index, value);
   unw_cursor_t *cursor = (unw_cursor_t *)context;
   unw_set_reg(cursor, index, value);
 }
@@ -487,18 +487,18 @@
   unw_cursor_t *cursor = (unw_cursor_t *)context;
   unw_word_t result;
   unw_get_reg(cursor, UNW_REG_IP, &result);
-  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64,
-   (void *)context, (uint64_t)result);
+  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIxPTR,
+   (void *)context, result);
   return (uintptr_t)result;
 }
 
 /// Called by personality handler during phase 2 to alter instruction pointer,
 /// such as setting where the landing pad is, so _Unwind_Resume() will
 /// start executing in the landing pad.
 _LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Cont

[libunwind] r316942 - Change unw_word_t to always have the same size as the pointer size

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Mon Oct 30 12:06:34 2017
New Revision: 316942

URL: http://llvm.org/viewvc/llvm-project?rev=316942&view=rev
Log:
Change unw_word_t to always have the same size as the pointer size

This matches the original libunwind API. This also unifies the
type between ARM EHABI and the other configurations, and allows
getting rid of a number of casts in log messages.

The cursor size updates for ppc and or1k are untested, but
unw_proc_info_t shrinks by 4 uint64_t units on i386 at least.

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

Modified:
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/include/libunwind.h
libunwind/trunk/src/Unwind-EHABI.cpp
libunwind/trunk/src/UnwindLevel1-gcc-ext.c
libunwind/trunk/src/UnwindLevel1.c
libunwind/trunk/src/libunwind.cpp

Modified: libunwind/trunk/include/__libunwind_config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/__libunwind_config.h?rev=316942&r1=316941&r2=316942&view=diff
==
--- libunwind/trunk/include/__libunwind_config.h (original)
+++ libunwind/trunk/include/__libunwind_config.h Mon Oct 30 12:06:34 2017
@@ -26,7 +26,7 @@
 # if defined(__i386__)
 #  define _LIBUNWIND_TARGET_I386
 #  define _LIBUNWIND_CONTEXT_SIZE 8
-#  define _LIBUNWIND_CURSOR_SIZE 19
+#  define _LIBUNWIND_CURSOR_SIZE 15
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_X86
 # elif defined(__x86_64__)
 #  define _LIBUNWIND_TARGET_X86_64 1
@@ -41,7 +41,7 @@
 # elif defined(__ppc__)
 #  define _LIBUNWIND_TARGET_PPC 1
 #  define _LIBUNWIND_CONTEXT_SIZE 117
-#  define _LIBUNWIND_CURSOR_SIZE 128
+#  define _LIBUNWIND_CURSOR_SIZE 124
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC
 # elif defined(__aarch64__)
 #  define _LIBUNWIND_TARGET_AARCH64 1
@@ -61,7 +61,7 @@
 # elif defined(__or1k__)
 #  define _LIBUNWIND_TARGET_OR1K 1
 #  define _LIBUNWIND_CONTEXT_SIZE 16
-#  define _LIBUNWIND_CURSOR_SIZE 28
+#  define _LIBUNWIND_CURSOR_SIZE 24
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 
_LIBUNWIND_HIGHEST_DWARF_REGISTER_OR1K
 # else
 #  error "Unsupported architecture."

Modified: libunwind/trunk/include/libunwind.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/libunwind.h?rev=316942&r1=316941&r2=316942&view=diff
==
--- libunwind/trunk/include/libunwind.h (original)
+++ libunwind/trunk/include/libunwind.h Mon Oct 30 12:06:34 2017
@@ -72,11 +72,10 @@ typedef struct unw_cursor_t unw_cursor_t
 typedef struct unw_addr_space *unw_addr_space_t;
 
 typedef int unw_regnum_t;
+typedef uintptr_t unw_word_t;
 #if defined(_LIBUNWIND_ARM_EHABI)
-typedef uint32_t unw_word_t;
 typedef uint64_t unw_fpreg_t;
 #else
-typedef uint64_t unw_word_t;
 typedef double unw_fpreg_t;
 #endif
 

Modified: libunwind/trunk/src/Unwind-EHABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-EHABI.cpp?rev=316942&r1=316941&r2=316942&view=diff
==
--- libunwind/trunk/src/Unwind-EHABI.cpp (original)
+++ libunwind/trunk/src/Unwind-EHABI.cpp Mon Oct 30 12:06:34 2017
@@ -14,6 +14,7 @@
 
 #if defined(_LIBUNWIND_ARM_EHABI)
 
+#include 
 #include 
 #include 
 #include 
@@ -468,11 +469,11 @@ unwind_phase1(unw_context_t *uc, unw_cur
   unw_word_t pc;
   unw_get_reg(cursor, UNW_REG_IP, &pc);
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase1(ex_ojb=%p): pc=0x%llX, start_ip=0x%llX, func=%s, "
-  "lsda=0x%llX, personality=0x%llX",
-  static_cast(exception_object), (long long)pc,
-  (long long)frameInfo.start_ip, functionName,
-  (long long)frameInfo.lsda, (long long)frameInfo.handler);
+  "unwind_phase1(ex_ojb=%p): pc=0x%" PRIxPTR ", start_ip=0x%" PRIxPTR 
", func=%s, "
+  "lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR,
+  static_cast(exception_object), pc,
+  frameInfo.start_ip, functionName,
+  frameInfo.lsda, frameInfo.handler);
 }
 
 // If there is a personality routine, ask it if it will want to stop at
@@ -584,11 +585,11 @@ static _Unwind_Reason_Code unwind_phase2
   (frameInfo.start_ip + offset > frameInfo.end_ip))
 functionName = ".anonymous.";
   _LIBUNWIND_TRACE_UNWINDING(
-  "unwind_phase2(ex_ojb=%p): start_ip=0x%llX, func=%s, sp=0x%llX, "
-  "lsda=0x%llX, personality=0x%llX",
-  static_cast(exception_object), (long long)frameInfo.start_ip,
-  functionName, (long long)sp, (long long)frameInfo.lsda,
-  (long long)frameInfo.handler);
+  "unwind_phase2(ex_ojb=%p): start_ip=0x%" PRIxPTR ", func=%s, sp=0x%" 
PRIxPTR ", "
+  "lsda=0x%" PRIxPTR ", personality=0x%" PRIxPTR "",
+  static_cast(exception_object), frameInfo.start_ip,
+  functionName, sp, frameI

[libcxx] r316941 - Implement LWG 3013 - some filesystem members should not be noexcept.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon Oct 30 11:59:59 2017
New Revision: 316941

URL: http://llvm.org/viewvc/llvm-project?rev=316941&view=rev
Log:
Implement LWG 3013 - some filesystem members should not be noexcept.

LWG 3013 points out that the constructors and increment members
of the directory iterators need to allocate, and therefore cannot
be marked noexcept.

It also points out that `is_empty` and `copy` likely need to allocate
as well, and as such can also not be noexcept.

This patch speculatively implements the resolution removing noexcept,
because libc++ does indeed have the possibility of throwing on allocation
failure.

Modified:
libcxx/trunk/include/experimental/filesystem

libcxx/trunk/test/std/experimental/filesystem/class.directory_iterator/directory_iterator.members/ctor.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/class.directory_iterator/directory_iterator.members/increment.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/ctor.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.copy/copy.pass.cpp

libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.is_empty/is_empty.pass.cpp

Modified: libcxx/trunk/include/experimental/filesystem
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/filesystem?rev=316941&r1=316940&r2=316941&view=diff
==
--- libcxx/trunk/include/experimental/filesystem (original)
+++ libcxx/trunk/include/experimental/filesystem Mon Oct 30 11:59:59 2017
@@ -81,10 +81,10 @@
 path canonical(const path& p, const path& base, error_code& ec);
 
 void copy(const path& from, const path& to);
-void copy(const path& from, const path& to, error_code& ec) _NOEXCEPT;
+void copy(const path& from, const path& to, error_code& ec);
 void copy(const path& from, const path& to, copy_options options);
 void copy(const path& from, const path& to, copy_options options,
-   error_code& ec) _NOEXCEPT;
+   error_code& ec);
 
 bool copy_file(const path& from, const path& to);
 bool copy_file(const path& from, const path& to, error_code& ec) _NOEXCEPT;
@@ -1351,7 +1351,7 @@ void copy(const path& __from, const path
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-void copy(const path& __from, const path& __to, error_code& __ec) _NOEXCEPT {
+void copy(const path& __from, const path& __to, error_code& __ec) {
 __copy(__from, __to, copy_options::none, &__ec);
 }
 
@@ -1362,7 +1362,7 @@ void copy(const path& __from, const path
 
 inline _LIBCPP_INLINE_VISIBILITY
 void copy(const path& __from, const path& __to,
-  copy_options __opt, error_code& __ec) _NOEXCEPT {
+  copy_options __opt, error_code& __ec) {
 __copy(__from, __to, __opt, &__ec);
 }
 
@@ -1561,7 +1561,7 @@ bool is_empty(const path& __p) {
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-bool is_empty(const path& __p, error_code& __ec) _NOEXCEPT {
+bool is_empty(const path& __p, error_code& __ec) {
 return __fs_is_empty(__p, &__ec);
 }
 
@@ -1903,12 +1903,12 @@ public:
 : directory_iterator(__p, nullptr, __opts)
 { }
 
-directory_iterator(const path& __p, error_code& __ec) _NOEXCEPT
+directory_iterator(const path& __p, error_code& __ec)
 : directory_iterator(__p, &__ec)
 { }
 
 directory_iterator(const path& __p, directory_options __opts,
-   error_code& __ec) _NOEXCEPT
+   error_code& __ec)
 : directory_iterator(__p, &__ec, __opts)
 { }
 
@@ -1943,7 +1943,7 @@ public:
 return __p;
 }
 
-directory_iterator& increment(error_code& __ec) _NOEXCEPT
+directory_iterator& increment(error_code& __ec)
 { return __increment(&__ec); }
 
 private:
@@ -2013,12 +2013,12 @@ public:
 
 _LIBCPP_INLINE_VISIBILITY
 recursive_directory_iterator(const path& __p,
-directory_options __xoptions, error_code& __ec) _NOEXCEPT
+directory_options __xoptions, error_code& __ec)
 : recursive_directory_iterator(__p, __xoptions, &__ec)
 { }
 
 _LIBCPP_INLINE_VISIBILITY
-recursive_directory_iterator(const path& __p, error_code& __ec) _NOEXCEPT
+recursive_directory_iterator(const path& __p, error_code& __ec)
 : recursive_directory_iterator(__p, directory_options::none,  &__ec)
 { }
 
@@ -2060,7 +2060,7 @@ public:
 }
 
 _LIBCPP_INLINE_VISIBILITY
-recursive_directory_iterator& increment(error_code& __ec) _NOEXCEPT
+recursive_directory_iterator& increment(error_code& __ec)
 { return __increment(&__ec); }
 
 _LIBCPP_FUNC_VIS directory_options options() const;

Modified: 
libcxx/trunk/test/std/experimental/filesystem/class.directory_iterator/directory_iterator.members/ctor.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-pr

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote:

> In https://reviews.llvm.org/D39149#910891, @bcain wrote:
>
> > In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
> >
> > > That is my diagnostic, so i guess this is the time to reply :)
> >
> >
> > ...
> >
> > > 3. In `-Wtautological-constant-compare`, ignore any comparisons that 
> > > compare with `std::numeric_limits`. Not a fan of this solution. <- Worst?
> >
> > ...
> >
> > Gee, #3 is the worst?  That one seems the most appealing to me.  I think I 
> > could understand it if you had reservations about ignoring comparisons 
> > against {CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is 
> > well-qualified and everything.
> >
> > Can you help me understand why you don't prefer it?
>
>
> That my initial bug was not caught by gcc either, even though `-Wtype-limits` 
> was enabled.


...

Ok, yes, that makes sense.

>> Also, is there any way that the warning could somehow be smart enough to 
>> know what sizeof(int) and sizeof(long) are?
> 
> I'm not sure i follow. It sure does know that, else it would not possible to 
> diagnose anything.

Yes, sorry, I had the logic reversed in my head.

...

>> As an aside, how widely has this new clang behavior been tested?  Is it 
>> possible that this new warning behavior might get rolled back if a 
>> big/important enough codebase triggers it?
> 
> Are you talking about false-positives* or about real true-positive warnings?
>  So far, to the best of my knowledge, there were no reports of 
> false-positives*.
>  If a false-positive is reported, and it is not obvious how to fix it, i 
> suppose it might be reverted.
>  True-positives are obviously not the justification for the revert, but a 
> validation of the validity of the diagnostic.
>  The cases like this one are troublesome. **If** it is possible to reduce a 
> case that obviously should not warn, **then** the diagnostic should be 
> adjusted not to warn.
> 
> - The fact that under *different* circumstances the comparison is not 
> tautological is not a false-positive.

Isn't this a case that it obviously should not warn?

What about another approach: (1) could we consider Shoaib's suggestion to keep 
the exhaustive check with this name and create a new one that's limited (and 
can take the current one's place in the enabled-by-Wall) or (2) could we create 
range check functions that have the warning disabled?

Option #2 is limited to benefiting only llvm or libcxx code.  But if we narrow 
our focus to just the llvm/libcxx code for the time being, we should have a 
common idiom for resolving this problem.  It's likely to show up elsewhere too. 
 The fact that we were considering ifdefs here versus the pragmas in the 
earlier commit is concerning.


https://reviews.llvm.org/D39149



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

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

[David Blaikie via Phabricator via cfe-commits]

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

Looks good - thanks!


https://reviews.llvm.org/D39428



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

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, 
aemerson.

I've also removed copy constructor to help clients not to shoot themselves in 
the foot with `BodyFarm stored = Manager.getBodyFarm()`.


https://reviews.llvm.org/D39428

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/BodyFarm.h
  lib/Analysis/AnalysisDeclContext.cpp


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(MD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(MD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -304,10 +305,8 @@
   return AC.get();
 }
 
-BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
-  if (!FunctionBodyFarm)
-FunctionBodyFarm = llvm::make_unique(ASTCtx, Injector.get());
-  return FunctionBodyFarm.get();
+BodyFarm& AnalysisDeclContextManager::getBodyFarm() {
+  return FunctionBodyFarm;
 }
 
 const StackFrameContext *
Index: include/clang/Analysis/BodyFarm.h
===
--- include/clang/Analysis/BodyFarm.h
+++ include/clang/Analysis/BodyFarm.h
@@ -39,6 +39,9 @@
   /// Factory method for creating bodies for Objective-C properties.
   Stmt *getBody(const ObjCMethodDecl *D);
 
+  /// Remove copy constructor to avoid accidental copying.
+  BodyFarm(const BodyFarm& other) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -419,9 +419,9 @@
   /// declarations from external source.
   std::unique_ptr Injector;
 
-  /// Pointer to a factory for creating and caching implementations for common
+  /// A factory for creating and caching implementations for common
   /// methods during the analysis.
-  std::unique_ptr FunctionBodyFarm;
+  BodyFarm FunctionBodyFarm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
@@ -476,7 +476,7 @@
   }
 
   /// Get and lazily create a {@code BodyFarm} instance.
-  BodyFarm *getBodyFarm();
+  BodyFarm& getBodyFarm();
 
   /// Discard all previously created AnalysisDeclContexts.
   void clear();


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -68,7 +68,8 @@
 bool addInitializers, bool addTemporaryDtors, bool addLifetime,
 bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch,
 bool addCXXNewAllocator, CodeInjector *injector)
-: ASTCtx(ASTCtx), Injector(injector), SynthesizeBodies(synthesizeBodies) {
+: ASTCtx(ASTCtx), Injector(injector), FunctionBodyFarm(ASTCtx, injector),
+  SynthesizeBodies(synthesizeBodies)  {
   cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
   cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
   cfgBuildOptions.AddInitializers = addInitializers;
@@ -88,7 +89,7 @@
 if (auto *CoroBody = dyn_cast_or_null(Body))
   Body = CoroBody->getBody();
 if (Manager && Manager->synthesizeBodies()) {
-  Stmt *SynthesizedBody = Manager->getBodyFarm()->getBody(FD);
+  Stmt *SynthesizedBody = Manager->getBodyFarm().getBody(FD);
   if (SynthesizedBody) {
 Body = SynthesizedBody;
 IsAutosynthesized = true;
@@ -99,7 +100,7 @@
   else if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 Stmt *Body = MD->getBody();
 if (Manager && Mana

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D39149#910931, @lebedev.ri wrote:

> - The fact that under *different* circumstances the comparison is not 
> tautological is not a false-positive.


It's not technically a false positive, but my suspicion is that it'll make the 
warning a lot less useful in a lot of cases.


https://reviews.llvm.org/D39149



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

[libcxx] r316939 - Fix PR35078 - recursive directory iterator's increment method throws incorrectly.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon Oct 30 11:43:21 2017
New Revision: 316939

URL: http://llvm.org/viewvc/llvm-project?rev=316939&view=rev
Log:
Fix PR35078 - recursive directory iterator's increment method throws 
incorrectly.

The guts of the increment method for recursive_directory_iterator
was failing to pass an error code object to calls to status/symlink_status,
which can throw under certain conditions.

This patch fixes the issues by correctly propagating the error codes.
However the noexcept still needs to be removed from the signature, as
mentioned in LWG 3014, but that change will be made in a separate commit.

Modified:
libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp

libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp

Modified: libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp?rev=316939&r1=316938&r2=316939&view=diff
==
--- libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp Mon Oct 30 
11:43:21 2017
@@ -296,24 +296,43 @@ void recursive_directory_iterator::__adv
 }
 
 bool recursive_directory_iterator::__try_recursion(error_code *ec) {
-
 bool rec_sym =
 bool(options() & directory_options::follow_directory_symlink);
+
 auto& curr_it = __imp_->__stack_.top();
 
-if (is_directory(curr_it.__entry_.status()) &&
-(!is_symlink(curr_it.__entry_.symlink_status()) || rec_sym))
-{
-std::error_code m_ec;
+bool skip_rec = false;
+std::error_code m_ec;
+if (!rec_sym) {
+  file_status st = curr_it.__entry_.symlink_status(m_ec);
+  if (m_ec && status_known(st))
+m_ec.clear();
+  if (m_ec || is_symlink(st) || !is_directory(st))
+skip_rec = true;
+} else {
+  file_status st = curr_it.__entry_.status(m_ec);
+  if (m_ec && status_known(st))
+m_ec.clear();
+  if (m_ec || !is_directory(st))
+skip_rec = true;
+}
+
+if (!skip_rec) {
 __dir_stream new_it(curr_it.__entry_.path(), __imp_->__options_, m_ec);
 if (new_it.good()) {
 __imp_->__stack_.push(_VSTD::move(new_it));
 return true;
 }
-if (m_ec) {
-__imp_.reset();
-set_or_throw(m_ec, ec,
-   "recursive_directory_iterator::operator++()");
+}
+if (m_ec) {
+const bool allow_eacess = bool(__imp_->__options_
+& directory_options::skip_permission_denied);
+if (m_ec.value() == EACCES && allow_eacess) {
+  if (ec) ec->clear();
+} else {
+  __imp_.reset();
+  set_or_throw(m_ec, ec,
+   "recursive_directory_iterator::operator++()");
 }
 }
 return false;

Modified: 
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp?rev=316939&r1=316938&r2=316939&view=diff
==
--- 
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
 Mon Oct 30 11:43:21 2017
@@ -237,4 +237,250 @@ TEST_CASE(access_denied_on_recursion_tes
 }
 }
 
+// See llvm.org/PR35078
+TEST_CASE(test_PR35078)
+{
+  using namespace std::experimental::filesystem;
+scoped_test_env env;
+const path testFiles[] = {
+env.create_dir("dir1"),
+env.create_dir("dir1/dir2"),
+env.create_dir("dir1/dir2/dir3"),
+env.create_file("dir1/file1"),
+env.create_file("dir1/dir2/dir3/file2")
+};
+const path startDir = testFiles[0];
+const path permDeniedDir = testFiles[1];
+const path nestedDir = testFiles[2];
+const path nestedFile = testFiles[3];
+
+// Change the permissions so we can no longer iterate
+permissions(permDeniedDir,
+perms::remove_perms|perms::group_exec
+   |perms::owner_exec|perms::others_exec);
+
+const std::error_code eacess_ec =
+std::make_error_code(std::errc::permission_denied);
+std::error_code ec = GetTestEC();
+
+const recursive_directory_iterator endIt;
+
+auto SetupState = [&](bool AllowEAccess, bool& SeenFile3) {
+  SeenFile3 = false;
+  auto Opts = AllowEAccess ? directory_options::skip_permission_denied
+  : directory_options::none;
+  recursive_directory_iterator it(startDir, Opts, ec);
+  while (!ec && it != endIt && *it != nestedDir) {
+if (*it ==

[PATCH] D39332: [clang-refactor] Introduce "local-qualified-rename" action.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:101
+  std::string NewQualifiedName) {
+  return QualifiedRenameRule(std::move(OldQualifiedName),
+ std::move(NewQualifiedName));

It might be better to find the declaration (and report error if needed) during 
in initiation, and then pass the `ND` to the class. Maybe then both 
`RenameOccurrences` and `QualifiedRenameRule` could subclass from one base 
class that actually does just this:

``` 
auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
  assert(!USRs.empty());
  return tooling::createRenameAtomicChanges(
  USRs, NewQualifiedName, Context.getASTContext().getTranslationUnitDecl());
```


https://reviews.llvm.org/D39332



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I'm thinking you could account for all possible type sizes in the existing 
(enabled by default) warning, and have a different warning for possibly 
tautological comparisons. E.g. if a `long` is being compared against `INT_MAX`, 
you know that's only tautological on some platforms, so it should go under 
`-Wpossible-tautological-constant-compare` (which would only be enabled by 
`-Weverything` and not `-Wall` or `-Wextra`); `-Wtautological-constant-compare` 
should be reserved for definitely tautological cases.


https://reviews.llvm.org/D39149



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

[PATCH] D39284: [c++2a] Decomposed _condition_

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: include/clang/Sema/DeclSpec.h:2015
 case ConditionContext:
+  return Lang.CPlusPlus2a;
 case MemberContext:

If there's no grammar ambiguities here (and I don't think there are), please 
accept this unconditionally, with an `ExtWarn`.



Comment at: lib/Parse/ParseExprCXX.cpp:1711
 /// [C++11] type-specifier-seq declarator braced-init-list
+/// [C++2a] type-specifier-seq ref-qualifier[opt] '[' identifier-list ']'
+/// brace-or-equal-initializer

This shouldn't be listed as `C++2a` until it's actually voted into the working 
draft. Listing it as `[Clang]` for a clang extension is fine.



Comment at: lib/Sema/SemaDeclCXX.cpp:696-698
+  // a for-range-declaration, or a condition in C++2a, but we parse it in more
+  // cases than that.
+  if (!D.mayHaveDecompositionDeclarator(getLangOpts())) {

Again, please don't guess what's going to be in C++2a. I would actually expect 
that we'll vote this in as a DR against C++17. It seems like a wording 
oversight to me.


https://reviews.llvm.org/D39284



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D39149#910891, @bcain wrote:

> In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:
>
> > That is my diagnostic, so i guess this is the time to reply :)
>
>
> ...
>
> > 3. In `-Wtautological-constant-compare`, ignore any comparisons that 
> > compare with `std::numeric_limits`. Not a fan of this solution. <- Worst?
>
> ...
>
> Gee, #3 is the worst?  That one seems the most appealing to me.  I think I 
> could understand it if you had reservations about ignoring comparisons 
> against {CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is 
> well-qualified and everything.
>
> Can you help me understand why you don't prefer it?


That my initial bug was not caught by gcc either, even though `-Wtype-limits` 
was enabled.
Because for whatever reason gcc basically does not issue diagnostics for 
templated functions.
Even though the tautologically-compared variable was *not* dependent on the 
template parameters in any way.
See for yourself, i think this is *really* bad: https://godbolt.org/g/zkq3UD

So all-size-fits-all things like that are just asking to be done wrong.
I fail to see how `ignore any comparisons that compare with 
std::numeric_limits` would not be the same pitfall, unfortunately.

> Also, is there any way that the warning could somehow be smart enough to know 
> what sizeof(int) and sizeof(long) are?

I'm not sure i follow. It sure does know that, else it would not possible to 
diagnose anything.
Or were you talking about

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> That is my diagnostic, so i guess this is the time to reply :)
>  As i see it, there are several options:
>  ...
>
> 5. The essential problem, i *think* is much like the problem with unreachable 
> code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” 
> . The check does not know/care whether 
> the comparison is tautological only with the current Fundamental type sizes, 
> or always. Perhaps this is the proper heading?


?

> As an aside, how widely has this new clang behavior been tested?  Is it 
> possible that this new warning behavior might get rolled back if a 
> big/important enough codebase triggers it?

Are you talking about false-positives* or about real true-positive warnings?
So far, to the best of my knowledge, there were no reports of false-positives*.
If a false-positive is reported, and it is not obvious how to fix it, i suppose 
it might be reverted.
True-positives are obviously not the justification for the revert, but a 
validation of the validity of the diagnostic.
The cases like this one are troublesome. **If** it is possible to reduce a case 
that obviously should not warn, **then** the diagnostic should be adjusted not 
to warn.

- The fact that under *different* circumstances the comparison is not 
tautological is not a false-positive.


https://reviews.llvm.org/D39149



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

Re: [PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp

[George Karpenkov via cfe-commits]

> On Oct 30, 2017, at 8:40 AM, David Blaikie  wrote:
> 
> 
> 
> On Mon, Oct 23, 2017 at 5:01 PM George Karpenkov via Phabricator via 
> cfe-commits mailto:cfe-commits@lists.llvm.org>> 
> wrote:
> george.karpenkov added a comment.
> 
> @dcoughlin the context I was thinking about is that if everyone consistently 
> runs `clang-format` (if we want that), then we never would have discussion.
> The alternative is that every run of `clang-format` would be followed by 
> manually reverting changes which were introduced by mistake (in this case, 
> because the file was moved).
> 
> clang-format has script (git-clang-format) for formatting only a diff, use 
> that or something like it so you're not reformatting unrelated lines. If 
> you're changing so much of the file, or it's so malformatted that local 
> format updates are going to be really inconsistent, reformat the entire file 
> in a standalone commit first, then submit your semantic changes separately.

But that’s what I was using: trouble is, the file got renamed, and clang-format 
then decided to reformat all of it.
(and another question is whether it is a reasonable thing to do)

> 
> (also there's a way to setup clang-format to "format changed lines on-save" 
> in vim, which is what I use - now I basically don't think about formatting :) 
> )
>  
> 
> 
> https://reviews.llvm.org/D39208 
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> 

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

r316936 - Undo accidental language mode change in this test.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Oct 30 11:06:18 2017
New Revision: 316936

URL: http://llvm.org/viewvc/llvm-project?rev=316936&view=rev
Log:
Undo accidental language mode change in this test.

Modified:
cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp

Modified: cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp?rev=316936&r1=316935&r2=316936&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp Mon Oct 30 11:06:18 
2017
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions %s
 // RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions %s 
-Wno-dynamic-exception-spec
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-c++1z-compat-mangling -DNO_COMPAT_MANGLING %s
-// RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
+// RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
 
 #if __cplusplus > 201402L
 


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

r316935 - Add a test to make sure that -Wdeprecated doesn't warn on use of 'throw()' in system headers (deprecated in C++17).

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Oct 30 11:05:10 2017
New Revision: 316935

URL: http://llvm.org/viewvc/llvm-project?rev=316935&view=rev
Log:
Add a test to make sure that -Wdeprecated doesn't warn on use of 'throw()' in 
system headers (deprecated in C++17).

Modified:
cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp
cfe/trunk/test/SemaCXX/deprecated.cpp

Modified: cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp?rev=316935&r1=316934&r2=316935&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1z-noexcept-function-type.cpp Mon Oct 30 11:05:10 
2017
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions %s
-// RUN: %clang_cc1 -std=c++1z -verify -fexceptions -fcxx-exceptions %s 
-Wno-dynamic-exception-spec
+// RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions %s 
-Wno-dynamic-exception-spec
 // RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-c++1z-compat-mangling -DNO_COMPAT_MANGLING %s
-// RUN: %clang_cc1 -std=c++14 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
+// RUN: %clang_cc1 -std=c++17 -verify -fexceptions -fcxx-exceptions 
-Wno-noexcept-type -DNO_COMPAT_MANGLING %s
 
 #if __cplusplus > 201402L
 

Modified: cfe/trunk/test/SemaCXX/deprecated.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/deprecated.cpp?rev=316935&r1=316934&r2=316935&view=diff
==
--- cfe/trunk/test/SemaCXX/deprecated.cpp (original)
+++ cfe/trunk/test/SemaCXX/deprecated.cpp Mon Oct 30 11:05:10 2017
@@ -93,3 +93,6 @@ namespace DeprecatedCopy {
   void g() { c1 = c2; } // expected-note {{implicit copy assignment operator 
for 'DeprecatedCopy::Dtor' first required here}}
 }
 #endif
+
+# 1 "/usr/include/system-header.h" 1 3
+void system_header_function(void) throw();


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

[PATCH] D39405: std::set_union accesses values after move.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

We should backport this, right @mclow.lists?


https://reviews.llvm.org/D39405



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

[PATCH] D39008: [CodeGen] Propagate may-alias'ness of lvalues with TBAA info

[John McCall via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D39008



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> That is my diagnostic, so i guess this is the time to reply :)


...

> 3. In `-Wtautological-constant-compare`, ignore any comparisons that compare 
> with `std::numeric_limits`. Not a fan of this solution. <- Worst?

...

Gee, #3 is the worst?  That one seems the most appealing to me.  I think I 
could understand it if you had reservations about ignoring comparisons against 
{CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is well-qualified and 
everything.

Can you help me understand why you don't prefer it?

Also, is there any way that the warning could somehow be smart enough to know 
what sizeof(int) and sizeof(long) are?

As an aside, how widely has this new clang behavior been tested?  Is it 
possible that this new warning behavior might get rolled back if a 
big/important enough codebase triggers it?


https://reviews.llvm.org/D39149



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:

> I dislike this change fairly strongly.


Agreed. Would you be happier with just disabling the diagnostics around the 
problematic parts?

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> 1. Disable that warning in `libc++` cmakelists. Be careful though, i think it 
> might disable the entire 'tautological comparison' diagnostic family.


Note that one of the tautological comparisons is in a header, so it affects all 
libc++ clients and not just libc++ itself.

> 2. Use preprocessor pragmas to disable the diagnostic for the selected code, 
> https://reviews.llvm.org/rL315882. Brittle, ugly, effective, has the least 
> impact. <- Best?

I was considering doing this instead. It also seems ugly in its own way, but 
possibly less ugly than this patch.

> 5. The essential problem, i *think* is much like the problem with unreachable 
> code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” 
> . The check does not know/care whether 
> the comparison is tautological only with the current Fundamental type sizes, 
> or always. Perhaps this is the proper heading?

Yeah, exactly. If the warning could only fire when the comparison was *always* 
going to be tautological, that would avoid any issues like this, but I'm not 
sure how best to go about that.


https://reviews.llvm.org/D39149



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai planned changes to this revision.
smeenai added a comment.

In https://reviews.llvm.org/D39149#910858, @mclow.lists wrote:

> If we have to go down this road, I'd prefer the approach used in 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?r1=315874&r2=315873&pathrev=315874
>
> (which is your solution #2)


All right, I'll change this patch accordingly.


https://reviews.llvm.org/D39149



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

If we have to go down this road, I'd prefer the approach used in 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?r1=315874&r2=315873&pathrev=315874


https://reviews.llvm.org/D39149



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

[PATCH] D35780: Introduce -nostdlib++ flag to disable linking the C++ standard library.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Sounds like this ended up being useful for Android too: 
https://github.com/android-ndk/ndk/issues/105#issuecomment-324179958


https://reviews.llvm.org/D35780



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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a subscriber: thakis.
lebedev.ri added a comment.

That is my diagnostic, so i guess this is the time to reply :)

In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:

> I dislike this change fairly strongly.
>  I would much rather pursue a clang-based solution (since clang is being 
> unhelpful here)
>  Don't know if we can get one, though.


As i see it, there are several options:

1. Disable that warning in `libc++` cmakelists. Be careful though, i think it 
might disable the entire 'tautological comparison' diagnostic family.
2. Use preprocessor pragmas to disable the diagnostic for the selected code, 
https://reviews.llvm.org/rL315882. Brittle, ugly, effective, has the least 
impact. <- Best?
3. In `-Wtautological-constant-compare`, ignore any comparisons that compare 
with `std::numeric_limits`. Not a fan of this solution. <- Worst?
4. Disable that diagnostic by default in clang. Also, not really a fan of this 
solution. I implemented it because it would have caught a real bug in my code 
in rather shorter time, see mail 
.
5. The essential problem, i *think* is much like the problem with unreachable 
code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” 
. The check does not know/care whether 
the comparison is tautological only with the current Fundamental type sizes, or 
always. Perhaps this is the proper heading?
6. ???


https://reviews.llvm.org/D39149



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

[PATCH] D38844: [analyzer] Make issue hash related tests more concise

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

I'd also include some info on how it's now possible to dump the issue hash. You 
introduce a new debugging function here "clang_analyzer_hashDump" but it's not 
mentioned in the commit message.

Thanks!




Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:68
   // (globals should not be invalidated, etc), hence the use of evalCall.
-  FnCheck Handler = llvm::StringSwitch(C.getCalleeName(CE))
-.Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
-.Case("clang_analyzer_checkInlined",
-  &ExprInspectionChecker::analyzerCheckInlined)
-.Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
-.Case("clang_analyzer_warnIfReached",
-  &ExprInspectionChecker::analyzerWarnIfReached)
-.Case("clang_analyzer_warnOnDeadSymbol",
-  &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
-.StartsWith("clang_analyzer_explain", 
&ExprInspectionChecker::analyzerExplain)
-.StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump)
-.Case("clang_analyzer_getExtent", 
&ExprInspectionChecker::analyzerGetExtent)
-.Case("clang_analyzer_printState",
-  &ExprInspectionChecker::analyzerPrintState)
-.Case("clang_analyzer_numTimesReached",
-  &ExprInspectionChecker::analyzerNumTimesReached)
-.Default(nullptr);
+  FnCheck Handler =
+  llvm::StringSwitch(C.getCalleeName(CE))

xazax.hun wrote:
> zaks.anna wrote:
> > Unrelated change?
> Since I touched this snippet I reformatted it using clang-format. Apart from 
> adding a new case before the default all other changes are formatting 
> changes. I will revert the formatting changes. So in general, we prefer to 
> minimize the diffs over converging to be clang-formatted?
It's much easier to review when the diff does not contain formatting changes 
intermixed with functional changes. Looks like you can configure clang-format 
to only format the diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D38844



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

[PATCH] D39423: [analyzer] Left shifting a negative value is undefined

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316924: [analyzer] Left shifting a negative value is 
undefined (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D39423?vs=120837&id=120841#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39423

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  cfe/trunk/test/Analysis/bitwise-ops.c


Index: cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: cfe/trunk/test/Analysis/bitwise-ops.c
===
--- cfe/trunk/test/Analysis/bitwise-ops.c
+++ cfe/trunk/test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is 
undefined because the left operand is negative}}
+  }
+  return 0;
+}


Index: cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: cfe/trunk/test/Analysis/bitwise-ops.c
===
--- cfe/trunk/test/Analysis/bitwise-ops.c
+++ cfe/trunk/test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is undefined because the left operand is negative}}
+  }
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316924 - [analyzer] Left shifting a negative value is undefined

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Oct 30 10:06:42 2017
New Revision: 316924

URL: http://llvm.org/viewvc/llvm-project?rev=316924&view=rev
Log:
[analyzer] Left shifting a negative value is undefined

The analyzer did not return an UndefVal in case a negative value was left
shifted. I also altered the UndefResultChecker to emit a clear warning in this
case.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
cfe/trunk/test/Analysis/bitwise-ops.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp?rev=316924&r1=316923&r2=316924&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp Mon Oct 30 
10:06:42 2017
@@ -137,6 +137,10 @@ void UndefResultChecker::checkPostStmt(c
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp?rev=316924&r1=316923&r2=316924&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp Mon Oct 30 10:06:42 
2017
@@ -225,6 +225,8 @@ BasicValueFactory::evalAPSInt(BinaryOper
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;

Modified: cfe/trunk/test/Analysis/bitwise-ops.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bitwise-ops.c?rev=316924&r1=316923&r2=316924&view=diff
==
--- cfe/trunk/test/Analysis/bitwise-ops.c (original)
+++ cfe/trunk/test/Analysis/bitwise-ops.c Mon Oct 30 10:06:42 2017
@@ -44,3 +44,10 @@ int testNegativeShift(int a) {
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is 
undefined because the left operand is negative}}
+  }
+  return 0;
+}


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

[PATCH] D39149: [libc++] Prevent tautological comparisons

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

I dislike this change fairly strongly.
I would much rather pursue a clang-based solution (since clang is being 
unhelpful here)
Don't know if we can get one, though.


https://reviews.llvm.org/D39149



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

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

[Reid Kleckner via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D39365#909912, @compnerd wrote:

> @rnk given that the remote unwinder is completely unimplemented ATM, I think 
> that this isn't a big concern.  I'm not sure that this makes anything worse, 
> but I do feel that it is making things better (especially since 
> `RemoteAddressSpace` is currently mostly just a '// FIXME: implement').


Sounds good to me, just wanted to check. :)


https://reviews.llvm.org/D39365



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

[PATCH] D39370: [clang-tidy] Detect bugs in bugprone-misplaced-operator-in-strlen-in-alloc even in the case the allocation function is called using a constant function pointer

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39370#910803, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39370#910791, @aaron.ballman wrote:
>
> > LGTM
> >
> > FYI: it's been difficult to perform this review because all of these 
> > reviews are touching the same chunk of code for something that's not been 
> > committed yet. It would be easier to review if all of these reviews were 
> > combined into the review adding the check.
>
>
> I am also working for the Static Analyzer where I received the comment that 
> development should be incremental to avoid huge patches. So I tried to use 
> the same approach here as well.


Incremental is definitely the way to go, but that's usually to prevent massive 
code dumps of large-scale functionality. For a single, relatively small check 
like this, I think it's fine to add all of this into one review because it's 
all so tightly related.


https://reviews.llvm.org/D39370



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

[PATCH] D39370: [clang-tidy] Detect bugs in bugprone-misplaced-operator-in-strlen-in-alloc even in the case the allocation function is called using a constant function pointer

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39370#910791, @aaron.ballman wrote:

> LGTM
>
> FYI: it's been difficult to perform this review because all of these reviews 
> are touching the same chunk of code for something that's not been committed 
> yet. It would be easier to review if all of these reviews were combined into 
> the review adding the check.


I am also working for the Static Analyzer where I received the comment that 
development should be incremental to avoid huge patches. So I tried to use the 
same approach here as well.


https://reviews.llvm.org/D39370



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

[PATCH] D39367: [clang-tidy] Add support for operator new[] in check bugprone-misplaced-operator-in-strlen-in-alloc

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D39367



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

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

[Tamas Berghammer via Phabricator via cfe-commits]

tberghammer added a comment.

2 fairly random ideas without looking into it too much:

- Can you do a diff of the debug_info dump before and after your change? 
Understanding what have changed should give us a pretty good clue about the 
issue.
- My first guess is that after your change we emit DW_TAG_enumeration_type for 
scoped enums (what seems to be the correct thing to do) instead of something 
else (not sure what) and you have to update 
https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp#L1512
 to handle it correctly.


https://reviews.llvm.org/D39239



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

[PATCH] D38688: [clang-tidy] Misc redundant expressions checker updated for macros

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:633-635
+// The function discards  (X < M1 && X <= M2), because it can always be
+// simplified to X < M, where M is the smaller one of the two macro
+// values.

barancsuk wrote:
> aaron.ballman wrote:
> > This worries me slightly for macros because those values can be overridden 
> > for different compilation targets. So the diagnostic may be correct for a 
> > particular configuration of the macros, but incorrect for another. Have you 
> > tested this against any large code bases to see what the quality of the 
> > diagnostics looks like?
> Thank you for your insight! 
> 
> Now that I come to think of it, although these expressions are redundant for 
> every configuration, there is no adequate simplification for each compilation 
> target, because they might change their behavior if the values vary.
> 
> A good example is the following expression, that is always redundant 
> regardless of the values that the macros hold.
> However, the particular macro that causes the redundancy can vary from one 
> compilation target to another.
> ```
> (X < MACRO1 || X < MACRO2)
> ```
> If MACRO1 > MACRO2, the appropriate simplification is `(X < MACRO1 )`, 
> and  if MACRO1 < MACRO2, it is `(X < MACRO2)`.
> 
> Even expressions, like `(X < MACRO1 || X != MACRO2)`, that can almost always 
> be simplified to `(X != MACRO2)`, change their behavior, if MACRO1 is equal 
> to MACRO2  (then they are always true), and can be considered conscious 
> development choices.
> 
> In conclusion, I think, it might be better now to skip these expressions, and 
> check for only the ones that contain the same macro twice, like `X < MACRO && 
> X > MACRO`.
> 
> In conclusion, I think, it might be better now to skip these expressions, and 
> check for only the ones that contain the same macro twice, like X < MACRO && 
> X > MACRO.

I think that approach makes the most sense, at least initially.


https://reviews.llvm.org/D38688



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

Re: [PATCH] D24886: Add [[clang::suppress(rule, ...)]] attribute

[Aaron Ballman via cfe-commits]

On Sun, Oct 29, 2017 at 4:42 AM, Hristo Hristov via Phabricator
 wrote:
> roccoor added a comment.
>
> Microsoft supports:
>
>   [[gsl::suppress(bounds.3)]]
>
> Clang requires:
>
>   [[gsl::suppress("bounds.3")]]
>
> Why is this inconsistency?

I believe that when we added this attribute, MSVC did not support the
attribute at all and so we did what was recommended by the C++ Core
Guideline maintainers.

It would be possible for us to support the Microsoft syntax, but it
may require special parsing support.

~Aaron

>
> Here is an example from  CppCoreGuidelines
>
>   [[suppress(bounds)]] char* raw_find(char* p, int n, char x)// find x in 
> p[0]..p[n - 1]
>   {
>   // ...
>   }
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24886
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39370: [clang-tidy] Detect bugs in bugprone-misplaced-operator-in-strlen-in-alloc even in the case the allocation function is called using a constant function pointer

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM

FYI: it's been difficult to perform this review because all of these reviews 
are touching the same chunk of code for something that's not been committed 
yet. It would be easier to review if all of these reviews were combined into 
the review adding the check.


https://reviews.llvm.org/D39370



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

[PATCH] D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions.

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

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

This patch looks good to me.
I am wondering whether it would make sense to have some kind of warning (even 
in the frontend if not in the analyzer) for using standard functions with 
non-standard signatures, so users start to file bugs for the maintainers of 
those libraries.


https://reviews.llvm.org/D39422



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

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39121#910735, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39121#910715, @aaron.ballman wrote:
>
> > The diagnostic tells the user that you surround the arg to strlen with 
> > parens to silence the diagnostic, but the fixit doesn't do that -- it moves 
> > the addition to the result. That's confusing behavior. However, if you add 
> > another fixit to surround the arg with parens (and leave in the existing 
> > one), then there are conflicting fixits (you don't want to apply them both) 
> > which would also be confusing.
>
>
> Then, I think we should rephrase the diagnostic message again. I am confident 
> that in at least 99.9% of the cases adding 1 to the argument is a mistake. So 
> the primary suggestion should be to move the addition to the result instead. 
> The suggestion to put the argument in extra parentheses should be secondary, 
> maybe also put in parentheses.


Agreed -- we want to keep the fixit with moving the +1 out to the result. 
However, I'm still worried that there'd be no way for the user to know how to 
silence the warning if the code is already correct -- I don't want users to 
disable a useful check because they don't have guidance on how to silence the 
diagnostic.

Do you have concrete numbers of how many diagnostics are triggered on large 
code bases, and the false positive rate?


https://reviews.llvm.org/D39121



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

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a reviewer: tberghammer.
CarlosAlbertoEnciso added a comment.

Hi Tamas,

I have added you to this review, as I think I have found something odd with the 
LLDB.

Basically after this intended patch to the compiler (to emit scoped information 
only for scoped enums) the following test fail:

tools/lldb/packages/Python/lldbsuite/test/lang/cpp/template/TestTemplateArgs.py

I have saved the objects generated before and after the change for both 
32/64bits mode.  The test case is checking for scoped information which is 
present in the debug information.

  def test_enum_args(self):
  frame = self.prepareProcess()
  
  # Make sure "member" can be displayed and also used in an expression
  # correctly
  member = frame.FindVariable('member')
  self.assertTrue(
  member.IsValid(),
  'make sure we find a local variabble named "member"')
  
  self.assertTrue(member.GetType().GetName() ==
  'EnumTemplate')

After some debugging, 'TestTemplateArgs.py' fails when retrieving the member 
type name, which is expected to have the 'EnumTemplate'. As 
per my previous comment, that string is present; but the expression 
'member.GetType().GetName()' returns just 'EnumTemplate' causing the 
test to fail.

I would appreciate it very much your feedback.

Thanks


https://reviews.llvm.org/D39239



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

[PATCH] D39423: [analyzer] Left shifting a negative value is undefined

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

xazax.hun created this revision.
Herald added subscribers: szepet, baloghadamsoftware, whisperity.

The analyzer did not return an UndefVal in case a negative value was left 
shifted.
I also added altered the UndefResultChecker to emit a clear warning in this 
case.


Repository:
  rL LLVM

https://reviews.llvm.org/D39423

Files:
  lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  test/Analysis/bitwise-ops.c


Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is 
undefined because the left operand is negative}}
+  }
+  return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())


Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -44,3 +44,10 @@
   }
   return 0;
 }
+
+int testNegativeLeftShift(int a) {
+  if (a == -3) {
+return a << 1; // expected-warning{{The result of the left shift is undefined because the left operand is negative}}
+  }
+  return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -225,6 +225,8 @@
   // test these conditions symbolically.
 
   // FIXME: Expand these checks to include all undefined behavior.
+  if (V1.isSigned() && V1.isNegative())
+return nullptr;
 
   if (V2.isSigned() && V2.isNegative())
 return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -137,6 +137,10 @@
 
 OS << " greater or equal to the width of type '"
<< B->getLHS()->getType().getAsString() << "'.";
+  } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
+ C.isNegative(B->getLHS())) {
+OS << "The result of the left shift is undefined because the left "
+  "operand is negative";
   } else {
 OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39422: [analyzer] pr34779: CStringChecker: Don't get crashed by non-standard standard library function definitions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

An assertion failure was triggered in pr34779 by defining `strcpy` as `void 
(unsigned char *, unsigned char *)`, while normally it is `char *(char *, char 
*)`. The assertion is removed because normally we try not to crash in such 
cases, but simply refuse to model. For now i did not try to actually model the 
modified strcpy, because it requires more work (unhardcode `CharTy` in a lot of 
places around the checker), and supporting other aspects of the non-standard 
definition requires even more work (i.e. we try to bind the return value, need 
to disable this mechanism when the return type is void), and if we try to model 
other similar functions (are other string functions accepting unsigned chars as 
well?) it'd bloat quite a bit.

So //for now the analyzer would not find C string errors// in code that defines 
string functions in a non-standard manner. It simply tries to avoid crashing. 
To think - maybe we should add more defensive checks (eg. into 
`CallDescription`).


https://reviews.llvm.org/D39422

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string-with-signedness.c


Index: test/Analysis/string-with-signedness.c
===
--- /dev/null
+++ test/Analysis/string-with-signedness.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration 
-analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+
+// expected-no-diagnostics
+
+void *strcpy(unsigned char *, unsigned char *);
+
+unsigned char a, b;
+void testUnsignedStrcpy() {
+  strcpy(&a, &b);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -289,8 +289,8 @@
   if (!ER)
 return state;
 
-  assert(ER->getValueType() == C.getASTContext().CharTy &&
-"CheckLocation should only be called with char* ElementRegions");
+  if (ER->getValueType() != C.getASTContext().CharTy)
+return state;
 
   // Get the size of the array.
   const SubRegion *superReg = cast(ER->getSuperRegion());
@@ -874,6 +874,8 @@
   if (!ER)
 return true; // cf top comment.
 
+  // FIXME: Does this crash when a non-standard definition
+  // of a library function is encountered?
   assert(ER->getValueType() == C.getASTContext().CharTy &&
  "IsFirstBufInBound should only be called with char* ElementRegions");
 


Index: test/Analysis/string-with-signedness.c
===
--- /dev/null
+++ test/Analysis/string-with-signedness.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration -analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+
+// expected-no-diagnostics
+
+void *strcpy(unsigned char *, unsigned char *);
+
+unsigned char a, b;
+void testUnsignedStrcpy() {
+  strcpy(&a, &b);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -289,8 +289,8 @@
   if (!ER)
 return state;
 
-  assert(ER->getValueType() == C.getASTContext().CharTy &&
-"CheckLocation should only be called with char* ElementRegions");
+  if (ER->getValueType() != C.getASTContext().CharTy)
+return state;
 
   // Get the size of the array.
   const SubRegion *superReg = cast(ER->getSuperRegion());
@@ -874,6 +874,8 @@
   if (!ER)
 return true; // cf top comment.
 
+  // FIXME: Does this crash when a non-standard definition
+  // of a library function is encountered?
   assert(ER->getValueType() == C.getASTContext().CharTy &&
  "IsFirstBufInBound should only be called with char* ElementRegions");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r316917 - Mark test as unsupported on C++98/03, since it uses move_iterator

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Oct 30 09:07:59 2017
New Revision: 316917

URL: http://llvm.org/viewvc/llvm-project?rev=316917&view=rev
Log:
Mark test as unsupported on C++98/03, since it uses move_iterator

Modified:

libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp

Modified: 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp?rev=316917&r1=316916&r2=316917&view=diff
==
--- 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 Mon Oct 30 09:07:59 2017
@@ -19,6 +19,8 @@
 //   set_union(InIter1 first1, InIter1 last1, InIter2 first2, InIter2 last2,
 // OutIter result, Compare comp);
 
+// UNSUPPORTED: c++98, c++03
+
 #include 
 #include 
 #include 


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

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: include/llvm/IR/Metadata.h:961
 
+  /// \brief Resolve a unique, unresolved node.
+  void resolve();

The `\brief ` is redundant and can be dropped.


https://reviews.llvm.org/D39396



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

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D39365#910713, @theraven wrote:

> Sorry - it looks as if I read the diff back to front.  I seem to be less 
> awake than I thought today...
>
> Reading the diff the correct way around, this seems like a definite 
> improvement.


Ok, thanks! Then it's just left to see if @rnk agrees with @compnerd 's 
reasoning.

>>> though the use of `PRIxPTR` vs `PRIXPTR` seems inconsistent (as is the 
>>> original use of `%x` vs `%X`.
>> 
>> Yes, I've kept these as inconsistent as they were originally - if peferred I 
>> can make the ones I touch consistently either upper or lower case.
> 
> I'd generally prefer `PRIxPTR`, because most of the time I either don't care 
> or want to copy and paste for comparison with objdump output (which uses 
> lower case).

Ok, I'll make it consistent in the next update or when committing.


https://reviews.llvm.org/D39365



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

[PATCH] D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> The only drawback is that the cloned function's (the thunk's) 
> DILocalVariableNodes will not appear in the newly cloned DISubprogram's node 
> variable list. Instead, the new node points to the original function's 
> variable list. However, since the only difference between the original 
> function and the thunk is the this-ptr adjustment, the resulting debug 
> information is correct, at least in the test cases I have looked at.

Does the thunk contain any local variables other than function arguments?
What I'm getting at is: If this is only a thunk, do we need any local variables 
at all?




Comment at: lib/CodeGen/CGVTables.cpp:130
+// Furthermore, the function resolves any DILocalVariable nodes referenced
+// by dbg.value intrinsics so they can be properly mapped during cloning.
+static void resolveTopLevelMetadata(llvm::Function *Fn,

`///`



Comment at: lib/CodeGen/CGVTables.cpp:140
+
+  // Find all debug.declare intrinsics and resolve the DILocalVariable nodes
+  // they are referencing.

`debug.declare` -> (llvm.)`dbg.declare`.



Comment at: lib/CodeGen/CGVTables.cpp:143
+  for (llvm::Function::iterator BB = Fn->begin(), E = Fn->end(); BB != E;
+   ++BB) {
+for (llvm::Instruction &I : *BB) {

does this work?
`for (auto &BB : Fn->getBasicBlockList())`



Comment at: lib/CodeGen/CGVTables.cpp:145
+for (llvm::Instruction &I : *BB) {
+  auto *DII = dyn_cast(&I);
+  if (DII) {

`if (auto *DII ...`


https://reviews.llvm.org/D39396



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

[PATCH] D39405: std::set_union accesses values after move.

[Marshall Clow via Phabricator via cfe-commits]

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

Committed as revision 316914. Thanks!


https://reviews.llvm.org/D39405



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

[libcxx] r316914 - Fix PR#35119 : set_union misbehaves with move_iterators. Thanks to Denis Yaroshevskiy for both the bug report and the fix.

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon Oct 30 08:50:00 2017
New Revision: 316914

URL: http://llvm.org/viewvc/llvm-project?rev=316914&view=rev
Log:
Fix PR#35119 : set_union misbehaves with move_iterators. Thanks to Denis 
Yaroshevskiy for both the bug report and the fix.

Added:

libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
Modified:
libcxx/trunk/include/algorithm

Modified: libcxx/trunk/include/algorithm
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=316914&r1=316913&r2=316914&view=diff
==
--- libcxx/trunk/include/algorithm (original)
+++ libcxx/trunk/include/algorithm Mon Oct 30 08:50:00 2017
@@ -5547,9 +5547,9 @@ __set_union(_InputIterator1 __first1, _I
 }
 else
 {
-*__result = *__first1;
 if (!__comp(*__first1, *__first2))
 ++__first2;
+*__result = *__first1;
 ++__first1;
 }
 }

Added: 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp?rev=316914&view=auto
==
--- 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 (added)
+++ 
libcxx/trunk/test/std/algorithms/alg.sorting/alg.set.operations/set.union/set_union_move.pass.cpp
 Mon Oct 30 08:50:00 2017
@@ -0,0 +1,44 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template
+//   requires OutputIterator
+// && OutputIterator
+// && Predicate
+// && Predicate
+//   OutIter
+//   set_union(InIter1 first1, InIter1 last1, InIter2 first2, InIter2 last2,
+// OutIter result, Compare comp);
+
+#include 
+#include 
+#include 
+#include 
+
+#include "MoveOnly.h"
+
+
+int main()
+{
+std::vector lhs, rhs;
+lhs.push_back(MoveOnly(2));
+rhs.push_back(MoveOnly(2));
+
+std::vector res;
+std::set_union(std::make_move_iterator(lhs.begin()),
+   std::make_move_iterator(lhs.end()),
+   std::make_move_iterator(rhs.begin()),
+   std::make_move_iterator(rhs.end()), 
std::back_inserter(res));
+
+assert(res.size() == 1);
+assert(res[0].get() == 2);
+}


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

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39121#910715, @aaron.ballman wrote:

> The diagnostic tells the user that you surround the arg to strlen with parens 
> to silence the diagnostic, but the fixit doesn't do that -- it moves the 
> addition to the result. That's confusing behavior. However, if you add 
> another fixit to surround the arg with parens (and leave in the existing 
> one), then there are conflicting fixits (you don't want to apply them both) 
> which would also be confusing.


Then, I think we should rephrase the diagnostic message again. I am confident 
that in at least 99.9% of the cases adding 1 to the argument is a mistake. So 
the primary suggestion should be to move the addition to the result instead. 
The suggestion to put the argument in extra parentheses should be secondary, 
maybe also put in parentheses.


https://reviews.llvm.org/D39121



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

Re: [PATCH] D39208: [Analyzer] Do not use static storage to for implementations created in BodyFarm.cpp

[David Blaikie via cfe-commits]

On Mon, Oct 23, 2017 at 5:01 PM George Karpenkov via Phabricator via
cfe-commits  wrote:

> george.karpenkov added a comment.
>
> @dcoughlin the context I was thinking about is that if everyone
> consistently runs `clang-format` (if we want that), then we never would
> have discussion.
> The alternative is that every run of `clang-format` would be followed by
> manually reverting changes which were introduced by mistake (in this case,
> because the file was moved).
>

clang-format has script (git-clang-format) for formatting only a diff, use
that or something like it so you're not reformatting unrelated lines. If
you're changing so much of the file, or it's so malformatted that local
format updates are going to be really inconsistent, reformat the entire
file in a standalone commit first, then submit your semantic changes
separately.

(also there's a way to setup clang-format to "format changed lines on-save"
in vim, which is what I use - now I basically don't think about formatting
:) )


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

Re: r316536 - [Analyzer] Store BodyFarm in std::unique_ptr

[David Blaikie via cfe-commits]

Could this be a value rather than indirected through a unique_ptr? Simply:

  BodyFarm BdyFrm;

& initialized in the ctors init list?

On Tue, Oct 24, 2017 at 4:53 PM George Karpenkov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: george.karpenkov
> Date: Tue Oct 24 16:53:19 2017
> New Revision: 316536
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316536&view=rev
> Log:
> [Analyzer] Store BodyFarm in std::unique_ptr
>
> Differential Revision: https://reviews.llvm.org/D39220
>
> Modified:
> cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
> cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=316536&r1=316535&r2=316536&view=diff
>
> ==
> --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original)
> +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Tue Oct 24
> 16:53:19 2017
> @@ -421,7 +421,7 @@ class AnalysisDeclContextManager {
>
>/// Pointer to a factory for creating and caching implementations for
> common
>/// methods during the analysis.
> -  BodyFarm *BdyFrm = nullptr;
> +  std::unique_ptr BdyFrm;
>
>/// Flag to indicate whether or not bodies should be synthesized
>/// for well-known functions.
> @@ -438,8 +438,6 @@ public:
>   bool addCXXNewAllocator = true,
>   CodeInjector *injector = nullptr);
>
> -  ~AnalysisDeclContextManager();
> -
>AnalysisDeclContext *getContext(const Decl *D);
>
>bool getUseUnoptimizedCFG() const {
>
> Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=316536&r1=316535&r2=316536&view=diff
>
> ==
> --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
> +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Tue Oct 24 16:53:19 2017
> @@ -306,8 +306,8 @@ AnalysisDeclContext *AnalysisDeclContext
>
>  BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
>if (!BdyFrm)
> -BdyFrm = new BodyFarm(ASTCtx, Injector.get());
> -  return BdyFrm;
> +BdyFrm = llvm::make_unique(ASTCtx, Injector.get());
> +  return BdyFrm.get();
>  }
>
>  const StackFrameContext *
> @@ -603,11 +603,6 @@ AnalysisDeclContext::~AnalysisDeclContex
>}
>  }
>
> -AnalysisDeclContextManager::~AnalysisDeclContextManager() {
> -  if (BdyFrm)
> -delete BdyFrm;
> -}
> -
>  LocationContext::~LocationContext() {}
>
>  LocationContextManager::~LocationContextManager() {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r316403 - [Analyzer] Fix for the memory leak: fix typo in if-statement.

[David Blaikie via cfe-commits]

I realize this was changed in a follow-up commit anyway, but for future
reference: There's no need (& best to avoid - simpler to read, avoids bugs
like this, etc) to conditionalize delete like this. Delete is a no-op on
null pointers anyway, so this dtor should just contain an unconditional
"delete BdyFrm;"

On Mon, Oct 23, 2017 at 6:09 PM George Karpenkov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: george.karpenkov
> Date: Mon Oct 23 18:09:43 2017
> New Revision: 316403
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316403&view=rev
> Log:
> [Analyzer] Fix for the memory leak: fix typo in if-statement.
>
> Modified:
> cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
>
> Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=316403&r1=316402&r2=316403&view=diff
>
> ==
> --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
> +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Mon Oct 23 18:09:43 2017
> @@ -604,7 +604,7 @@ AnalysisDeclContext::~AnalysisDeclContex
>  }
>
>  AnalysisDeclContextManager::~AnalysisDeclContextManager() {
> -  if (!BdyFrm)
> +  if (BdyFrm)
>  delete BdyFrm;
>  }
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39008: [CodeGen] Propagate may-alias'ness of lvalues with TBAA info

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev updated this revision to Diff 120828.
kosarev added a comment.

- Fixed comparing and hashing of TBAA access descriptors.
- Rebased on top of https://reviews.llvm.org/D39177.

Thanks John!


https://reviews.llvm.org/D39008

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGValue.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/CodeGenTBAA.cpp
  lib/CodeGen/CodeGenTBAA.h

Index: lib/CodeGen/CodeGenTBAA.h
===
--- lib/CodeGen/CodeGenTBAA.h
+++ lib/CodeGen/CodeGenTBAA.h
@@ -32,11 +32,22 @@
 namespace CodeGen {
 class CGRecordLayout;
 
+// TBAAAccessKind - A kind of TBAA memory access descriptor.
+enum class TBAAAccessKind : unsigned {
+  Ordinary,
+  MayAlias,
+};
+
 // TBAAAccessInfo - Describes a memory access in terms of TBAA.
 struct TBAAAccessInfo {
+  TBAAAccessInfo(TBAAAccessKind Kind, llvm::MDNode *BaseType,
+ llvm::MDNode *AccessType, uint64_t Offset)
+: Kind(Kind), BaseType(BaseType), AccessType(AccessType), Offset(Offset)
+  {}
+
   TBAAAccessInfo(llvm::MDNode *BaseType, llvm::MDNode *AccessType,
  uint64_t Offset)
-: BaseType(BaseType), AccessType(AccessType), Offset(Offset)
+: TBAAAccessInfo(TBAAAccessKind::Ordinary, BaseType, AccessType, Offset)
   {}
 
   explicit TBAAAccessInfo(llvm::MDNode *AccessType)
@@ -47,12 +58,31 @@
 : TBAAAccessInfo(/* AccessType= */ nullptr)
   {}
 
+  static TBAAAccessInfo getMayAliasInfo() {
+return TBAAAccessInfo(TBAAAccessKind::MayAlias, /* BaseType= */ nullptr,
+  /* AccessType= */ nullptr, /* Offset= */ 0);
+  }
+
+  bool isMayAlias() const { return Kind == TBAAAccessKind::MayAlias; }
+
   bool operator==(const TBAAAccessInfo &Other) const {
-return BaseType == Other.BaseType &&
+return Kind == Other.Kind &&
+   BaseType == Other.BaseType &&
AccessType == Other.AccessType &&
Offset == Other.Offset;
   }
 
+  bool operator!=(const TBAAAccessInfo &Other) const {
+return !(*this == Other);
+  }
+
+  explicit operator bool() const {
+return *this != TBAAAccessInfo();
+  }
+
+  /// Kind - The kind of the access descriptor.
+  TBAAAccessKind Kind;
+
   /// BaseType - The base/leading access type. May be null if this access
   /// descriptor represents an access that is not considered to be an access
   /// to an aggregate or union member.
@@ -139,14 +169,15 @@
   /// getAccessTagInfo - Get TBAA tag for a given memory access.
   llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
 
-  /// getMayAliasAccessInfo - Get TBAA information that represents may-alias
-  /// accesses.
-  TBAAAccessInfo getMayAliasAccessInfo();
-
   /// mergeTBAAInfoForCast - Get merged TBAA information for the purpose of
   /// type casts.
   TBAAAccessInfo mergeTBAAInfoForCast(TBAAAccessInfo SourceInfo,
   TBAAAccessInfo TargetInfo);
+
+  /// mergeTBAAInfoForConditionalOperator - Get merged TBAA information for the
+  /// purpose of conditional operator.
+  TBAAAccessInfo mergeTBAAInfoForConditionalOperator(TBAAAccessInfo InfoA,
+ TBAAAccessInfo InfoB);
 };
 
 }  // end namespace CodeGen
@@ -156,30 +187,34 @@
 
 template<> struct DenseMapInfo {
   static clang::CodeGen::TBAAAccessInfo getEmptyKey() {
+unsigned UnsignedKey = DenseMapInfo::getEmptyKey();
 return clang::CodeGen::TBAAAccessInfo(
+  static_cast(UnsignedKey),
   DenseMapInfo::getEmptyKey(),
   DenseMapInfo::getEmptyKey(),
   DenseMapInfo::getEmptyKey());
   }
 
   static clang::CodeGen::TBAAAccessInfo getTombstoneKey() {
+unsigned UnsignedKey = DenseMapInfo::getTombstoneKey();
 return clang::CodeGen::TBAAAccessInfo(
+  static_cast(UnsignedKey),
   DenseMapInfo::getTombstoneKey(),
   DenseMapInfo::getTombstoneKey(),
   DenseMapInfo::getTombstoneKey());
   }
 
   static unsigned getHashValue(const clang::CodeGen::TBAAAccessInfo &Val) {
-return DenseMapInfo::getHashValue(Val.BaseType) ^
+auto KindValue = static_cast(Val.Kind);
+return DenseMapInfo::getHashValue(KindValue) ^
+   DenseMapInfo::getHashValue(Val.BaseType) ^
DenseMapInfo::getHashValue(Val.AccessType) ^
DenseMapInfo::getHashValue(Val.Offset);
   }
 
   static bool isEqual(const clang::CodeGen::TBAAAccessInfo &LHS,
   const clang::CodeGen::TBAAAccessInfo &RHS) {
-return LHS.BaseType == RHS.BaseType &&
-   LHS.AccessType == RHS.AccessType &&
-   LHS.Offset == RHS.Offset;
+return LHS == RHS;
   }
 };
 
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -88,6 +88,25 @@
   return false;
 }
 
+///

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39121#910440, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D39121#909255, @aaron.ballman wrote:
>
> > My only remaining concern is with the diagnostic message/fixit interaction 
> > itself. Let's see if @alexfh has any suggestions there, or we think of an 
> > improvement ourselves.
>
>
> What do you mean by message/fixit interaction and what is your concern there?


The diagnostic tells the user that you surround the arg to strlen with parens 
to silence the diagnostic, but the fixit doesn't do that -- it moves the 
addition to the result. That's confusing behavior. However, if you add another 
fixit to surround the arg with parens (and leave in the existing one), then 
there are conflicting fixits (you don't want to apply them both) which would 
also be confusing.


https://reviews.llvm.org/D39121



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

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D39365#910622, @mstorsjo wrote:

> In https://reviews.llvm.org/D39365#910590, @theraven wrote:
>
> > This makes things worse for us.  On CHERI, `[u]intptr_t` is a (`typedef` 
> > for a) built-in type that can hold a capability.  Having `unw_word_t` be 
> > `uintptr_t`
>
>
> For understanding, I guess you meant "Having `unw_word_t` be `uint64_t`" 
> here? Because othewise, that's exactly the change I'm doing - currently it's 
> `uint64_t` while I'm proposing making it `uintptr_t` - that you're saying is 
> easier to work with?


Sorry - it looks as if I read the diff back to front.  I seem to be less awake 
than I thought today...

Reading the diff the correct way around, this seems like a definite improvement.

>> though the use of `PRIxPTR` vs `PRIXPTR` seems inconsistent (as is the 
>> original use of `%x` vs `%X`.
> 
> Yes, I've kept these as inconsistent as they were originally - if peferred I 
> can make the ones I touch consistently either upper or lower case.

I'd generally prefer `PRIxPTR`, because most of the time I either don't care or 
want to copy and paste for comparison with objdump output (which uses lower 
case).


https://reviews.llvm.org/D39365



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

[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> however as far as I see this will mean the LoopUnroller AST matchers can't be 
> reused unless I change them.

Hmm. What is the problem here? I'm expecting library linkage issues (i.e. 
libAnalysis is not linked to libASTMatchers directly), but i guess it'd better 
to have the whole thing in libAnalysis then (Peter, WDYT?). Or maybe we could 
move the code around a bit, so that we're still in libStaticAnalyzerCore but at 
roughly the same moment of time.

Because you're duplicating the solution for exact same problem, using different 
technology, and this problem is not trivial (eg. you still need to consider 
reference-type declstmts and initializer lists which are not taken into account 
in your solution yet) so i'd dislike the idea of duplicating it.




Comment at: lib/Analysis/CallGraph.cpp:104
+
+  void markModifiedVars(Stmt *S) {
+// Increment/Decrement, taking address of variable

A hint, even if we don't use visitors: the whole point of having a visitor is 
about not needing to make a pattern-matching (switch or sequence of dyn_casts) 
by statement kind yourself, like you do in this function. You already have 
VisitUnaryOperator, VisitBinaryOperator, VisitCallExpr, etc., so you can add 
the code there.


Repository:
  rL LLVM

https://reviews.llvm.org/D37897



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

r316910 - [clang-format] Handle CRLF correctly when formatting escaped newlines

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Mon Oct 30 07:41:34 2017
New Revision: 316910

URL: http://llvm.org/viewvc/llvm-project?rev=316910&view=rev
Log:
[clang-format] Handle CRLF correctly when formatting escaped newlines

Subscribers: klimek

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

Contributed by @peterbudai!

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

Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=316910&r1=316909&r2=316910&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Mon Oct 30 07:41:34 2017
@@ -554,13 +554,21 @@ FormatToken *FormatTokenLexer::getNextTo
   // take them into account as whitespace - this pattern is quite frequent
   // in macro definitions.
   // FIXME: Add a more explicit test.
-  while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\' &&
- FormatTok->TokenText[1] == '\n') {
+  while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\') {
+unsigned SkippedWhitespace = 0;
+if (FormatTok->TokenText.size() > 2 &&
+(FormatTok->TokenText[1] == '\r' && FormatTok->TokenText[2] == '\n'))
+  SkippedWhitespace = 3;
+else if (FormatTok->TokenText[1] == '\n')
+  SkippedWhitespace = 2;
+else
+  break;
+
 ++FormatTok->NewlinesBefore;
-WhitespaceLength += 2;
-FormatTok->LastNewlineOffset = 2;
+WhitespaceLength += SkippedWhitespace;
+FormatTok->LastNewlineOffset = SkippedWhitespace;
 Column = 0;
-FormatTok->TokenText = FormatTok->TokenText.substr(2);
+FormatTok->TokenText = FormatTok->TokenText.substr(SkippedWhitespace);
   }
 
   FormatTok->WhitespaceRange = SourceRange(

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=316910&r1=316909&r2=316910&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Oct 30 07:41:34 2017
@@ -2629,14 +2629,39 @@ TEST_F(FormatTest, FormatUnbalancedStruc
 }
 
 TEST_F(FormatTest, EscapedNewlines) {
-  EXPECT_EQ(
-  "#define A \\\n  int i;  \\\n  int j;",
-  format("#define A \\\nint i;\\\n  int j;", getLLVMStyleWithColumns(11)));
+  FormatStyle Narrow = getLLVMStyleWithColumns(11);
+  EXPECT_EQ("#define A \\\n  int i;  \\\n  int j;",
+format("#define A \\\nint i;\\\n  int j;", Narrow));
   EXPECT_EQ("#define A\n\nint i;", format("#define A \\\n\n int i;"));
   EXPECT_EQ("template  f();", format("\\\ntemplate  f();"));
   EXPECT_EQ("/* \\  \\  \\\n */", format("\\\n/* \\  \\  \\\n */"));
   EXPECT_EQ("", format(""));
 
+  FormatStyle AlignLeft = getLLVMStyle();
+  AlignLeft.AlignEscapedNewlines = FormatStyle::ENAS_Left;
+  EXPECT_EQ("#define MACRO(x) \\\n"
+"private: \\\n"
+"  int x(int a);\n",
+format("#define MACRO(x) \\\n"
+   "private: \\\n"
+   "  int x(int a);\n",
+   AlignLeft));
+
+  // CRLF line endings
+  EXPECT_EQ("#define A \\\r\n  int i;  \\\r\n  int j;",
+format("#define A \\\r\nint i;\\\r\n  int j;", Narrow));
+  EXPECT_EQ("#define A\r\n\r\nint i;", format("#define A \\\r\n\r\n int i;"));
+  EXPECT_EQ("template  f();", format("\\\ntemplate  f();"));
+  EXPECT_EQ("/* \\  \\  \\\r\n */", format("\\\r\n/* \\  \\  \\\r\n */"));
+  EXPECT_EQ("", format(""));
+  EXPECT_EQ("#define MACRO(x) \\\r\n"
+"private: \\\r\n"
+"  int x(int a);\r\n",
+format("#define MACRO(x) \\\r\n"
+   "private: \\\r\n"
+   "  int x(int a);\r\n",
+   AlignLeft));
+
   FormatStyle DontAlign = getLLVMStyle();
   DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign;
   DontAlign.MaxEmptyLinesToKeep = 3;
@@ -2659,7 +2684,8 @@ TEST_F(FormatTest, EscapedNewlines) {
"\\\n"
"  public: \\\n"
"void baz(); \\\n"
-   "  };", DontAlign));
+   "  };",
+   DontAlign));
 }
 
 TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) {


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

[PATCH] D39374: CodeGen: Fix insertion position of addrspace cast for alloca

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316909: CodeGen: Fix insertion position of addrspace cast 
for alloca (authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D39374?vs=120609&id=120823#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39374

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/test/CodeGenCXX/vla.cpp


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -75,7 +75,11 @@
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
 llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
-Builder.SetInsertPoint(AllocaInsertPt);
+// When ArraySize is nullptr, alloca is inserted at AllocaInsertPt,
+// otherwise alloca is inserted at the current insertion point of the
+// builder.
+if (!ArraySize)
+  Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
Index: cfe/trunk/test/CodeGenCXX/vla.cpp
===
--- cfe/trunk/test/CodeGenCXX/vla.cpp
+++ cfe/trunk/test/CodeGenCXX/vla.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin %s -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin %s -emit-llvm -o - | 
FileCheck -check-prefixes=X64,CHECK %s
+// RUN: %clang_cc1 -std=c++11 -triple amdgcn---amdgiz %s -emit-llvm -o - | 
FileCheck -check-prefixes=AMD,CHECK %s
 
 template
 struct S {
@@ -9,18 +10,26 @@
 int f() {
   // Make sure that the reference here is enough to trigger the instantiation 
of
   // the static data member.
-  // CHECK: @_ZN1SIiE1nE = linkonce_odr global i32 5
+  // CHECK: @_ZN1SIiE1nE = linkonce_odr{{.*}} global i32 5
   int a[S::n];
   return sizeof a;
 }
 
 // rdar://problem/9506377
 void test0(void *array, int n) {
   // CHECK-LABEL: define void @_Z5test0Pvi(
-  // CHECK:  [[ARRAY:%.*]] = alloca i8*, align 8
-  // CHECK-NEXT: [[N:%.*]] = alloca i32, align 4
-  // CHECK-NEXT: [[REF:%.*]] = alloca i16*, align 8
-  // CHECK-NEXT: [[S:%.*]] = alloca i16, align 2
+  // X64:[[ARRAY:%.*]] = alloca i8*, align 8
+  // AMD:[[ARRAY0:%.*]] = alloca i8*, align 8, addrspace(5)
+  // AMD-NEXT:   [[ARRAY:%.*]] = addrspacecast i8* addrspace(5)* [[ARRAY0]] to 
i8**
+  // X64-NEXT:   [[N:%.*]] = alloca i32, align 4
+  // AMD:[[N0:%.*]] = alloca i32, align 4, addrspace(5)
+  // AMD-NEXT:   [[N:%.*]] = addrspacecast i32 addrspace(5)* [[N0]] to i32*
+  // X64-NEXT:   [[REF:%.*]] = alloca i16*, align 8
+  // AMD:[[REF0:%.*]] = alloca i16*, align 8, addrspace(5)
+  // AMD-NEXT:   [[REF:%.*]] = addrspacecast i16* addrspace(5)* [[REF0]] to 
i16**
+  // X64-NEXT:   [[S:%.*]] = alloca i16, align 2
+  // AMD:[[S0:%.*]] = alloca i16, align 2, addrspace(5)
+  // AMD-NEXT:   [[S:%.*]] = addrspacecast i16 addrspace(5)* [[S0]] to i16*
   // CHECK-NEXT: store i8* 
   // CHECK-NEXT: store i32
 
@@ -59,6 +68,8 @@
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
+  // AMD: %__end = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -75,13 +86,16 @@
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* 
{{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //CHECK-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
 
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
+  // AMD: %__end = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
@@ -105,7 +119,8 @@
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS:%.*]] = udiv i64 [[VLA_SIZEOF]], 
[[VLA_SIZEOF_DIM2]]
   //CHECK-NEXT: [[VLA_END_INDEX:%.*]] = mul nsw i64 [[VLA_NUM_ELEMENTS]], 
[[VLA_DIM2_PRE]]
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* 
{{%.*}}, i64 [[VLA_END_INDEX]]
-  //CHECK-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //AMD-NEXT: store i32* [