[PATCH] D52133: [analyzer] A testing facility for testing relationships between symbols.

2018-09-14 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs, baloghadamsoftware.
Herald added subscribers: cfe-commits, Szelethus, jfb, mikhail.ramalho.

A test introduced in https://reviews.llvm.org/rC329780 was disabled in 
https://reviews.llvm.org/rC342317 for the reason explained in 
https://reviews.llvm.org/D41938?id=130403#inline-371070 - tests shouldn't test 
dump infrastructure when all they care about is how symbols relate to each 
other.

Add a new feature to ExprInspection: `clang_analyzer_denote()` and 
`clang_analyzer_explain()`. The former adds a notation to a symbol, the latter 
expresses another symbol in terms of previously denoted symbols.

It's currently a bit wonky - doesn't print parentheses and only supports 
denoting atomic symbols. But it's even more readable that way.

I also noticed that an important use case was omitted in these tests. Namely, 
tests for unsigned integer rearrangement are done with signed-type symbols 
stored in unsigned variables and we also need to test unsigned symbols stored 
in unsigned variables (and ideally also unsigned symbols stored in signed 
variables).


Repository:
  rC Clang

https://reviews.llvm.org/D52133

Files:
  docs/analyzer/DebugChecks.rst
  lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- test/Analysis/svalbuilder-rearrange-comparisons.c
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -1,10 +1,8 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-binary-operation-simplification=true -verify -analyzer-config eagerly-assume=false %s
 
-// Temporary xfailing, as debug printing functionality has changed.
-// XFAIL: *
-
-void clang_analyzer_dump(int x);
 void clang_analyzer_eval(int x);
+void clang_analyzer_denote(int x, const char *literal);
+void clang_analyzer_express(int x);
 
 void exit(int);
 
@@ -29,907 +27,951 @@
 
 void compare_different_symbol_equal() {
   int x = f(), y = f();
-  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
-  clang_analyzer_dump(y); // expected-warning{{conj_$9{int}}}
-  clang_analyzer_dump(x == y);
-  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 0}}
+  clang_analyzer_denote(x, "$x");
+  clang_analyzer_denote(y, "$y");
+  clang_analyzer_express(x == y); // expected-warning {{$x - $y == 0}}
 }
 
 void compare_different_symbol_plus_left_int_equal() {
-  int x = f()+1, y = f();
-  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
-  clang_analyzer_dump(y); // expected-warning{{conj_$9{int}}}
-  clang_analyzer_dump(x == y);
-  // expected-warning@-1{{((conj_$9{int}) - (conj_$2{int})) == 1}}
+  int x = f(), y = f();
+  clang_analyzer_denote(x, "$x");
+  clang_analyzer_denote(y, "$y");
+  x += 1;
+  clang_analyzer_express(x == y); // expected-warning {{$y - $x == 1}}
 }
 
 void compare_different_symbol_minus_left_int_equal() {
-  int x = f()-1, y = f();
-  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
-  clang_analyzer_dump(y); // expected-warning{{conj_$9{int}}}
-  clang_analyzer_dump(x == y);
-  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 1}}
+  int x = f(), y = f();
+  clang_analyzer_denote(x, "$x");
+  clang_analyzer_denote(y, "$y");
+  x -= 1;
+  clang_analyzer_express(x == y); // expected-warning {{$x - $y == 1}}
 }
 
 void compare_different_symbol_plus_right_int_equal() {
-  int x = f(), y = f()+2;
-  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
-  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) + 2}}
-  clang_analyzer_dump(x == y);
-  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 2}}
+  int x = f(), y = f();
+  clang_analyzer_denote(x, "$x");
+  clang_analyzer_denote(y, "$y");
+  y += 2;
+  clang_analyzer_express(y); // expected-warning {{$y + 2}}
+  clang_analyzer_express(x == y); // expected-warning {{$x - $y == 2}}
 }
 
 void compare_different_symbol_minus_right_int_equal() {
-  int x = f(), y = f()-2;
-  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
-  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) - 2}}
-  clang_analyzer_dump(x == y);
-  // expected-warning@-1{{((conj_$9{int}) - (conj_$2{int})) == 2}}
+  int x = f(), y = f();
+  clang_analyzer_denote(x, "$x");
+  clang_analyzer_denote(y, "$y");
+  y -= 2;
+  clang_analyzer_express(y); // expected-warning {{$y - 2}}
+  clang_analyzer_express(x == y); // expected-warning {{$y - $x == 2}}
 }
 
 void compare_different_symbol_plus_left_plus_right_int_equal() {
-  int x = f()+2, y = f()+1;
-  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
-  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) + 1}}
-  clang_analyzer_dump(x == y);
-  // expected-warning@-1{{((conj_$9{int}) - 

[PATCH] D52132: [CMake] Use cannonical triples for Fuchsia runtimes

2018-09-14 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added reviewers: mcgrathr, jakehehrlich.
Herald added subscribers: cfe-commits, mgorny.

This ensures that whether the user uses short or cannonical version
of the triple, Clang will still find the runtimes under the cannonical
triple name.


Repository:
  rC Clang

https://reviews.llvm.org/D52132

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

Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -35,31 +35,31 @@
   foreach(target i386;x86_64;armhf;aarch64)
 if(LINUX_${target}_SYSROOT)
   # Set the per-target builtins options.
-  list(APPEND BUILTIN_TARGETS "${target}-linux-gnu")
-  set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-  set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
-  set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "")
+  list(APPEND BUILTIN_TARGETS "${target}-unknown-linux-gnu")
+  set(BUILTINS_${target}-unknown-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+  set(BUILTINS_${target}-unknown-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+  set(BUILTINS_${target}-unknown-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "")
 
   # Set the per-target runtimes options.
-  list(APPEND RUNTIME_TARGETS "${target}-linux-gnu")
-  set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
+  list(APPEND RUNTIME_TARGETS "${target}-unknown-linux-gnu")
+  set(RUNTIMES_${target}-unknown-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+  set(RUNTIMES_${target}-unknown-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE BOOL "")
 endif()
   endforeach()
 endif()
@@ -75,44 +75,44 @@
 
   foreach(target x86_64;aarch64)
 # Set the per-target builtins options.
-list(APPEND BUILTIN_TARGETS "${target}-fuchsia")
-

[PATCH] D52114: [analyzer] Further printing improvements: use declarations

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342316: [analyzer] Further printing improvements: use 
declarations, (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52114?vs=165565=165632#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52114

Files:
  lib/AST/Stmt.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/MemRegion.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp

Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -235,8 +235,7 @@
   const Stmt *S = I.first.getStmt();
   assert(S != nullptr && "Expected non-null Stmt");
 
-  Out << "(LC " << LC->getID() << " <" << (const void *)LC << ">, S "
-  << S->getID(Context) << " <" << (const void *)S << ">) ";
+  Out << "(LC" << LC->getID() << ", S" << S->getID(Context) << ") ";
   S->printPretty(Out, /*Helper=*/nullptr, PP);
   Out << " : " << I.second << NL;
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3108,16 +3108,16 @@
   const Stmt *S = Loc.castAs().getStmt();
   assert(S != nullptr && "Expecting non-null Stmt");
 
-  Out << S->getStmtClassName() << ' '
+  Out << S->getStmtClassName() << " S"
   << S->getID(Context) << " <" << (const void *)S << "> ";
   S->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
  /*Indentation=*/2, /*NewlineSymbol=*/"\\l");
   printLocation(Out, S->getBeginLoc());
 
   if (Loc.getAs())
-Out << "\\lPreStmt\\l;";
+Out << "\\lPreStmt\\l";
   else if (Loc.getAs())
-Out << "\\lPostLoad\\l;";
+Out << "\\lPostLoad\\l";
   else if (Loc.getAs())
 Out << "\\lPostStore\\l";
   else if (Loc.getAs())
@@ -3171,9 +3171,8 @@
 static_cast(State->getStateManager().getOwningEngine())
 ->getGraph();
 
-Out << "StateID: " << State->getID() << " <" << (const void *)State.get()
-<< ">"
-<< " NodeID: " << N->getID() << " <" << (const void *)N << ">\\|";
+Out << "StateID: ST" << State->getID() << ", NodeID: N" << N->getID()
+<< " <" << (const void *)N << ">\\|";
 
 bool SameAsAllPredecessors =
 std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) {
Index: lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -83,7 +83,10 @@
 }
 
 void SymbolConjured::dumpToStream(raw_ostream ) const {
-  os << "conj_$" << getSymbolID() << '{' << T.getAsString() << '}';
+  os << "conj_$" << getSymbolID() << '{' << T.getAsString()
+<< ", LC" << LCtx->getID() << ", S" << S->getID(
+  LCtx->getDecl()->getASTContext()) << ", #" << Count
+<< '}';
 }
 
 void SymbolDerived::dumpToStream(raw_ostream ) const {
Index: lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- lib/StaticAnalyzer/Core/MemRegion.cpp
+++ lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -457,7 +457,7 @@
 }
 
 void AllocaRegion::dumpToStream(raw_ostream ) const {
-  os << "alloca{" << static_cast(Ex) << ',' << Cnt << '}';
+  os << "alloca{S" << Ex->getID(getContext()) << ',' << Cnt << '}';
 }
 
 void FunctionCodeRegion::dumpToStream(raw_ostream ) const {
@@ -481,12 +481,12 @@
 
 void CompoundLiteralRegion::dumpToStream(raw_ostream ) const {
   // FIXME: More elaborate pretty-printing.
-  os << "{ " << static_cast(CL) <<  " }";
+  os << "{ S" << CL->getID(getContext()) <<  " }";
 }
 
 void CXXTempObjectRegion::dumpToStream(raw_ostream ) const {
-  os << "temp_object{" << getValueType().getAsString() << ','
- << static_cast(Ex) << '}';
+  os << "temp_object{" << getValueType().getAsString() << ", "
+ << "S" << Ex->getID(getContext()) << '}';
 }
 
 void CXXBaseObjectRegion::dumpToStream(raw_ostream ) const {
@@ -535,7 +535,7 @@
   if (const IdentifierInfo *ID = VD->getIdentifier())
 os << ID->getName();
   else
-os << "VarRegion{" << static_cast(this) << '}';
+os << "VarRegion{D" << VD->getID() << '}';
 }
 
 LLVM_DUMP_METHOD void RegionRawOffset::dump() const {
Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -307,7 +307,6 @@
   assert(Out && "Wrong allocator used");
   assert(*Out % alignof(Stmt) == 0 && "Wrong alignment information");
   return *Out / alignof(Stmt);
-
 }
 
 CompoundStmt::CompoundStmt(ArrayRef Stmts, SourceLocation LB,

r342317 - [analyzer] Temporary disabling svalbuilder-rearrange-comparisons test

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:35:06 2018
New Revision: 342317

URL: http://llvm.org/viewvc/llvm-project?rev=342317=rev
Log:
[analyzer] Temporary disabling svalbuilder-rearrange-comparisons test

As debug printing has changed, and format was not guaranteed to be
stable.
Artem is currently working on a better solution.

Modified:
cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c

Modified: cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c?rev=342317=342316=342317=diff
==
--- cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c (original)
+++ cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c Fri Sep 14 
19:35:06 2018
@@ -1,5 +1,8 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin 
-analyzer-config aggressive-binary-operation-simplification=true -verify 
-analyzer-config eagerly-assume=false %s
 
+// Temporary xfailing, as debug printing functionality has changed.
+// XFAIL: *
+
 void clang_analyzer_dump(int x);
 void clang_analyzer_eval(int x);
 


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


r342316 - [analyzer] Further printing improvements: use declarations,

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:34:45 2018
New Revision: 342316

URL: http://llvm.org/viewvc/llvm-project?rev=342316=rev
Log:
[analyzer] Further printing improvements: use declarations,

skip pointers whenever redundant, use unique prefixes.

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

Modified:
cfe/trunk/lib/AST/Stmt.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp

Modified: cfe/trunk/lib/AST/Stmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=342316=342315=342316=diff
==
--- cfe/trunk/lib/AST/Stmt.cpp (original)
+++ cfe/trunk/lib/AST/Stmt.cpp Fri Sep 14 19:34:45 2018
@@ -307,7 +307,6 @@ int64_t Stmt::getID(const ASTContext 
   assert(Out && "Wrong allocator used");
   assert(*Out % alignof(Stmt) == 0 && "Wrong alignment information");
   return *Out / alignof(Stmt);
-
 }
 
 CompoundStmt::CompoundStmt(ArrayRef Stmts, SourceLocation LB,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp?rev=342316=342315=342316=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp Fri Sep 14 19:34:45 2018
@@ -235,8 +235,7 @@ void Environment::print(raw_ostream 
   const Stmt *S = I.first.getStmt();
   assert(S != nullptr && "Expected non-null Stmt");
 
-  Out << "(LC " << LC->getID() << " <" << (const void *)LC << ">, S "
-  << S->getID(Context) << " <" << (const void *)S << ">) ";
+  Out << "(LC" << LC->getID() << ", S" << S->getID(Context) << ") ";
   S->printPretty(Out, /*Helper=*/nullptr, PP);
   Out << " : " << I.second << NL;
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342316=342315=342316=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 14 19:34:45 2018
@@ -3108,16 +3108,16 @@ struct DOTGraphTraits : p
   const Stmt *S = Loc.castAs().getStmt();
   assert(S != nullptr && "Expecting non-null Stmt");
 
-  Out << S->getStmtClassName() << ' '
+  Out << S->getStmtClassName() << " S"
   << S->getID(Context) << " <" << (const void *)S << "> ";
   S->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
  /*Indentation=*/2, /*NewlineSymbol=*/"\\l");
   printLocation(Out, S->getBeginLoc());
 
   if (Loc.getAs())
-Out << "\\lPreStmt\\l;";
+Out << "\\lPreStmt\\l";
   else if (Loc.getAs())
-Out << "\\lPostLoad\\l;";
+Out << "\\lPostLoad\\l";
   else if (Loc.getAs())
 Out << "\\lPostStore\\l";
   else if (Loc.getAs())
@@ -3171,9 +3171,8 @@ struct DOTGraphTraits : p
 static_cast(State->getStateManager().getOwningEngine())
 ->getGraph();
 
-Out << "StateID: " << State->getID() << " <" << (const void *)State.get()
-<< ">"
-<< " NodeID: " << N->getID() << " <" << (const void *)N << 
">\\|";
+Out << "StateID: ST" << State->getID() << ", NodeID: N" << N->getID()
+<< " <" << (const void *)N << ">\\|";
 
 bool SameAsAllPredecessors =
 std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) 
{

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=342316=342315=342316=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Fri Sep 14 19:34:45 2018
@@ -457,7 +457,7 @@ void MemRegion::dumpToStream(raw_ostream
 }
 
 void AllocaRegion::dumpToStream(raw_ostream ) const {
-  os << "alloca{" << static_cast(Ex) << ',' << Cnt << '}';
+  os << "alloca{S" << Ex->getID(getContext()) << ',' << Cnt << '}';
 }
 
 void FunctionCodeRegion::dumpToStream(raw_ostream ) const {
@@ -481,12 +481,12 @@ void BlockDataRegion::dumpToStream(raw_o
 
 void CompoundLiteralRegion::dumpToStream(raw_ostream ) const {
   // FIXME: More elaborate pretty-printing.
-  os << "{ " << static_cast(CL) <<  " }";
+  os << "{ S" << CL->getID(getContext()) <<  " }";
 }
 
 void CXXTempObjectRegion::dumpToStream(raw_ostream ) const {
-  os << "temp_object{" << getValueType().getAsString() << ','
- << static_cast(Ex) << '}';
+  os << "temp_object{" << getValueType().getAsString() << ", "
+ << "S" 

[PATCH] D52113: Generate unique identifiers for Decl objects

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342315: Generate unique identifiers for Decl objects 
(authored by george.karpenkov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52113?vs=165564=165631#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52113

Files:
  include/clang/AST/DeclBase.h
  lib/AST/DeclBase.cpp


Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -930,6 +930,13 @@
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
+int64_t Decl::getID() const {
+  Optional Out = getASTContext().getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Decl) == 0 && "Wrong alignment information");
+  return *Out / alignof(Decl);
+}
+
 const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
   QualType Ty;
   if (const auto *D = dyn_cast(this))
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1141,6 +1141,9 @@
 
   void dump(raw_ostream , bool Deserialize = false) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID() const;
+
   /// Looks through the Decl's underlying type to extract a FunctionType
   /// when possible. Will return null if the type underlying the Decl does not
   /// have a FunctionType.


Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -930,6 +930,13 @@
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
+int64_t Decl::getID() const {
+  Optional Out = getASTContext().getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Decl) == 0 && "Wrong alignment information");
+  return *Out / alignof(Decl);
+}
+
 const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
   QualType Ty;
   if (const auto *D = dyn_cast(this))
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1141,6 +1141,9 @@
 
   void dump(raw_ostream , bool Deserialize = false) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID() const;
+
   /// Looks through the Decl's underlying type to extract a FunctionType
   /// when possible. Will return null if the type underlying the Decl does not
   /// have a FunctionType.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r342315 - Generate unique identifiers for Decl objects

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:03:58 2018
New Revision: 342315

URL: http://llvm.org/viewvc/llvm-project?rev=342315=rev
Log:
Generate unique identifiers for Decl objects

The generated identifier is stable across multiple runs,
and can be a great visualization or debugging aide.

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

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

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=342315=342314=342315=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Fri Sep 14 19:03:58 2018
@@ -1141,6 +1141,9 @@ public:
 
   void dump(raw_ostream , bool Deserialize = false) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID() const;
+
   /// Looks through the Decl's underlying type to extract a FunctionType
   /// when possible. Will return null if the type underlying the Decl does not
   /// have a FunctionType.

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=342315=342314=342315=diff
==
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri Sep 14 19:03:58 2018
@@ -930,6 +930,13 @@ bool Decl::AccessDeclContextSanity() con
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
+int64_t Decl::getID() const {
+  Optional Out = getASTContext().getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Decl) == 0 && "Wrong alignment information");
+  return *Out / alignof(Decl);
+}
+
 const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
   QualType Ty;
   if (const auto *D = dyn_cast(this))


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


r342314 - [analyzer] Generate and use stable identifiers for LocationContext

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:03:36 2018
New Revision: 342314

URL: http://llvm.org/viewvc/llvm-project?rev=342314=rev
Log:
[analyzer] Generate and use stable identifiers for LocationContext

Those are not created in the allocator.
Since they are created fairly rarely, a counter overhead should not
affect the memory consumption.

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

Modified:
cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp

Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=342314=342313=342314=diff
==
--- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original)
+++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Fri Sep 14 19:03:36 
2018
@@ -227,17 +227,23 @@ private:
   AnalysisDeclContext *Ctx;
 
   const LocationContext *Parent;
+  int64_t ID;
 
 protected:
   LocationContext(ContextKind k, AnalysisDeclContext *ctx,
-  const LocationContext *parent)
-  : Kind(k), Ctx(ctx), Parent(parent) {}
+  const LocationContext *parent,
+  int64_t ID)
+  : Kind(k), Ctx(ctx), Parent(parent), ID(ID) {}
 
 public:
   virtual ~LocationContext();
 
   ContextKind getKind() const { return Kind; }
 
+  int64_t getID() const {
+return ID;
+  }
+
   AnalysisDeclContext *getAnalysisDeclContext() const { return Ctx; }
 
   const LocationContext *getParent() const { return Parent; }
@@ -297,8 +303,9 @@ class StackFrameContext : public Locatio
 
   StackFrameContext(AnalysisDeclContext *ctx, const LocationContext *parent,
 const Stmt *s, const CFGBlock *blk,
-unsigned idx)
-  : LocationContext(StackFrame, ctx, parent), CallSite(s),
+unsigned idx,
+int64_t ID)
+  : LocationContext(StackFrame, ctx, parent, ID), CallSite(s),
 Block(blk), Index(idx) {}
 
 public:
@@ -334,8 +341,8 @@ class ScopeContext : public LocationCont
   const Stmt *Enter;
 
   ScopeContext(AnalysisDeclContext *ctx, const LocationContext *parent,
-   const Stmt *s)
-  : LocationContext(Scope, ctx, parent), Enter(s) {}
+   const Stmt *s, int64_t ID)
+  : LocationContext(Scope, ctx, parent, ID), Enter(s) {}
 
 public:
   ~ScopeContext() override = default;
@@ -361,9 +368,10 @@ class BlockInvocationContext : public Lo
   const void *ContextData;
 
   BlockInvocationContext(AnalysisDeclContext *ctx,
- const LocationContext *parent,
- const BlockDecl *bd, const void *contextData)
-  : LocationContext(Block, ctx, parent), BD(bd), ContextData(contextData) 
{}
+ const LocationContext *parent, const BlockDecl *bd,
+ const void *contextData, int64_t ID)
+  : LocationContext(Block, ctx, parent, ID), BD(bd),
+ContextData(contextData) {}
 
 public:
   ~BlockInvocationContext() override = default;
@@ -389,6 +397,9 @@ public:
 class LocationContextManager {
   llvm::FoldingSet Contexts;
 
+  /// ID used for generating a new location context.
+  int64_t NewID = 0;
+
 public:
   ~LocationContextManager();
 

Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=342314=342313=342314=diff
==
--- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
+++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Fri Sep 14 19:03:36 2018
@@ -385,7 +385,7 @@ LocationContextManager::getLocationConte
   LOC *L = cast_or_null(Contexts.FindNodeOrInsertPos(ID, InsertPos));
 
   if (!L) {
-L = new LOC(ctx, parent, d);
+L = new LOC(ctx, parent, d, ++NewID);
 Contexts.InsertNode(L, InsertPos);
   }
   return L;
@@ -402,7 +402,7 @@ LocationContextManager::getStackFrame(An
   auto *L =
cast_or_null(Contexts.FindNodeOrInsertPos(ID, 
InsertPos));
   if (!L) {
-L = new StackFrameContext(ctx, parent, s, blk, idx);
+L = new StackFrameContext(ctx, parent, s, blk, idx, ++NewID);
 Contexts.InsertNode(L, InsertPos);
   }
   return L;
@@ -427,7 +427,7 @@ LocationContextManager::getBlockInvocati
 cast_or_null(Contexts.FindNodeOrInsertPos(ID,
 
InsertPos));
   if (!L) {
-L = new BlockInvocationContext(ctx, parent, BD, ContextData);
+L = new BlockInvocationContext(ctx, parent, BD, ContextData, ++NewID);
 Contexts.InsertNode(L, InsertPos);
   }
   return L;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
URL: 

r342313 - [analyzer] Dump reproducible identifiers for statements in exploded graph in store

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:03:17 2018
New Revision: 342313

URL: http://llvm.org/viewvc/llvm-project?rev=342313=rev
Log:
[analyzer] Dump reproducible identifiers for statements in exploded graph in 
store

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h?rev=342313=342312=342313=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h Fri 
Sep 14 19:03:17 2018
@@ -93,6 +93,7 @@ public:
   }
 
   void print(raw_ostream , const char *NL, const char *Sep,
+ const ASTContext ,
  const LocationContext *WithLC = nullptr) const;
 };
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp?rev=342313=342312=342313=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp Fri Sep 14 19:03:17 2018
@@ -202,7 +202,9 @@ EnvironmentManager::removeDeadBindings(E
 }
 
 void Environment::print(raw_ostream , const char *NL,
-const char *Sep, const LocationContext *WithLC) const {
+const char *Sep,
+const ASTContext ,
+const LocationContext *WithLC) const {
   if (ExprBindings.isEmpty())
 return;
 
@@ -222,8 +224,7 @@ void Environment::print(raw_ostream 
 
   assert(WithLC);
 
-  LangOptions LO; // FIXME.
-  PrintingPolicy PP(LO);
+  PrintingPolicy PP = Context.getPrintingPolicy();
 
   Out << NL << NL << "Expressions by stack frame:" << NL;
   WithLC->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) {
@@ -234,8 +235,9 @@ void Environment::print(raw_ostream 
   const Stmt *S = I.first.getStmt();
   assert(S != nullptr && "Expected non-null Stmt");
 
-  Out << "(" << (const void *)LC << ',' << (const void *)S << ") ";
-  S->printPretty(Out, nullptr, PP);
+  Out << "(LC" << (const void *)LC << ", S" << S->getID(Context) << " <"
+  << (const void *)S << "> ) ";
+  S->printPretty(Out, /*Helper=*/nullptr, PP);
   Out << " : " << I.second << NL;
 }
   });

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342313=342312=342313=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 14 19:03:17 2018
@@ -3109,7 +3109,7 @@ struct DOTGraphTraits : p
   assert(S != nullptr && "Expecting non-null Stmt");
 
   Out << S->getStmtClassName() << ' '
-  << S->getID(Context) << " (" << (const void *)S << ") ";
+  << S->getID(Context) << " <" << (const void *)S << "> ";
   S->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
  /*Indentation=*/2, /*NewlineSymbol=*/"\\l");
   printLocation(Out, S->getBeginLoc());
@@ -3171,9 +3171,9 @@ struct DOTGraphTraits : p
 static_cast(State->getStateManager().getOwningEngine())
 ->getGraph();
 
-Out << "StateID: " << State->getID() << " (" << (const void *)State.get()
-<< ")"
-<< " NodeID: " << N->getID() << " (" << (const void *)N << 
")\\|";
+Out << "StateID: " << State->getID() << " <" << (const void *)State.get()
+<< ">"
+<< " NodeID: " << N->getID() << " <" << (const void *)N << 
">\\|";
 
 bool SameAsAllPredecessors =
 std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) 
{

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp?rev=342313=342312=342313=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp Fri Sep 14 19:03:17 2018
@@ -456,14 +456,16 @@ void ProgramState::setStore(const StoreR
 //  State pretty-printing.
 
//===--===//
 
-void ProgramState::print(raw_ostream , const char *NL, const char *Sep,
+void 

r342312 - [analyzer] Use correct end-of-line character when printing statements for exploded graph

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:02:56 2018
New Revision: 342312

URL: http://llvm.org/viewvc/llvm-project?rev=342312=rev
Log:
[analyzer] Use correct end-of-line character when printing statements for 
exploded graph

Prevents bad centering.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342312=342311=342312=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 14 19:02:56 2018
@@ -3071,12 +3071,14 @@ struct DOTGraphTraits : p
 if (const auto *C = dyn_cast(Label)) {
   Out << "\\lcase ";
   if (C->getLHS())
-C->getLHS()->printPretty(Out, nullptr,
- Context.getPrintingPolicy());
+C->getLHS()->printPretty(
+Out, nullptr, Context.getPrintingPolicy(),
+/*Indentation=*/0, /*NewlineSymbol=*/"\\l");
 
   if (const Stmt *RHS = C->getRHS()) {
 Out << " .. ";
-RHS->printPretty(Out, nullptr, Context.getPrintingPolicy());
+RHS->printPretty(Out, nullptr, Context.getPrintingPolicy(),
+ /*Indetation=*/0, /*NewlineSymbol=*/"\\l");
   }
 
   Out << ":";
@@ -3108,7 +3110,8 @@ struct DOTGraphTraits : p
 
   Out << S->getStmtClassName() << ' '
   << S->getID(Context) << " (" << (const void *)S << ") ";
-  S->printPretty(Out, nullptr, Context.getPrintingPolicy());
+  S->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
+ /*Indentation=*/2, /*NewlineSymbol=*/"\\l");
   printLocation(Out, S->getBeginLoc());
 
   if (Loc.getAs())


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


[PATCH] D51824: StmtPrinter: allow customizing the end-of-line character

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342311: StmtPrinter: allow customizing the end-of-line 
character (authored by george.karpenkov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51824?vs=164549=165627#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51824

Files:
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/lib/AST/DeclPrinter.cpp
  cfe/trunk/lib/AST/StmtPrinter.cpp

Index: cfe/trunk/lib/AST/DeclPrinter.cpp
===
--- cfe/trunk/lib/AST/DeclPrinter.cpp
+++ cfe/trunk/lib/AST/DeclPrinter.cpp
@@ -544,7 +544,7 @@
   prettyPrintAttributes(D);
   if (Expr *Init = D->getInitExpr()) {
 Out << " = ";
-Init->printPretty(Out, nullptr, Policy, Indentation, );
+Init->printPretty(Out, nullptr, Policy, Indentation, "\n", );
   }
 }
 
Index: cfe/trunk/lib/AST/StmtPrinter.cpp
===
--- cfe/trunk/lib/AST/StmtPrinter.cpp
+++ cfe/trunk/lib/AST/StmtPrinter.cpp
@@ -69,14 +69,16 @@
 unsigned IndentLevel;
 PrinterHelper* Helper;
 PrintingPolicy Policy;
+std::string NL;
 const ASTContext *Context;
 
   public:
 StmtPrinter(raw_ostream , PrinterHelper *helper,
 const PrintingPolicy , unsigned Indentation = 0,
+StringRef NL = "\n",
 const ASTContext *Context = nullptr)
 : OS(os), IndentLevel(Indentation), Helper(helper), Policy(Policy),
-  Context(Context) {}
+  NL(NL), Context(Context) {}
 
 void PrintStmt(Stmt *S) {
   PrintStmt(S, Policy.Indentation);
@@ -88,11 +90,11 @@
 // If this is an expr used in a stmt context, indent and newline it.
 Indent();
 Visit(S);
-OS << ";\n";
+OS << ";" << NL;
   } else if (S) {
 Visit(S);
   } else {
-Indent() << "<<>>\n";
+Indent() << "<<>>" << NL;
   }
   IndentLevel -= SubIndent;
 }
@@ -128,7 +130,7 @@
 }
 
 void VisitStmt(Stmt *Node) LLVM_ATTRIBUTE_UNUSED {
-  Indent() << "<>\n";
+  Indent() << "<>" << NL;
 }
 
 void VisitExpr(Expr *Node) LLVM_ATTRIBUTE_UNUSED {
@@ -152,7 +154,7 @@
 /// PrintRawCompoundStmt - Print a compound stmt without indenting the {, and
 /// with no newline after the }.
 void StmtPrinter::PrintRawCompoundStmt(CompoundStmt *Node) {
-  OS << "{\n";
+  OS << "{" << NL;
   for (auto *I : Node->body())
 PrintStmt(I);
 
@@ -169,19 +171,19 @@
 }
 
 void StmtPrinter::VisitNullStmt(NullStmt *Node) {
-  Indent() << ";\n";
+  Indent() << ";" << NL;
 }
 
 void StmtPrinter::VisitDeclStmt(DeclStmt *Node) {
   Indent();
   PrintRawDeclStmt(Node);
-  OS << ";\n";
+  OS << ";" << NL;
 }
 
 void StmtPrinter::VisitCompoundStmt(CompoundStmt *Node) {
   Indent();
   PrintRawCompoundStmt(Node);
-  OS << "\n";
+  OS << "" << NL;
 }
 
 void StmtPrinter::VisitCaseStmt(CaseStmt *Node) {
@@ -191,18 +193,18 @@
 OS << " ... ";
 PrintExpr(Node->getRHS());
   }
-  OS << ":\n";
+  OS << ":" << NL;
 
   PrintStmt(Node->getSubStmt(), 0);
 }
 
 void StmtPrinter::VisitDefaultStmt(DefaultStmt *Node) {
-  Indent(-1) << "default:\n";
+  Indent(-1) << "default:" << NL;
   PrintStmt(Node->getSubStmt(), 0);
 }
 
 void StmtPrinter::VisitLabelStmt(LabelStmt *Node) {
-  Indent(-1) << Node->getName() << ":\n";
+  Indent(-1) << Node->getName() << ":" << NL;
   PrintStmt(Node->getSubStmt(), 0);
 }
 
@@ -225,9 +227,9 @@
   if (auto *CS = dyn_cast(If->getThen())) {
 OS << ' ';
 PrintRawCompoundStmt(CS);
-OS << (If->getElse() ? ' ' : '\n');
+OS << (If->getElse() ? " " : NL);
   } else {
-OS << '\n';
+OS << NL;
 PrintStmt(If->getThen());
 if (If->getElse()) Indent();
   }
@@ -238,12 +240,12 @@
 if (auto *CS = dyn_cast(Else)) {
   OS << ' ';
   PrintRawCompoundStmt(CS);
-  OS << '\n';
+  OS << NL;
 } else if (auto *ElseIf = dyn_cast(Else)) {
   OS << ' ';
   PrintRawIfStmt(ElseIf);
 } else {
-  OS << '\n';
+  OS << NL;
   PrintStmt(If->getElse());
 }
   }
@@ -266,9 +268,9 @@
   if (auto *CS = dyn_cast(Node->getBody())) {
 OS << " ";
 PrintRawCompoundStmt(CS);
-OS << "\n";
+OS << NL;
   } else {
-OS << "\n";
+OS << NL;
 PrintStmt(Node->getBody());
   }
 }
@@ -279,7 +281,7 @@
 PrintRawDeclStmt(DS);
   else
 PrintExpr(Node->getCond());
-  OS << ")\n";
+  OS << ")" << NL;
   PrintStmt(Node->getBody());
 }
 
@@ -289,14 +291,14 @@
 PrintRawCompoundStmt(CS);
 OS << " ";
   } else {
-OS << "\n";
+OS << NL;
 PrintStmt(Node->getBody());
 Indent();
   }
 
   OS << "while (";
   PrintExpr(Node->getCond());
-  OS << ");\n";
+  OS << ");" << NL;
 }
 
 void StmtPrinter::VisitForStmt(ForStmt *Node) {
@@ -321,9 +323,9 @@
 
   if (auto *CS = dyn_cast(Node->getBody())) {
 PrintRawCompoundStmt(CS);
-OS << "\n";

r342311 - StmtPrinter: allow customizing the end-of-line character

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:02:31 2018
New Revision: 342311

URL: http://llvm.org/viewvc/llvm-project?rev=342311=rev
Log:
StmtPrinter: allow customizing the end-of-line character

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

Modified:
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/DeclPrinter.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=342311=342310=342311=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Fri Sep 14 19:02:31 2018
@@ -424,6 +424,7 @@ public:
   void dumpPretty(const ASTContext ) const;
   void printPretty(raw_ostream , PrinterHelper *Helper,
const PrintingPolicy , unsigned Indentation = 0,
+   StringRef NewlineSymbol = "\n",
const ASTContext *Context = nullptr) const;
 
   /// viewAST - Visualize an AST rooted at this Stmt* using GraphViz.  Only

Modified: cfe/trunk/lib/AST/DeclPrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclPrinter.cpp?rev=342311=342310=342311=diff
==
--- cfe/trunk/lib/AST/DeclPrinter.cpp (original)
+++ cfe/trunk/lib/AST/DeclPrinter.cpp Fri Sep 14 19:02:31 2018
@@ -544,7 +544,7 @@ void DeclPrinter::VisitEnumConstantDecl(
   prettyPrintAttributes(D);
   if (Expr *Init = D->getInitExpr()) {
 Out << " = ";
-Init->printPretty(Out, nullptr, Policy, Indentation, );
+Init->printPretty(Out, nullptr, Policy, Indentation, "\n", );
   }
 }
 

Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=342311=342310=342311=diff
==
--- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
+++ cfe/trunk/lib/AST/StmtPrinter.cpp Fri Sep 14 19:02:31 2018
@@ -69,14 +69,16 @@ namespace {
 unsigned IndentLevel;
 PrinterHelper* Helper;
 PrintingPolicy Policy;
+std::string NL;
 const ASTContext *Context;
 
   public:
 StmtPrinter(raw_ostream , PrinterHelper *helper,
 const PrintingPolicy , unsigned Indentation = 0,
+StringRef NL = "\n",
 const ASTContext *Context = nullptr)
 : OS(os), IndentLevel(Indentation), Helper(helper), Policy(Policy),
-  Context(Context) {}
+  NL(NL), Context(Context) {}
 
 void PrintStmt(Stmt *S) {
   PrintStmt(S, Policy.Indentation);
@@ -88,11 +90,11 @@ namespace {
 // If this is an expr used in a stmt context, indent and newline it.
 Indent();
 Visit(S);
-OS << ";\n";
+OS << ";" << NL;
   } else if (S) {
 Visit(S);
   } else {
-Indent() << "<<>>\n";
+Indent() << "<<>>" << NL;
   }
   IndentLevel -= SubIndent;
 }
@@ -128,7 +130,7 @@ namespace {
 }
 
 void VisitStmt(Stmt *Node) LLVM_ATTRIBUTE_UNUSED {
-  Indent() << "<>\n";
+  Indent() << "<>" << NL;
 }
 
 void VisitExpr(Expr *Node) LLVM_ATTRIBUTE_UNUSED {
@@ -152,7 +154,7 @@ namespace {
 /// PrintRawCompoundStmt - Print a compound stmt without indenting the {, and
 /// with no newline after the }.
 void StmtPrinter::PrintRawCompoundStmt(CompoundStmt *Node) {
-  OS << "{\n";
+  OS << "{" << NL;
   for (auto *I : Node->body())
 PrintStmt(I);
 
@@ -169,19 +171,19 @@ void StmtPrinter::PrintRawDeclStmt(const
 }
 
 void StmtPrinter::VisitNullStmt(NullStmt *Node) {
-  Indent() << ";\n";
+  Indent() << ";" << NL;
 }
 
 void StmtPrinter::VisitDeclStmt(DeclStmt *Node) {
   Indent();
   PrintRawDeclStmt(Node);
-  OS << ";\n";
+  OS << ";" << NL;
 }
 
 void StmtPrinter::VisitCompoundStmt(CompoundStmt *Node) {
   Indent();
   PrintRawCompoundStmt(Node);
-  OS << "\n";
+  OS << "" << NL;
 }
 
 void StmtPrinter::VisitCaseStmt(CaseStmt *Node) {
@@ -191,18 +193,18 @@ void StmtPrinter::VisitCaseStmt(CaseStmt
 OS << " ... ";
 PrintExpr(Node->getRHS());
   }
-  OS << ":\n";
+  OS << ":" << NL;
 
   PrintStmt(Node->getSubStmt(), 0);
 }
 
 void StmtPrinter::VisitDefaultStmt(DefaultStmt *Node) {
-  Indent(-1) << "default:\n";
+  Indent(-1) << "default:" << NL;
   PrintStmt(Node->getSubStmt(), 0);
 }
 
 void StmtPrinter::VisitLabelStmt(LabelStmt *Node) {
-  Indent(-1) << Node->getName() << ":\n";
+  Indent(-1) << Node->getName() << ":" << NL;
   PrintStmt(Node->getSubStmt(), 0);
 }
 
@@ -225,9 +227,9 @@ void StmtPrinter::PrintRawIfStmt(IfStmt
   if (auto *CS = dyn_cast(If->getThen())) {
 OS << ' ';
 PrintRawCompoundStmt(CS);
-OS << (If->getElse() ? ' ' : '\n');
+OS << (If->getElse() ? " " : NL);
   } else {
-OS << '\n';
+OS << NL;
 PrintStmt(If->getThen());
 if (If->getElse()) Indent();
   }
@@ -238,12 

r342309 - Support generating unique identifiers for Stmt objects

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:01:47 2018
New Revision: 342309

URL: http://llvm.org/viewvc/llvm-project?rev=342309=rev
Log:
Support generating unique identifiers for Stmt objects

The generated identifiers are stable across multiple runs, and can be a
great debug or visualization aid.

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

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

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=342309=342308=342309=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Fri Sep 14 19:01:47 2018
@@ -413,6 +413,9 @@ public:
   void dump(raw_ostream , SourceManager ) const;
   void dump(raw_ostream ) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID(const ASTContext ) const;
+
   /// dumpColor - same as dump(), but forces color highlighting.
   void dumpColor() const;
 

Modified: cfe/trunk/lib/AST/Stmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=342309=342308=342309=diff
==
--- cfe/trunk/lib/AST/Stmt.cpp (original)
+++ cfe/trunk/lib/AST/Stmt.cpp Fri Sep 14 19:01:47 2018
@@ -302,6 +302,14 @@ SourceLocation Stmt::getEndLoc() const {
   llvm_unreachable("unknown statement kind");
 }
 
+int64_t Stmt::getID(const ASTContext ) const {
+  Optional Out = Context.getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Stmt) == 0 && "Wrong alignment information");
+  return *Out / alignof(Stmt);
+
+}
+
 CompoundStmt::CompoundStmt(ArrayRef Stmts, SourceLocation LB,
SourceLocation RB)
 : Stmt(CompoundStmtClass), LBraceLoc(LB), RBraceLoc(RB) {


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


r342310 - [analyzer] Dump unique identifiers for statements in exploded graph

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:02:09 2018
New Revision: 342310

URL: http://llvm.org/viewvc/llvm-project?rev=342310=rev
Log:
[analyzer] Dump unique identifiers for statements in exploded graph

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342310=342309=342310=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 14 19:02:09 2018
@@ -2974,7 +2974,7 @@ struct DOTGraphTraits : p
   }
 
   static void dumpProgramPoint(ProgramPoint Loc,
-   const PrintingPolicy ,
+   const ASTContext ,
llvm::raw_string_ostream ) {
 switch (Loc.getKind()) {
 case ProgramPoint::BlockEntranceKind:
@@ -3019,9 +3019,7 @@ struct DOTGraphTraits : p
 case ProgramPoint::PreImplicitCallKind: {
   ImplicitCallPoint PC = Loc.castAs();
   Out << "PreCall: ";
-
-  // FIXME: Get proper printing options.
-  PC.getDecl()->print(Out, LangOptions());
+  PC.getDecl()->print(Out, Context.getLangOpts());
   printLocation(Out, PC.getLocation());
   break;
 }
@@ -3029,9 +3027,7 @@ struct DOTGraphTraits : p
 case ProgramPoint::PostImplicitCallKind: {
   ImplicitCallPoint PC = Loc.castAs();
   Out << "PostCall: ";
-
-  // FIXME: Get proper printing options.
-  PC.getDecl()->print(Out, LangOptions());
+  PC.getDecl()->print(Out, Context.getLangOpts());
   printLocation(Out, PC.getLocation());
   break;
 }
@@ -3045,8 +3041,7 @@ struct DOTGraphTraits : p
   else {
 QualType Ty = Init->getTypeSourceInfo()->getType();
 Ty = Ty.getLocalUnqualifiedType();
-LangOptions LO; // FIXME.
-Ty.print(Out, LO);
+Ty.print(Out, Context.getLangOpts());
   }
   break;
 }
@@ -3060,8 +3055,7 @@ struct DOTGraphTraits : p
 SourceLocation SLoc = T->getBeginLoc();
 
 Out << "\\|Terminator: ";
-LangOptions LO; // FIXME.
-E.getSrc()->printTerminator(Out, LO);
+E.getSrc()->printTerminator(Out, Context.getLangOpts());
 
 if (SLoc.isFileID()) {
   Out << "\\lline="
@@ -3076,13 +3070,13 @@ struct DOTGraphTraits : p
   if (Label) {
 if (const auto *C = dyn_cast(Label)) {
   Out << "\\lcase ";
-  LangOptions LO; // FIXME.
   if (C->getLHS())
-C->getLHS()->printPretty(Out, nullptr, PrintingPolicy(LO));
+C->getLHS()->printPretty(Out, nullptr,
+ Context.getPrintingPolicy());
 
   if (const Stmt *RHS = C->getRHS()) {
 Out << " .. ";
-RHS->printPretty(Out, nullptr, PrintingPolicy(LO));
+RHS->printPretty(Out, nullptr, Context.getPrintingPolicy());
   }
 
   Out << ":";
@@ -3112,8 +3106,9 @@ struct DOTGraphTraits : p
   const Stmt *S = Loc.castAs().getStmt();
   assert(S != nullptr && "Expecting non-null Stmt");
 
-  Out << S->getStmtClassName() << ' ' << (const void *)S << ' ';
-  S->printPretty(Out, nullptr, PP);
+  Out << S->getStmtClassName() << ' '
+  << S->getID(Context) << " (" << (const void *)S << ") ";
+  S->printPretty(Out, nullptr, Context.getPrintingPolicy());
   printLocation(Out, S->getBeginLoc());
 
   if (Loc.getAs())
@@ -3149,12 +3144,12 @@ struct DOTGraphTraits : p
 }
 
 ProgramStateRef State = N->getState();
-const auto  = State->getStateManager().getContext().getPrintingPolicy();
+const ASTContext  = State->getStateManager().getContext();
 
 // Dump program point for all the previously skipped nodes.
 const ExplodedNode *OtherNode = FirstHiddenNode;
 while (true) {
-  dumpProgramPoint(OtherNode->getLocation(), PP, Out);
+  dumpProgramPoint(OtherNode->getLocation(), Context, Out);
 
   if (const ProgramPointTag *Tag = OtherNode->getLocation().getTag())
 Out << "\\lTag:" << Tag->getTagDescription();


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


[PATCH] D51822: Support generating unique identifiers for Stmt objects

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342309: Support generating unique identifiers for Stmt 
objects (authored by george.karpenkov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51822?vs=164547=165625#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51822

Files:
  include/clang/AST/Stmt.h
  lib/AST/Stmt.cpp


Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -413,6 +413,9 @@
   void dump(raw_ostream , SourceManager ) const;
   void dump(raw_ostream ) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID(const ASTContext ) const;
+
   /// dumpColor - same as dump(), but forces color highlighting.
   void dumpColor() const;
 
Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -302,6 +302,14 @@
   llvm_unreachable("unknown statement kind");
 }
 
+int64_t Stmt::getID(const ASTContext ) const {
+  Optional Out = Context.getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Stmt) == 0 && "Wrong alignment information");
+  return *Out / alignof(Stmt);
+
+}
+
 CompoundStmt::CompoundStmt(ArrayRef Stmts, SourceLocation LB,
SourceLocation RB)
 : Stmt(CompoundStmtClass), LBraceLoc(LB), RBraceLoc(RB) {


Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -413,6 +413,9 @@
   void dump(raw_ostream , SourceManager ) const;
   void dump(raw_ostream ) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID(const ASTContext ) const;
+
   /// dumpColor - same as dump(), but forces color highlighting.
   void dumpColor() const;
 
Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -302,6 +302,14 @@
   llvm_unreachable("unknown statement kind");
 }
 
+int64_t Stmt::getID(const ASTContext ) const {
+  Optional Out = Context.getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Stmt) == 0 && "Wrong alignment information");
+  return *Out / alignof(Stmt);
+
+}
+
 CompoundStmt::CompoundStmt(ArrayRef Stmts, SourceLocation LB,
SourceLocation RB)
 : Stmt(CompoundStmtClass), LBraceLoc(LB), RBraceLoc(RB) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D51821: [analyzer] Skip printing duplicate nodes, even if nodes have multiple predecessors/successors

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342308: [analyzer] Skip printing duplicate nodes, even if 
nodes have multiple… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51821?vs=165619=165624#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51821

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp


Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3177,7 +3177,12 @@
 << ")"
 << " NodeID: " << N->getID() << " (" << (const void *)N << 
")\\|";
 
-State->printDOT(Out, N->getLocationContext());
+bool SameAsAllPredecessors =
+std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) 
{
+  return P->getState() == State;
+});
+if (!SameAsAllPredecessors)
+  State->printDOT(Out, N->getLocationContext());
 return Out.str();
   }
 };


Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3177,7 +3177,12 @@
 << ")"
 << " NodeID: " << N->getID() << " (" << (const void *)N << ")\\|";
 
-State->printDOT(Out, N->getLocationContext());
+bool SameAsAllPredecessors =
+std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) {
+  return P->getState() == State;
+});
+if (!SameAsAllPredecessors)
+  State->printDOT(Out, N->getLocationContext());
 return Out.str();
   }
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r342308 - [analyzer] Skip printing duplicate nodes, even if nodes have multiple predecessors/successors

2018-09-14 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Sep 14 19:01:26 2018
New Revision: 342308

URL: http://llvm.org/viewvc/llvm-project?rev=342308=rev
Log:
[analyzer] Skip printing duplicate nodes, even if nodes have multiple 
predecessors/successors

Still generate a node, but leave the redundant field empty.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342308=342307=342308=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 14 19:01:26 2018
@@ -3177,7 +3177,12 @@ struct DOTGraphTraits : p
 << ")"
 << " NodeID: " << N->getID() << " (" << (const void *)N << 
")\\|";
 
-State->printDOT(Out, N->getLocationContext());
+bool SameAsAllPredecessors =
+std::all_of(N->pred_begin(), N->pred_end(), [&](const ExplodedNode *P) 
{
+  return P->getState() == State;
+});
+if (!SameAsAllPredecessors)
+  State->printDOT(Out, N->getLocationContext());
 return Out.str();
   }
 };


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


r342307 - [modules] Don't bother creating a global module fragment when building a

2018-09-14 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri Sep 14 18:59:39 2018
New Revision: 342307

URL: http://llvm.org/viewvc/llvm-project?rev=342307=rev
Log:
[modules] Don't bother creating a global module fragment when building a
header module.

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

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=342307=342306=342307=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Fri Sep 14 18:59:39 2018
@@ -827,7 +827,9 @@ void Sema::emitAndClearUnusedLocalTypede
 /// is parsed. Note that the ASTContext may have already injected some
 /// declarations.
 void Sema::ActOnStartOfTranslationUnit() {
-  if (getLangOpts().ModulesTS) {
+  if (getLangOpts().ModulesTS &&
+  (getLangOpts().getCompilingModule() == LangOptions::CMK_ModuleInterface 
||
+   getLangOpts().getCompilingModule() == LangOptions::CMK_None)) {
 SourceLocation StartOfTU =
 SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
 


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


[PATCH] D52089: [clangd] Get rid of AST matchers in SymbolCollector. NFC

2018-09-14 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment.

why?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52089



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


r342305 - [modules] Driver support for precompiling a collection of files as a single

2018-09-14 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri Sep 14 18:21:16 2018
New Revision: 342305

URL: http://llvm.org/viewvc/llvm-project?rev=342305=rev
Log:
[modules] Driver support for precompiling a collection of files as a single
action.

Added:
cfe/trunk/test/Driver/header-module.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/Action.h
cfe/trunk/lib/Driver/Action.cpp
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/Types.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=342305=342304=342305=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Sep 14 18:21:16 
2018
@@ -296,6 +296,9 @@ def warn_drv_vectorize_needs_hvx : Warni
   "auto-vectorization requires HVX, use -mhvx to enable it">,
   InGroup;
 
+def err_drv_module_header_wrong_kind : Error<
+  "header file '%0' input type '%1' does not match type of prior input "
+  "in module compilation; use '-x %2' to override">;
 def err_drv_modules_validate_once_requires_timestamp : Error<
   "option '-fmodules-validate-once-per-build-session' requires "
   "'-fbuild-session-timestamp=' or 
'-fbuild-session-file='">;

Modified: cfe/trunk/include/clang/Driver/Action.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Action.h?rev=342305=342304=342305=diff
==
--- cfe/trunk/include/clang/Driver/Action.h (original)
+++ cfe/trunk/include/clang/Driver/Action.h Fri Sep 14 18:21:16 2018
@@ -59,6 +59,7 @@ public:
 OffloadClass,
 PreprocessJobClass,
 PrecompileJobClass,
+HeaderModulePrecompileJobClass,
 AnalyzeJobClass,
 MigrateJobClass,
 CompileJobClass,
@@ -398,12 +399,36 @@ public:
 class PrecompileJobAction : public JobAction {
   void anchor() override;
 
+protected:
+  PrecompileJobAction(ActionClass Kind, Action *Input, types::ID OutputType);
+
 public:
   PrecompileJobAction(Action *Input, types::ID OutputType);
 
   static bool classof(const Action *A) {
-return A->getKind() == PrecompileJobClass;
+return A->getKind() == PrecompileJobClass ||
+   A->getKind() == HeaderModulePrecompileJobClass;
+  }
+};
+
+class HeaderModulePrecompileJobAction : public PrecompileJobAction {
+  void anchor() override;
+
+  const char *ModuleName;
+
+public:
+  HeaderModulePrecompileJobAction(Action *Input, types::ID OutputType,
+  const char *ModuleName);
+
+  static bool classof(const Action *A) {
+return A->getKind() == HeaderModulePrecompileJobClass;
   }
+
+  void addModuleHeaderInput(Action *Input) {
+getInputs().push_back(Input);
+  }
+
+  const char *getModuleName() const { return ModuleName; }
 };
 
 class AnalyzeJobAction : public JobAction {

Modified: cfe/trunk/lib/Driver/Action.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Action.cpp?rev=342305=342304=342305=diff
==
--- cfe/trunk/lib/Driver/Action.cpp (original)
+++ cfe/trunk/lib/Driver/Action.cpp Fri Sep 14 18:21:16 2018
@@ -26,6 +26,7 @@ const char *Action::getClassName(ActionC
 return "offload";
   case PreprocessJobClass: return "preprocessor";
   case PrecompileJobClass: return "precompiler";
+  case HeaderModulePrecompileJobClass: return "header-module-precompiler";
   case AnalyzeJobClass: return "analyzer";
   case MigrateJobClass: return "migrator";
   case CompileJobClass: return "compiler";
@@ -319,6 +320,19 @@ void PrecompileJobAction::anchor() {}
 PrecompileJobAction::PrecompileJobAction(Action *Input, types::ID OutputType)
 : JobAction(PrecompileJobClass, Input, OutputType) {}
 
+PrecompileJobAction::PrecompileJobAction(ActionClass Kind, Action *Input,
+ types::ID OutputType)
+: JobAction(Kind, Input, OutputType) {
+  assert(isa((Action*)this) && "invalid action kind");
+}
+
+void HeaderModulePrecompileJobAction::anchor() {}
+
+HeaderModulePrecompileJobAction::HeaderModulePrecompileJobAction(
+Action *Input, types::ID OutputType, const char *ModuleName)
+: PrecompileJobAction(HeaderModulePrecompileJobClass, Input, OutputType),
+  ModuleName(ModuleName) {}
+
 void AnalyzeJobAction::anchor() {}
 
 AnalyzeJobAction::AnalyzeJobAction(Action *Input, types::ID OutputType)

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=342305=342304=342305=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ 

r342306 - [modules] Support use of -E on modules built from the command line.

2018-09-14 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri Sep 14 18:21:18 2018
New Revision: 342306

URL: http://llvm.org/viewvc/llvm-project?rev=342306=rev
Log:
[modules] Support use of -E on modules built from the command line.

Modified:
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
cfe/trunk/test/Modules/no-module-map.cpp

Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=342306=342305=342306=diff
==
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Fri Sep 14 18:21:18 2018
@@ -587,12 +587,12 @@ bool FrontendAction::BeginSourceFile(Com
   assert(ASTModule && "module file does not define its own module");
   Input = FrontendInputFile(ASTModule->PresumedModuleMapFile, Kind);
 } else {
-  auto  = CI.getSourceManager();
-  FileID ID = SM.getMainFileID();
-  if (auto *File = SM.getFileEntryForID(ID))
+  auto  = AST->getSourceManager();
+  FileID ID = OldSM.getMainFileID();
+  if (auto *File = OldSM.getFileEntryForID(ID))
 Input = FrontendInputFile(File->getName(), Kind);
   else
-Input = FrontendInputFile(SM.getBuffer(ID), Kind);
+Input = FrontendInputFile(OldSM.getBuffer(ID), Kind);
 }
 setCurrentInput(Input, std::move(AST));
   }

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=342306=342305=342306=diff
==
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Fri Sep 14 18:21:18 2018
@@ -825,7 +825,7 @@ void PrintPreprocessedAction::ExecuteAct
   }
 
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(BinaryMode, getCurrentFile());
+  CI.createDefaultOutputFile(BinaryMode, getCurrentFileOrBufferName());
   if (!OS) return;
 
   // If we're preprocessing a module map, start by dumping the contents of the
@@ -837,8 +837,6 @@ void PrintPreprocessedAction::ExecuteAct
   OS->write_escaped(Input.getFile());
   (*OS) << "\"\n";
 }
-// FIXME: Include additional information here so that we don't need the
-// original source files to exist on disk.
 getCurrentModule()->print(*OS);
 (*OS) << "#pragma clang module contents\n";
   }

Modified: cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp?rev=342306=342305=342306=diff
==
--- cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/Rewrite/FrontendActions.cpp Fri Sep 14 18:21:18 2018
@@ -181,7 +181,7 @@ RewriteObjCAction::CreateASTConsumer(Com
 void RewriteMacrosAction::ExecuteAction() {
   CompilerInstance  = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(true, getCurrentFile());
+  CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
   if (!OS) return;
 
   RewriteMacrosInInput(CI.getPreprocessor(), OS.get());
@@ -190,7 +190,7 @@ void RewriteMacrosAction::ExecuteAction(
 void RewriteTestAction::ExecuteAction() {
   CompilerInstance  = getCompilerInstance();
   std::unique_ptr OS =
-  CI.createDefaultOutputFile(false, getCurrentFile());
+  CI.createDefaultOutputFile(false, getCurrentFileOrBufferName());
   if (!OS) return;
 
   DoRewriteTest(CI.getPreprocessor(), OS.get());
@@ -265,7 +265,8 @@ public:
 
 bool RewriteIncludesAction::BeginSourceFileAction(CompilerInstance ) {
   if (!OutputStream) {
-OutputStream = CI.createDefaultOutputFile(true, getCurrentFile());
+OutputStream =
+CI.createDefaultOutputFile(true, getCurrentFileOrBufferName());
 if (!OutputStream)
   return false;
   }

Modified: cfe/trunk/test/Modules/no-module-map.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/no-module-map.cpp?rev=342306=342305=342306=diff
==
--- cfe/trunk/test/Modules/no-module-map.cpp (original)
+++ cfe/trunk/test/Modules/no-module-map.cpp Fri Sep 14 18:21:18 2018
@@ -4,6 +4,11 @@
 // RUN: %clang_cc1 -fmodules-ts -fmodule-file=%t.pcm %s 
-I%S/Inputs/no-module-map -verify -DB
 // RUN: %clang_cc1 -fmodules-ts -fmodule-file=%t.pcm %s 
-I%S/Inputs/no-module-map -verify -DA -DB
 
+// RUN: %clang_cc1 -E %t.pcm -o - | FileCheck %s
+// RUN: %clang_cc1 -frewrite-imports -E %t.pcm -o - | FileCheck %s
+// CHECK: # {{.*}}a.h
+// CHECK: # {{.*}}b.h
+
 #ifdef B
 // expected-no-diagnostics
 #endif


___
cfe-commits mailing list
cfe-commits@lists.llvm.org

r342304 - [modules] Frontend support for building a header module from a list of

2018-09-14 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Fri Sep 14 18:21:15 2018
New Revision: 342304

URL: http://llvm.org/viewvc/llvm-project?rev=342304=rev
Log:
[modules] Frontend support for building a header module from a list of
headaer files.

Added:
cfe/trunk/test/Modules/Inputs/no-module-map/
cfe/trunk/test/Modules/Inputs/no-module-map/a.h
cfe/trunk/test/Modules/Inputs/no-module-map/b.h
cfe/trunk/test/Modules/no-module-map.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticIDs.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/LangOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/FrontendAction.h
cfe/trunk/include/clang/Frontend/FrontendActions.h
cfe/trunk/include/clang/Frontend/FrontendOptions.h
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=342304=342303=342304=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Fri Sep 14 
18:21:15 2018
@@ -187,6 +187,8 @@ def err_module_build_requires_fmodules :
   "module compilation requires '-fmodules'">;
 def err_module_interface_requires_modules_ts : Error<
   "module interface compilation requires '-fmodules-ts'">;
+def err_header_module_requires_modules : Error<
+  "header module compilation requires '-fmodules' or '-fmodules-ts'">;
 def warn_module_config_mismatch : Warning<
   "module file %0 cannot be loaded due to a configuration mismatch with the 
current "
   "compilation">, InGroup>, 
DefaultError;
@@ -224,6 +226,10 @@ def remark_module_build_done : Remark<"f
 def err_modules_embed_file_not_found :
   Error<"file '%0' specified by '-fmodules-embed-file=' not found">,
   DefaultFatal;
+def err_module_header_file_not_found :
+  Error<"module header file '%0' not found">, DefaultFatal;
+def err_module_header_file_invalid :
+  Error<"unexpected module header file input '%0'">, DefaultFatal;
 
 def err_test_module_file_extension_version : Error<
   "test module file extension '%0' has different version (%1.%2) than expected 
"

Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=342304=342303=342304=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Fri Sep 14 18:21:15 2018
@@ -30,7 +30,7 @@ namespace clang {
 enum {
   DIAG_SIZE_COMMON=  300,
   DIAG_SIZE_DRIVER=  200,
-  DIAG_SIZE_FRONTEND  =  100,
+  DIAG_SIZE_FRONTEND  =  150,
   DIAG_SIZE_SERIALIZATION =  120,
   DIAG_SIZE_LEX   =  400,
   DIAG_SIZE_PARSE =  500,

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=342304=342303=342304=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 14 18:21:15 
2018
@@ -9090,6 +9090,8 @@ def err_invalid_type_for_program_scope_v
 let CategoryName = "Modules Issue" in {
 def err_module_decl_in_module_map_module : Error<
   "'module' declaration found while building module from module map">;
+def err_module_decl_in_header_module : Error<
+  "'module' declaration found while building header module">;
 def err_module_interface_implementation_mismatch : Error<
   "missing 'export' specifier in module declaration while "
   "building module interface">;

Modified: cfe/trunk/include/clang/Basic/LangOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.h?rev=342304=342303=342304=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.h (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.h Fri Sep 14 18:21:15 2018
@@ -73,8 +73,11 @@ public:
 /// Compiling a module from a module map.
 CMK_ModuleMap,
 
+/// Compiling a module from a list of header files.
+CMK_HeaderModule,
+
 /// Compiling a C++ modules TS module interface unit.
-

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Looks good other than the warning, which I don't yet understand.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4683-4686
+  "Member '%0' marked with 'exclude_from_explicit_instantiation' attribute is "
+  "not defined but an explicit template instantiation declaration exists. "
+  "Reliance on this member being defined by an explicit template instantiation 
"
+  "will lead to link errors.">;

Diagnostics should start with a lowercase letter and not end with a period.

That said, I'm not sure I see why this diagnostic is correct / useful. If the 
entity is never used, then there's no link error. And if it is ever used, then 
you should get an implicit instantiation like normal, and we already have a 
diagnostic for the case where an entity is implicitly instantiated and no 
definition is available.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2581-2582
+if (Function->hasAttr()) {
+  if (TSK == TSK_ExplicitInstantiationDeclaration &&
+  !Pattern->isDefined()) {
+Diag(Function->getLocation(),

Nit: we prefer to left-align continuation lines (clang-format will do that for 
you).


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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


[PATCH] D52113: Generate unique identifiers for Decl objects

2018-09-14 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

We're getting pretty good at it ^^


https://reviews.llvm.org/D52113



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added inline comments.



Comment at: clang/lib/Frontend/DependencyFile.cpp:340
+return;
+  StringRef Filename = File->getName();
+  if (!FileMatchesDepCriteria(Filename.data(), FileType))

vsapsai wrote:
> rsmith wrote:
> > Should we really be using the (arbitrary) name of the file from the 
> > `FileEntry` rather than the name as written? It looks like the 
> > corresponding code for `InclusionDirective` uses the name as written 
> > instead.
> `InclusionDirective` covers only cases when file is not found. Existing files 
> are handled in `FileChanged`. Advantage of using `File->getName()` is that 
> for input with absolute paths file name will be absolute too.
Ah right, that makes sense.


https://reviews.llvm.org/D30882



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments.



Comment at: clang/lib/Frontend/DependencyFile.cpp:300-304
   StringRef Filename = FE->getName();
   if (!FileMatchesDepCriteria(Filename.data(), FileType))
 return;
 
   AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));

This was the role model.



Comment at: clang/lib/Frontend/DependencyFile.cpp:340
+return;
+  StringRef Filename = File->getName();
+  if (!FileMatchesDepCriteria(Filename.data(), FileType))

rsmith wrote:
> Should we really be using the (arbitrary) name of the file from the 
> `FileEntry` rather than the name as written? It looks like the corresponding 
> code for `InclusionDirective` uses the name as written instead.
`InclusionDirective` covers only cases when file is not found. Existing files 
are handled in `FileChanged`. Advantage of using `File->getName()` is that for 
input with absolute paths file name will be absolute too.


https://reviews.llvm.org/D30882



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


[PATCH] D51910: [Modules] Add platform feature to requires clause

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: docs/Modules.rst:476-477
+
+*platform-environment*
+  A platform-environment variant (e.g. ``linux-gnueabi``, ``windows-msvc``) is 
available.
 

What is the reason to allow these to be combined into a single feature name 
rather than treating them as two distinct features? (Eg, `requires linux, 
gnueabi` rather than `requires linux-gnueabi`)?


https://reviews.llvm.org/D51910



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


[PATCH] D51910: [Modules] Add platform feature to requires clause

2018-09-14 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments.



Comment at: lib/Basic/Module.cpp:101
+  // variant (2), the simulator is hardcoded as part of the platform name. Both
+  // forms above should match "iossimulator" and "ios-simulator" features.
+  if (Target.getTriple().isOSDarwin() && PlatformEnv.endswith("simulator"))

Ah.. it looks like you are fully aware of this :-)


https://reviews.llvm.org/D51910



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Frontend/DependencyFile.cpp:340
+return;
+  StringRef Filename = File->getName();
+  if (!FileMatchesDepCriteria(Filename.data(), FileType))

Should we really be using the (arbitrary) name of the file from the `FileEntry` 
rather than the name as written? It looks like the corresponding code for 
`InclusionDirective` uses the name as written instead.


https://reviews.llvm.org/D30882



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


[PATCH] D51910: [Modules] Add platform feature to requires clause

2018-09-14 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments.



Comment at: docs/Modules.rst:470
+*platform variant*
+  A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, 
``win32``, ``linux``, etc) is available.
 

aprantl wrote:
> Does this work with platforms+environment combinations, such as the ios 
> simulator on Darwin?
Is `iossimulator` a thing? The triple is called 
`x86_64-apple-ios-11.2.3-simulator` and the SDK calls itself `iphonesimulator`, 
so I wonder if this shouldn't be called `ios-simulator` or `iphonesimulator` 
instead.


https://reviews.llvm.org/D51910



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/Frontend/DependencyFile.cpp:325
+void DFGImpl::HasInclude(SourceLocation Loc, const FileEntry *File) {
+  if (!File)
+return;

vsapsai wrote:
> rsmith wrote:
> > Have you thought about whether we should add a dependency even for a 
> > missing file under `-MG` (`AddMissingHeaderDeps`) mode? I think it's 
> > probably better to not do so (ie, the behavior in this patch), but it seems 
> > worth considering.
> Do you know how Make uses these missing files? Or maybe where I can find 
> more. The only documentation I found says
> 
> > This feature is used in automatic updating of makefiles.
> 
> Which is not particularly illuminating.
> 
> Currently I prefer not to include not found `__has_include` files because 
> they aren't really missing, it's OK if they aren't there and nothing has to 
> be done to fix that. But I'd like to confirm if my understanding aligns with 
> Make behaviour.
I believe the setup is this:

 * your project uses generated headers
 * in order to do the first compilation, you need to know which source files 
might include those generated headers

So you use `-M -MG` to build your make file dependencies as a separate step 
before your first "real" compilation, and the `-MG` flag causes dependencies on 
the not-yet-existing generated files to be emitted by the compiler.

Given that the purpose of `-MG` (as I understand it) is to cope with generated 
files, and that using `__has_include` to find a generated file would be 
peculiar (it's much more likely that the named file simply doesn't exist and 
shouldn't have a dependency generated for it), I think it's better to omit 
dependencies for nonexistent files for `__has_include` even under `-MG`.


https://reviews.llvm.org/D30882



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


[PATCH] D51910: [Modules] Add platform feature to requires clause

2018-09-14 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno updated this revision to Diff 165612.
bruno added a comment.

Addressed Adrian's review. Added support to consider the environment and well 
the combination platform-environment. @aprantl also handled multiple variants 
of simulator combinations.


https://reviews.llvm.org/D51910

Files:
  docs/Modules.rst
  lib/Basic/Module.cpp
  lib/Lex/ModuleMap.cpp
  test/Modules/target-platform-features.m

Index: test/Modules/target-platform-features.m
===
--- /dev/null
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,83 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/InputsA
+
+// RUN: echo "module RequiresMacOS {"   >> %t/InputsA/module.map
+// RUN: echo "  requires macos" >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+// RUN: echo "module RequiresNotiOS {"  >> %t/InputsA/module.map
+// RUN: echo "  requires !ios"  >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+// RUN: echo "module RequiresMain {">> %t/InputsA/module.map
+// RUN: echo "  module SubRequiresNotiOS {" >> %t/InputsA/module.map
+// RUN: echo "requires !ios">> %t/InputsA/module.map
+// RUN: echo "  }"  >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+
+// RUN: %clang_cc1 -triple=x86_64-apple-macosx10.6 -DENABLE_DARWIN -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsA -verify %s 
+// expected-no-diagnostics
+
+// RUN: not %clang_cc1 -triple=arm64-apple-ios -DENABLE_DARWIN -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsA %s  2> %t/notios
+// RUN: FileCheck %s -check-prefix=CHECK-IOS < %t/notios
+#ifdef ENABLE_DARWIN
+// CHECK-IOS: module 'RequiresMacOS' requires feature 'macos'
+@import RequiresMacOS;
+// CHECK-IOS: module 'RequiresNotiOS' is incompatible with feature 'ios'
+@import RequiresNotiOS;
+// We should never get errors for submodules that don't match
+// CHECK-IOS-NOT: module 'RequiresMain'
+@import RequiresMain;
+#endif
+
+// RUN: mkdir %t/InputsB
+// RUN: echo "module RequiresiOSSimA {" >> %t/InputsB/module.map
+// RUN: echo "  requires iossimulator"  >> %t/InputsB/module.map
+// RUN: echo "}">> %t/InputsB/module.map
+// RUN: echo "module RequiresiOSSimB {" >> %t/InputsB/module.map
+// RUN: echo "  requires ios-simulator" >> %t/InputsB/module.map
+// RUN: echo "}">> %t/InputsB/module.map
+// RUN: %clang_cc1 -triple=x86_64-apple-iossimulator -DENABLE_IOSSIM -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsB %s  -verify
+// RUN: %clang_cc1 -triple=x86_64-apple-ios-simulator -DENABLE_IOSSIM -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsB %s  -verify
+
+#ifdef ENABLE_IOSSIM
+@import RequiresiOSSimA;
+@import RequiresiOSSimB;
+#endif
+
+// RUN: mkdir %t/InputsC
+// RUN: echo "module RequiresLinuxEABIA {"  >> %t/InputsC/module.map
+// RUN: echo "  requires linux-gnueabi" >> %t/InputsC/module.map
+// RUN: echo "}">> %t/InputsC/module.map
+// RUN: echo "module RequiresLinuxEABIB {"  >> %t/InputsC/module.map
+// RUN: echo "  requires gnueabi"   >> %t/InputsC/module.map
+// RUN: echo "}">> %t/InputsC/module.map
+// RUN: echo "module RequiresLinuxEABIC {"  >> %t/InputsC/module.map
+// RUN: echo "  requires linux" >> %t/InputsC/module.map
+// RUN: echo "}">> %t/InputsC/module.map
+// RUN: %clang_cc1 -triple=armv8r-none-linux-gnueabi -DENABLE_LINUXEABI -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsC %s -verify
+
+#ifdef ENABLE_LINUXEABI
+@import RequiresLinuxEABIA;
+@import RequiresLinuxEABIB;
+@import RequiresLinuxEABIC;
+#endif
+
+// RUN: mkdir %t/InputsD
+// RUN: echo "module RequiresWinMSVCA {"  >> %t/InputsD/module.map
+// RUN: echo "  requires windows" >> %t/InputsD/module.map
+// RUN: echo "}"  >> %t/InputsD/module.map
+// RUN: echo "module RequiresWinMSVCB {"  >> %t/InputsD/module.map
+// RUN: echo "  requires windows-msvc">> %t/InputsD/module.map
+// RUN: echo "}"  >> %t/InputsD/module.map
+// RUN: echo "module RequiresWinMSVCC {"  >> %t/InputsD/module.map
+// RUN: echo "  requires msvc">> %t/InputsD/module.map
+// RUN: echo "}"  >> %t/InputsD/module.map
+// RUN: %clang_cc1 -triple=thumbv7-unknown-windows-msvc -DENABLE_WINMSVC -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x objective-c -I%t/InputsD %s  -verify
+
+#ifdef ENABLE_WINMSVC
+@import RequiresWinMSVCA;
+@import RequiresWinMSVCB;

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai marked 2 inline comments as done.
vsapsai added inline comments.



Comment at: lib/Frontend/DependencyFile.cpp:325
+void DFGImpl::HasInclude(SourceLocation Loc, const FileEntry *File) {
+  if (!File)
+return;

rsmith wrote:
> Have you thought about whether we should add a dependency even for a missing 
> file under `-MG` (`AddMissingHeaderDeps`) mode? I think it's probably better 
> to not do so (ie, the behavior in this patch), but it seems worth considering.
Do you know how Make uses these missing files? Or maybe where I can find more. 
The only documentation I found says

> This feature is used in automatic updating of makefiles.

Which is not particularly illuminating.

Currently I prefer not to include not found `__has_include` files because they 
aren't really missing, it's OK if they aren't there and nothing has to be done 
to fix that. But I'd like to confirm if my understanding aligns with Make 
behaviour.


https://reviews.llvm.org/D30882



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai marked 3 inline comments as done.
vsapsai added inline comments.



Comment at: include/clang/Lex/PPCallbacks.h:263
+  /// \brief Callback invoked when a has_include directive is read.
+  virtual void HasInclude(SourceLocation Loc, const FileEntry *File) {
+  }

rsmith wrote:
> This callback seems pretty unhelpful in the case where lookup of the file 
> failed (you just get a source location). Could we pass in the `FileName` and 
> `IsAngled` flag too (like we do for `InclusionDirective`)? I think that's 
> what (eg) a callback tracking anti-dependencies (for a more sophisticated 
> build system that can handle them) would want.
Added. Also added `FileType` so we can ignore system headers for `-MMD`. And 
tweaked doxygen comment to be more consistent with other callbacks.



Comment at: test/Frontend/dependency-gen-has-include.c:11-15
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-has-include.o
+// CHECK: dependency-gen-has-include.c
+// CHECK: a/header.h

dexonsmith wrote:
> This covers quoted includes when there is a single `-I`.  I think there are a 
> couple of other cases worth testing:
> - Multiple `-I` arguments: all should be listed.
> - Angle includes: all `-isystem` should be listed too.
> - Also `-F` and `-iframework`.
I've added a test with system headers. Overall, my testing strategy was to test 
all cases for the following dimensions:
* file exists/file doesn't exist;
* file is user/system header;
* `__has_include`/`__has_include_next` is used.

But I don't test the full matrix as I don't think it provides enough value.



Comment at: test/Frontend/dependency-gen-has-include.c:17
+#if __has_include("a/header.h")
+#endif

bruno wrote:
> Can you also add a testcase for `__has_include_next`?
Added.


https://reviews.llvm.org/D30882



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


[PATCH] D51808: [CUDA] Ignore uncallable functions when we check for usual deallocators.

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:551
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->isUsualDeallocationFunction() ||
-Decl->isCopyAssignmentOperator() || Decl->isMoveAssignmentOperator() ||
-Decl->size_overridden_methods() > 0)
+Decl->isOverloadedOperator() || Decl->size_overridden_methods() > 0)
   return SK_Invalid;

I believe the `isOverloadedOperator` check here is redundant. The caller 
already checked that the name is an identifier.

If you want to keep some check of this kind, I'd suggest checking 
`D->getIdentifier()` at the start of this function and removing the check here. 
(Note that `CXXMethodDecl` is not the only `Decl` subclass that can have a 
non-identifier name.)



Comment at: clang/lib/AST/DeclCXX.cpp:2068-2076
+  bool Result = true;
+  for (const auto *D : R) {
+if (const auto *FD = dyn_cast(D)) {
+  Matches.push_back(FD);
   if (FD->getNumParams() == 1)
-return false;
+Result = false;
+}

I think it would make more sense to put only the 1-parameter functions into 
`Matches`, and rename it to `PreventedBy` or something: then it's the list of 
single-parameter usual deallocation functions that prevent this one being a 
usual deallocation function.


https://reviews.llvm.org/D51808



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


[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 165610.
vsapsai added a comment.

- Improve tests, fix -MMD, address comments.


https://reviews.llvm.org/D30882

Files:
  clang/include/clang/Lex/PPCallbacks.h
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Frontend/dependency-gen-has-include.c

Index: clang/test/Frontend/dependency-gen-has-include.c
===
--- /dev/null
+++ clang/test/Frontend/dependency-gen-has-include.c
@@ -0,0 +1,40 @@
+// REQUIRES: shell
+
+// Basic test
+// RUN: rm -rf %t.dir
+// RUN: mkdir %t.dir
+// RUN: mkdir %t.dir/a
+// RUN: echo "#ifndef HEADER_A" > %t.dir/a/header.h
+// RUN: echo "#define HEADER_A" >> %t.dir/a/header.h
+// RUN: echo "#endif" >> %t.dir/a/header.h
+// RUN: mkdir %t.dir/system
+// RUN: echo "#define SYSTEM_HEADER" > %t.dir/system/system-header.h
+// RUN: mkdir %t.dir/next-a
+// RUN: echo "#if __has_include_next()" > %t.dir/next-a/next-header.h
+// RUN: echo "#endif" >> %t.dir/next-a/next-header.h
+// RUN: mkdir %t.dir/next-b
+// RUN: echo "#define NEXT_HEADER" > %t.dir/next-b/next-header.h
+
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir -isystem %t.dir/system -I %t.dir/next-a -I %t.dir/next-b
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-has-include.o
+// CHECK: dependency-gen-has-include.c
+// CHECK: a/header.h
+// CHECK-NOT: missing/file.h
+// CHECK: system/system-header.h
+// CHECK: next-a/next-header.h
+// CHECK: next-b/next-header.h
+
+// Verify that we ignore system headers in user-only headers mode.
+// RUN: %clang -MMD -MF %t.dir/user-headers.deps %s -fsyntax-only -I %t.dir -isystem %t.dir/system -I %t.dir/next-a -I %t.dir/next-b
+// RUN: FileCheck -input-file=%t.dir/user-headers.deps --check-prefix CHECK-USER-HEADER %s
+// CHECK-USER-HEADER-NOT: system/system-header.h
+
+#if __has_include("a/header.h")
+#endif
+#if __has_include("missing/file.h")
+#endif
+#if __has_include()
+#endif
+
+#include 
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -23,6 +23,7 @@
 #include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Lex/DirectoryLookup.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
@@ -1242,6 +1243,13 @@
   PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, LookupFromFile,
 CurDir, nullptr, nullptr, nullptr, nullptr);
 
+  if (PPCallbacks *Callbacks = PP.getPPCallbacks()) {
+SrcMgr::CharacteristicKind FileType = SrcMgr::C_User;
+if (File)
+  FileType = PP.getHeaderSearchInfo().getFileDirFlavor(File);
+Callbacks->HasInclude(FilenameLoc, Filename, isAngled, File, FileType);
+  }
+
   // Get the result value.  A result of true means the file exists.
   return File != nullptr;
 }
Index: clang/lib/Frontend/DependencyFile.cpp
===
--- clang/lib/Frontend/DependencyFile.cpp
+++ clang/lib/Frontend/DependencyFile.cpp
@@ -200,6 +200,10 @@
   const Module *Imported,
   SrcMgr::CharacteristicKind FileType) override;
 
+  void HasInclude(SourceLocation Loc, StringRef SpelledFilename, bool IsAngled,
+  const FileEntry *File,
+  SrcMgr::CharacteristicKind FileType) override;
+
   void EndOfMainFile() override {
 OutputDependencyFile();
   }
@@ -328,6 +332,17 @@
   }
 }
 
+void DFGImpl::HasInclude(SourceLocation Loc, StringRef SpelledFilename,
+ bool IsAngled, const FileEntry *File,
+ SrcMgr::CharacteristicKind FileType) {
+  if (!File)
+return;
+  StringRef Filename = File->getName();
+  if (!FileMatchesDepCriteria(Filename.data(), FileType))
+return;
+  AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
+}
+
 bool DFGImpl::AddFilename(StringRef Filename) {
   if (FilesSet.insert(Filename).second) {
 Files.push_back(Filename);
Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -276,6 +276,12 @@
SourceRange Range) {
   }
 
+  /// Hook called when a '__has_include' or '__has_include_next' directive is
+  /// read.
+  virtual void HasInclude(SourceLocation Loc, StringRef FileName, bool IsAngled,
+  const FileEntry *File,
+  SrcMgr::CharacteristicKind FileType) {}
+
   /// Hook called when a source range is skipped.
   /// \param Range The SourceRange that was skipped. The range begins at the
   /// \#if/\#else directive and ends after the \#endif/\#else 

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

2018-09-14 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai commandeered this revision.
vsapsai added a reviewer: pete.
vsapsai added a comment.

Taking over the change, I'll address the reviewers' comments.


https://reviews.llvm.org/D30882



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


[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

2018-09-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

This is definitely the right thing to do, thanks for finding it!
I've got a long comment about how everything used to be simpler in the good old 
days :-) I'm itching to refactor a bit, but land this first.




Comment at: clangd/index/FileIndex.cpp:34
   CollectorOpts.CountReferences = false;
+  CollectorOpts.CollectMacro = true;
   if (!URISchemes.empty())

I don't understand this change. Don't we also want this only if IndexMacros is 
true?
(It's possible it doesn't make a difference due to implementation details but 
it seems worth being clear here)



Comment at: clangd/index/FileIndex.h:93
 std::pair
 indexAST(ASTContext , std::shared_ptr PP,
+ bool IndexMacros = false,

I'm concerned about the proliferation of parameters here. (Not new with this 
patch, it's been a continuous process - the first version had one input and one 
output!)
It's heading in the direction of being a catch-all "collect some data from an 
AST" function, at which point each caller has to think hard about every option 
and we're going to end up with bugs.
(For example: TestTU::index() passes "false" for IndexMacros. It seems to me 
maybe it should be "true". But it's really hard to tell.)
That's also pretty similar to the mission of SymbolCollector itself, so we're 
going to get some confusion there.

As far as I can tell, there's now two types of indexing that we do:
  - preamble indexing: we look at all decls, and only index those outside the 
main file. We index macros. We don't collect references. Inputs are ASTContext 
and PP.
  - mainfile-style indexing: we look only at top-level decls in the main file. 
We don't index macros. We collect references from the main file. Inputs are 
ParsedAST.
This really looks like it should be 2 functions with 2 and 1 parameters, rather 
than 1 function with 4.
Then callers will have two styles of indexing (with names!) to choose between, 
rather than being required to design their own. And we can get rid of the "is 
this a main-file index?" hacks in the implementation.

Sorry for jumping on you here, this change isn't any worse than the three 
previous patches that added knobs to this function.
This doesn't need to be addressed in this patch - could be before or after, and 
I'm happy to take this on myself. But I think we shouldn't kick this can down 
the road too much further, eventually we end up with APIs like clang :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078



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


[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

2018-09-14 Thread David CARLIER via Phabricator via cfe-commits
devnexen updated this revision to Diff 165604.

https://reviews.llvm.org/D49722

Files:
  lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  test/Analysis/cstring-syntax.c

Index: test/Analysis/cstring-syntax.c
===
--- test/Analysis/cstring-syntax.c
+++ test/Analysis/cstring-syntax.c
@@ -7,6 +7,7 @@
 char  *strncat(char *, const char *, size_t);
 size_t strlen (const char *s);
 size_t strlcpy(char *, const char *, size_t);
+size_t strlcat(char *, const char *, size_t);
 
 void testStrncat(const char *src) {
   char dest[10];
@@ -33,3 +34,21 @@
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}}
 }
+
+void testStrlcat(const char *src) {
+  char dest[10];
+  size_t badlen = 20;
+  size_t ulen;
+  strlcpy(dest, "a", sizeof("a") - 1);
+  strlcat(dest, "", (sizeof("") - 1) - sizeof(dest) - 1);
+  strlcpy(dest, "012345678", sizeof(dest));
+  strlcat(dest, "910", sizeof(dest));
+  strlcpy(dest, "0123456789", sizeof(dest));
+  strlcpy(dest, "0123456789", sizeof(dest));
+  strlcat(dest, "0123456789", badlen / 2);
+  strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
+  strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
+  strlcat(dest, src, ulen);
+  strlcpy(dest, src, 5);
+  strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof() or lower}}
+}
Index: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
@@ -90,7 +90,16 @@
   ///   strlcpy(dst, "abcd", 4);
   ///   strlcpy(dst + 3, "abcd", 2);
   ///   strlcpy(dst, "abcd", cpy);
-  bool containsBadStrlcpyPattern(const CallExpr *CE);
+  /// Identify erroneous patterns in the last argument to strlcat - the number
+  /// of bytes to copy.
+  /// The bad pattern checked is when the last argument is basically
+  /// pointing to the destination buffer size or argument larger or
+  /// equal to.
+  ///   char dst[2];
+  ///   strlcat(dst, src2, sizeof(dst));
+  ///   strlcat(dst, src2, 2);
+  ///   strlcat(dst, src2, 10);
+  bool containsBadStrlcpyStrlcatPattern(const CallExpr *CE);
 
 public:
   WalkAST(const CheckerBase *Checker, BugReporter , AnalysisDeclContext *AC)
@@ -142,15 +151,19 @@
   return false;
 }
 
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
 return false;
+  const FunctionDecl *FD = CE->getDirectCallee();
+  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
   const auto *DstArgDecl = dyn_cast(DstArg->IgnoreParenImpCasts());
   const auto *LenArgDecl = dyn_cast(LenArg->IgnoreParenLValueCasts());
   uint64_t DstOff = 0;
+  if (isSizeof(LenArg, DstArg))
+return false;
   // - size_t dstlen = sizeof(dst)
   if (LenArgDecl) {
 const auto *LenArgVal = dyn_cast(LenArgDecl->getDecl());
@@ -181,8 +194,14 @@
   if (const auto *Buffer = dyn_cast(DstArgDecl->getType())) {
 ASTContext  = BR.getContext();
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
-  return true;
+auto RemainingBufferLen = BufferLen - DstOff;
+if (Append) {
+  if (RemainingBufferLen <= ILRawVal)
+return true;
+} else {
+  if (RemainingBufferLen < ILRawVal)
+return true;
+}
   }
 }
   }
@@ -220,7 +239,7 @@
  LenArg->getSourceRange());
 }
   } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) {
-if (containsBadStrlcpyPattern(CE)) {
+if (containsBadStrlcpyStrlcatPattern(CE)) {
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
   PathDiagnosticLocation Loc =
@@ -234,6 +253,29 @@
   if (!DstName.empty())
 os << "Replace with the value 'sizeof(" << DstName << ")` or lower";
 
+  BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument",
+ "C String API", os.str(), Loc,
+ LenArg->getSourceRange());
+}
+  } else if (CheckerContext::isCLibraryFunction(FD, "strlcat")) {
+if (containsBadStrlcpyStrlcatPattern(CE)) {
+  const Expr *DstArg = CE->getArg(0);
+  const Expr *LenArg = CE->getArg(2);
+  PathDiagnosticLocation Loc =
+PathDiagnosticLocation::createBegin(LenArg, BR.getSourceManager(), AC);
+
+  StringRef DstName = getPrintableName(DstArg);
+
+  

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

2018-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: test/Analysis/cstring-syntax.c:49
+  strlcat(dest, "0123456789", badlen / 2);
+  strlcat(dest, "0123456789", badlen); // expected-warning {{The third 
argument allows to potentially copy more bytes than it should. Replace with the 
value 'badlen' - strlen(dest) - 1 or lower}}
+  strlcat(dest, "0123456789", badlen - strlen(dest) - 1);

devnexen wrote:
> NoQ wrote:
> > The suggested fix is a bit weird.
> > 
> > The correct code for appending `src` to `dst` is either `strlcat(dst, src, 
> > sizeof(dst));` (the approach suggested by the man page) or `strlcat(dst + 
> > strlen(dst) + 1, src, sizeof(dst) - strlen(dst) - 1)` (which is equivalent 
> > but faster if you already know `strlen(dst)`). In both cases you can 
> > specify a smaller value but not a larger value.
> In fact in this case the message is misleading/a bit wrong.
I think `strlcat` is very clumsy to you if you need to add an offset to 
`dest`...

For
`strlcat(dst + strlen(dst) + 1, src, sizeof(dst) - strlen(dst) - 1)`

I suppose you meant:

`strlcpy(dst + strlen(dst), src, sizeof(dst) - strlen(dst))`

... but the suggestion does not look very appealing. `strlcat(dst, ..., 
sizeof(dst)` if good enough as a suggested fix.


https://reviews.llvm.org/D49722



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


r342290 - test/Driver/output-file-cleanup.c: delete non-readable temporary file

2018-09-14 Thread Fangrui Song via cfe-commits
Author: maskray
Date: Fri Sep 14 14:36:35 2018
New Revision: 342290

URL: http://llvm.org/viewvc/llvm-project?rev=342290=rev
Log:
test/Driver/output-file-cleanup.c: delete non-readable temporary file

%t-dir/2.c made tools (rsync, ripgrep, ...) sad (EACCES warning).

Modified:
cfe/trunk/test/Driver/output-file-cleanup.c

Modified: cfe/trunk/test/Driver/output-file-cleanup.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=342290=342289=342290=diff
==
--- cfe/trunk/test/Driver/output-file-cleanup.c (original)
+++ cfe/trunk/test/Driver/output-file-cleanup.c Fri Sep 14 14:36:35 2018
@@ -41,3 +41,4 @@ invalid C code
 // RUN: not %clang -S %t-dir/1.c %t-dir/2.c
 // RUN: test -f %t-dir/1.s
 // RUN: test ! -f %t-dir/2.s
+// RUN: rm -f %t-dir/2.c


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


[PATCH] D52076: [CodeComplete] Add completions for filenames in #include directives.

2018-09-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Addressed the comments I was sure about.
A couple of open questions in SemaCodeComplete about the formatting of the 
completions, but want to know what you think.




Comment at: lib/Sema/SemaCodeComplete.cpp:8046
+ !EC && It != vfs::directory_iterator(); It.increment(EC)) {
+  if (++Count == 1000) // If we happen to hit a huge directory,
+break; // bail out early so we're not too slow.

ilya-biryukov wrote:
> The size of `/usr/include` on my machine is ~830 files and dirs, which is a 
> little close to a limit.
> Not sure if the number of files grow with time or with a number of installed 
> packages, but maybe we want a slightly higher limit to make sure it's won 
> stop showing some results from this important include dir anytime soon?
Wow, I only have 300.

Bumped the limit to 2500. This is based on handwavy advice on the internet that 
bad filesystems get slow in the low thousands.



Comment at: lib/Sema/SemaCodeComplete.cpp:8076
+  // header maps are not (currently) enumerable.
+  break;
+case DirectoryLookup::LT_NormalDir:

ilya-biryukov wrote:
> NIT: Maybe return from each branch and put llvm_unreachable at the end of the 
> function?
> To get an error in case the new enum value is added in addition to the 
> compiler warning.
> 
> Feel free to ignore if you like the current version more, the compiler 
> warning is not likely to go unnoticed anyway.
That won't give an error, that will give undefined behavior! (If a new value is 
added and the warning is missed)
The current code will just skip over the directory in that case, which I think 
is fine. (We have -Werror buildbots)

(Unless you mean return a dummy value, which is a little to unusual for my 
taste)


Repository:
  rC Clang

https://reviews.llvm.org/D52076



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


[PATCH] D52076: [CodeComplete] Add completions for filenames in #include directives.

2018-09-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 165593.
sammccall marked 9 inline comments as done.
sammccall added a comment.

Unify common completion code from angled/quoted strings in Lexer.
Handle #include paths with \ on windows (normalize them to /)
Document why we picked particular extensions for headers.
Increase per-dir listing limit to 2500.

1. Updating https://reviews.llvm.org/D52076: [CodeComplete] Add completions for 
filenames in #include directives. #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

:


Repository:
  rC Clang

https://reviews.llvm.org/D52076

Files:
  include/clang-c/Index.h
  include/clang/Lex/CodeCompletionHandler.h
  include/clang/Lex/Lexer.h
  include/clang/Lex/Preprocessor.h
  include/clang/Parse/Parser.h
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-files.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -499,6 +499,10 @@
   contexts = CXCompletionContext_NaturalLanguage;
   break;
 }
+case CodeCompletionContext::CCC_IncludedFile: {
+  contexts = CXCompletionContext_IncludedFile;
+  break;
+}
 case CodeCompletionContext::CCC_SelectorName: {
   contexts = CXCompletionContext_ObjCSelectorName;
   break;
Index: test/CodeCompletion/included-files.cpp
===
--- /dev/null
+++ test/CodeCompletion/included-files.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t && mkdir %t && cp %s %t/main.cc && mkdir %t/a
+// RUN: touch %t/foo.h && touch %t/foo.cc && touch %t/a/foosys %t/a/foosys.h
+
+// Quoted string shows header-ish files from CWD, and all from system.
+#include "foo.h"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:5:13 %t/main.cc | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: "foo.cc"
+// CHECK-1: "foo.h"
+// CHECK-1: "foosys"
+
+// Quoted string with dir shows header-ish files in that subdir.
+#include "a/foosys"
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:12:13 %t/main.cc | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2-NOT: "a/foosys"
+// CHECK-2: "a/foosys.h"
+// CHECK-2-NOT: "a/foosys"
+// CHECK-2-NOT: "foo.h"
+// CHECK-2-NOT: "foosys"
+
+// Angled string showes all files, but only in system dirs.
+#include 
+// RUN: %clang -fsyntax-only -isystem %t/a -Xclang -code-completion-at=%t/main.cc:21:13 %t/main.cc | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3-NOT: 
+// CHECK-3-NOT: 
+// CHECK-3: 
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -32,6 +32,8 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Path.h"
 #include 
 #include 
 #include 
@@ -7994,6 +7996,120 @@
   // for the expanded tokens.
 }
 
+// This handles completion inside an #include filename, e.g. #include getAllocator().CopyString(
+ (Angled ? "<" : "\"") + RelDir + "/");
+  // We need the native slashes for the actual file system interactions.
+  SmallString<128> NativeRelDir = StringRef(RelDir);
+  llvm::sys::path::native(NativeRelDir);
+  auto FS = getSourceManager().getFileManager().getVirtualFileSystem();
+
+  ResultBuilder Results(*this, CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo(),
+CodeCompletionContext::CCC_IncludedFile);
+  llvm::DenseSet SeenResults; // To deduplicate results.
+
+  // Helper: adds one file or directory completion result.
+  auto AddCompletion = [&](StringRef Filename, bool IsDirectory) {
+SmallString<64> TypedChunk = Filename;
+// Directory completion is up to the slash, e.g. ' : '"');
+auto R = SeenResults.insert(TypedChunk);
+if (R.second) { // New completion
+  const char *InternedTyped = Results.getAllocator().CopyString(TypedChunk);
+  *R.first = InternedTyped; // Avoid dangling StringRef.
+  CodeCompletionBuilder Builder(CodeCompleter->getAllocator(),
+CodeCompleter->getCodeCompletionTUInfo());
+  Builder.AddTextChunk(InternedPrefix);
+  Builder.AddTypedTextChunk(InternedTyped);
+  // The result is a "Pattern", which is pretty opaque.
+  // We may want to include the real filename to allow smart ranking.

Re: [clang-tools-extra] r342036 - Fix buildbots after r342027

2018-09-14 Thread Kirill Bobyrev via cfe-commits
Oh, I see. Thank you very much for fixing it!

I had concerns about platform dependence, but I’ve seen seen some tests in 
libcxx which use `if […]` so I thought it should be alright (because there was 
no REQUIRES there). I can see now, that it is a Darwin test, my bad.

Again, thank you very much for helping and apologies for inconvenience.

Kind regards,
Kirill

> On 14 Sep 2018, at 22:53, Reid Kleckner  wrote:
> 
> That construct does not work on Windows, where we don't use bash. We use the 
> lit internal shell, which doesn't support 'if [ ...]'. I marked it REQUIRES: 
> shell in r342282.
> 
> On Wed, Sep 12, 2018 at 2:29 AM Kirill Bobyrev via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: omtcyfz
> Date: Wed Sep 12 02:27:55 2018
> New Revision: 342036
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=342036=rev 
> 
> Log:
> Fix buildbots after r342027
> 
> Modified:
> clang-tools-extra/trunk/test/clangd/index-tools.test
> 
> Modified: clang-tools-extra/trunk/test/clangd/index-tools.test
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/index-tools.test?rev=342036=342035=342036=diff
>  
> 
> ==
> --- clang-tools-extra/trunk/test/clangd/index-tools.test (original)
> +++ clang-tools-extra/trunk/test/clangd/index-tools.test Wed Sep 12 02:27:55 
> 2018
> @@ -1,2 +1,3 @@
>  # RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > 
> %t.index
> -# RUN: %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log 
> --benchmark_min_time=0.01
> +# FIXME: By default, benchmarks are excluded from the list of default 
> targets hence not built. Find a way to depend on benchmarks to run the next 
> command.
> +# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then 
> %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log 
> --benchmark_min_time=0.01 ; fi
> 
> 
> ___
> 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] D51905: Front-end of the implementation of the interleaving algorithm

2018-09-14 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich added a comment.

In https://reviews.llvm.org/D51905#1234308, @zhaomo wrote:

> In https://reviews.llvm.org/D51905#1231383, @vlad.tsyrklevich wrote:
>
> > This change causes all compiler-rt cfi tests to be UNSUPPORTED for me 
> > locally, do you have any idea why that might be? The lit changes don't make 
> > it immediately clear.
>
>
> Not sure why that happened. How did you run the compiler-rt tests? Did you 
> use ninja check-cfi?


I ran them with llvm-lit against the tests under test/cfi. It looks like 
something caused the default test suite to change where all of the tests were 
marked unsupported (the test suite ran without lld and my default linker 
otherwise is bfd so LTO was unsupported.) Not a problem with the change.




Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:939
 
+/// We should only interleave vtables when the module has the hidden
+/// LTO visibility, cfi-vcall is enabled and EnableVTableInterleaving

zhaomo wrote:
> vlad.tsyrklevich wrote:
> > doxygen comments normally go in the corresponding header file.
> I just followed the style of the rest of the file. All the comments above 
> functions use ///. Do I need to change them?
Disregard that, I hadn't realized that was the style elsewhere.


https://reviews.llvm.org/D51905



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


[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

2018-09-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Thank you for working on this!

In https://reviews.llvm.org/D52120#1235480, @lebedev.ri wrote:

> In https://reviews.llvm.org/D52120#1235479, @shuaiwang wrote:
>
> > @lebedev.ri could you help test whether this fully resolves PR38891? Thanks!
>
>
> That is what i'm trying to do here :)
>  Still waiting for the build to finish...


Ok, got the results.
Without that change, my check fires ~1k times.
With this change it only fires 572 times.
So this clearly //changes// things.
The entirety of the new build log: F7199224: buildlog.txt 

I see no mentions of `emplace_back`/`push_back`.
Though some of that is still false-positives (pointers, va_arg, callback 
lambdas passed as templated function param), but i'll file further bugs i guess.

As far i'm concerned, this is good.
But please wait for the actual code review.

Once again, thank you for working on this!


Repository:
  rC Clang

https://reviews.llvm.org/D52120



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


[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

2018-09-14 Thread David CARLIER via Phabricator via cfe-commits
devnexen added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:199
+if (Append)
+  RemainingBufferLen -= 1;
+if (RemainingBufferLen < ILRawVal)

MaskRay wrote:
> `RemainingBufferLen` is `uint64_t`. Can the `-= 1` overflow?
That s a good point. I may redo as it was before.


https://reviews.llvm.org/D49722



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


Re: [clang-tools-extra] r342036 - Fix buildbots after r342027

2018-09-14 Thread Reid Kleckner via cfe-commits
That construct does not work on Windows, where we don't use bash. We use
the lit internal shell, which doesn't support 'if [ ...]'. I marked it
REQUIRES: shell in r342282.

On Wed, Sep 12, 2018 at 2:29 AM Kirill Bobyrev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: omtcyfz
> Date: Wed Sep 12 02:27:55 2018
> New Revision: 342036
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342036=rev
> Log:
> Fix buildbots after r342027
>
> Modified:
> clang-tools-extra/trunk/test/clangd/index-tools.test
>
> Modified: clang-tools-extra/trunk/test/clangd/index-tools.test
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/index-tools.test?rev=342036=342035=342036=diff
>
> ==
> --- clang-tools-extra/trunk/test/clangd/index-tools.test (original)
> +++ clang-tools-extra/trunk/test/clangd/index-tools.test Wed Sep 12
> 02:27:55 2018
> @@ -1,2 +1,3 @@
>  # RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs
> > %t.index
> -# RUN: %clangd-benchmark-dir/IndexBenchmark %t.index
> %p/Inputs/requests.log --benchmark_min_time=0.01
> +# FIXME: By default, benchmarks are excluded from the list of default
> targets hence not built. Find a way to depend on benchmarks to run the next
> command.
> +# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then
> %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log
> --benchmark_min_time=0.01 ; fi
>
>
> ___
> 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


[clang-tools-extra] r342282 - Mark index-tools.test as REQUIRES: shell so that it does not run with the internal lit shell which does not support "if"

2018-09-14 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Sep 14 13:51:07 2018
New Revision: 342282

URL: http://llvm.org/viewvc/llvm-project?rev=342282=rev
Log:
Mark index-tools.test as REQUIRES: shell so that it does not run with the 
internal lit shell which does not support "if"

Modified:
clang-tools-extra/trunk/test/clangd/index-tools.test

Modified: clang-tools-extra/trunk/test/clangd/index-tools.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/index-tools.test?rev=342282=342281=342282=diff
==
--- clang-tools-extra/trunk/test/clangd/index-tools.test (original)
+++ clang-tools-extra/trunk/test/clangd/index-tools.test Fri Sep 14 13:51:07 
2018
@@ -1,5 +1,6 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets 
hence not built. Find a way to depend on benchmarks to run the next command.
+# REQUIRES: shell
 # RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then 
%clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json 
--benchmark_min_time=0.01 ; fi
 # Pass invalid JSON file and check that IndexBenchmark fails to parse it.
 # RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then not 
%clangd-benchmark-dir/IndexBenchmark %t.index %t --benchmark_min_time=0.01 ; fi


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


r342281 - Relax assumption about default method calling convention in new test

2018-09-14 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Fri Sep 14 13:50:39 2018
New Revision: 342281

URL: http://llvm.org/viewvc/llvm-project?rev=342281=rev
Log:
Relax assumption about default method calling convention in new test

Modified:
cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp

Modified: cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp?rev=342281=342280=342281=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-lambda.cpp Fri Sep 14 13:50:39 2018
@@ -6,7 +6,7 @@ void lambda_in_func(int ) {
   // CHECK: [[ref_slot:%.*]] = getelementptr inbounds %class.anon, 
%class.anon* {{.*}}, i32 0, i32 0, !dbg [[lambda_decl_loc:![0-9]+]]
   // CHECK-NEXT: %1 = load i32*, i32** %ref.addr, align {{.*}}, !dbg 
[[capture_init_loc:![0-9]+]]
   // CHECK-NEXT: store i32* %1, i32** %0, align {{.*}}, !dbg 
[[lambda_decl_loc]]
-  // CHECK-NEXT: call void {{.*}}, !dbg [[lambda_call_loc:![0-9]+]]
+  // CHECK-NEXT: call {{.*}}void {{.*}}, !dbg [[lambda_call_loc:![0-9]+]]
 
   auto helper = [   // CHECK: [[lambda_decl_loc]] = !DILocation(line: 
[[@LINE]], column: 17
  &]() { // CHECK: [[capture_init_loc]] = !DILocation(line: 
[[@LINE]], column: 18


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


[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

2018-09-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+  // - sizeof(dst)
+  if (Append && isSizeof(LenArg, DstArg))
+return true;

george.karpenkov wrote:
> george.karpenkov wrote:
> > george.karpenkov wrote:
> > > I am confused on what is happening in this branch and why is it bad. 
> > > Could we add a commend?
> > Sorry I'm still confused about this branch: could you clarify?
> Ah, OK, I see it needs one more byte due to null-termination.
I think the `if (isSizeof(LenArg, DstArg))` check is fine so it returns `false`.

`strlcat(dst, src, sizeof dst)` is good.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:199
+if (Append)
+  RemainingBufferLen -= 1;
+if (RemainingBufferLen < ILRawVal)

`RemainingBufferLen` is `uint64_t`. Can the `-= 1` overflow?


https://reviews.llvm.org/D49722



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


[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

2018-09-14 Thread Tanoy Sinha via Phabricator via cfe-commits
tks2103 added a comment.

ping @GorNishanov


Repository:
  rC Clang

https://reviews.llvm.org/D51741



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


[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

2018-09-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D52120#1235479, @shuaiwang wrote:

> @lebedev.ri could you help test whether this fully resolves PR38891? Thanks!


That is what i'm trying to do here :)
Still waiting for the build to finish...


Repository:
  rC Clang

https://reviews.llvm.org/D52120



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


[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

2018-09-14 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment.

@lebedev.ri could you help test whether this fully resolves PR38891? Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D52120



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


[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

2018-09-14 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang created this revision.
shuaiwang added reviewers: lebedev.ri, JonasToth.
Herald added subscribers: cfe-commits, Szelethus, mikhail.ramalho, a.sidorin, 
szepet, xazax.hun.
Herald added a reviewer: george.karpenkov.

This is a follow up of https://reviews.llvm.org/D52008 and should make the 
analyzer being able to handle perfect forwardings in real world cases where 
forwardings are done through multiple layers of function calls with 
`std::forward`.

Fixes PR38891.


Repository:
  rC Clang

https://reviews.llvm.org/D52120

Files:
  lib/Analysis/ExprMutationAnalyzer.cpp
  unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -373,36 +373,46 @@
 }
 
 TEST(ExprMutationAnalyzerTest, Move) {
-  // Technically almost the same as ByNonConstRRefArgument, just double checking
-  const auto AST = tooling::buildASTFromCode(
+  const std::string Move =
   "namespace std {"
   "template struct remove_reference { typedef T type; };"
   "template struct remove_reference { typedef T type; };"
   "template struct remove_reference { typedef T type; };"
   "template typename std::remove_reference::type&& "
-  "move(T&& t) noexcept; }"
-  "void f() { struct A {}; A x; std::move(x); }");
-  const auto Results =
+  "move(T&& t) noexcept {} }";
+  auto AST = tooling::buildASTFromCode(
+  Move + "void f() { struct A {}; A x; std::move(x); }");
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x)"));
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  Move + "void f() { struct A {}; A x, y; std::move(x) = y; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x) = y"));
 }
 
 TEST(ExprMutationAnalyzerTest, Forward) {
-  // Technically almost the same as ByNonConstRefArgument, just double checking
-  const auto AST = tooling::buildASTFromCode(
+  const std::string Forward =
   "namespace std {"
   "template struct remove_reference { typedef T type; };"
   "template struct remove_reference { typedef T type; };"
   "template struct remove_reference { typedef T type; };"
   "template T&& "
-  "forward(typename std::remove_reference::type&) noexcept;"
+  "forward(typename std::remove_reference::type&) noexcept {}"
   "template T&& "
-  "forward(typename std::remove_reference::type&&) noexcept;"
-  "void f() { struct A {}; A x; std::forward(x); }");
-  const auto Results =
+  "forward(typename std::remove_reference::type&&) noexcept {} }";
+  auto AST = tooling::buildASTFromCode(
+  Forward + "void f() { struct A {}; A x; std::forward(x); }");
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  Forward + "void f() { struct A {}; A x, y; std::forward(x) = y; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
-  ElementsAre("std::forward(x)"));
+  ElementsAre("std::forward(x) = y"));
 }
 
 TEST(ExprMutationAnalyzerTest, CallUnresolved) {
@@ -639,6 +649,26 @@
   "void f() { int x; S s(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
+  AST = tooling::buildASTFromCode(
+  "namespace std {"
+  "template struct remove_reference { typedef T type; };"
+  "template struct remove_reference { typedef T type; };"
+  "template struct remove_reference { typedef T type; };"
+  "template T&& "
+  "forward(typename std::remove_reference::type& t) noexcept"
+  "{ return t; }"
+  "template T&& "
+  "forward(typename std::remove_reference::type&& t) noexcept"
+  "{ return t; } }"
+  "template  void u(Args&...);"
+  "template  void h(Args&&... args)"
+  "{ u(std::forward(args)...); }"
+  "template  void g(Args&&... args)"
+  "{ h(std::forward(args)...); }"
+  "void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
 }
 
 TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
@@ -683,6 +713,26 @@
   "void f() { int x; S s(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "namespace std {"
+  "template struct remove_reference 

[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

2018-09-14 Thread Shuai Wang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342271: [analyzer] Handle forwarding reference better in 
ExprMutationAnalyzer. (authored by shuaiwang, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52008

Files:
  cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
  cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -79,7 +79,8 @@
  ::findArrayElementMutation,
  ::findCastMutation,
  ::findRangeLoopMutation,
- ::findReferenceMutation}) {
+ ::findReferenceMutation,
+ ::findFunctionArgMutation}) {
 if (const Stmt *S = (this->*Finder)(Exp))
   return Results[Exp] = S;
   }
@@ -192,10 +193,15 @@
 
   // Used as non-const-ref argument when calling a function.
   // An argument is assumed to be non-const-ref when the function is unresolved.
+  // Instantiated template functions are not handled here but in
+  // findFunctionArgMutation which has additional smarts for handling forwarding
+  // references.
   const auto NonConstRefParam = forEachArgumentWithParam(
   equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(;
+  const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
   const auto AsNonConstRefArg = anyOf(
-  callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam),
+  callExpr(NonConstRefParam, NotInstantiated),
+  cxxConstructExpr(NonConstRefParam, NotInstantiated),
   callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
  cxxDependentScopeMemberExpr(),
  hasType(templateTypeParmType(),
@@ -305,4 +311,82 @@
   return findDeclMutation(Refs);
 }
 
+const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
+  const auto NonConstRefParam = forEachArgumentWithParam(
+  equalsNode(Exp),
+  parmVarDecl(hasType(nonConstReferenceType())).bind("parm"));
+  const auto IsInstantiated = hasDeclaration(isInstantiated());
+  const auto FuncDecl = hasDeclaration(functionDecl().bind("func"));
+  const auto Matches = match(
+  findAll(expr(anyOf(callExpr(NonConstRefParam, IsInstantiated, FuncDecl),
+ cxxConstructExpr(NonConstRefParam, IsInstantiated,
+  FuncDecl)))
+  .bind("expr")),
+  Stm, Context);
+  for (const auto  : Matches) {
+const auto *Exp = Nodes.getNodeAs("expr");
+const auto *Func = Nodes.getNodeAs("func");
+if (!Func->getBody())
+  return Exp;
+
+const auto *Parm = Nodes.getNodeAs("parm");
+const ArrayRef AllParams =
+Func->getPrimaryTemplate()->getTemplatedDecl()->parameters();
+QualType ParmType =
+AllParams[std::min(Parm->getFunctionScopeIndex(),
+   AllParams.size() - 1)]
+->getType();
+if (const auto *T = ParmType->getAs())
+  ParmType = T->getPattern();
+
+// If param type is forwarding reference, follow into the function
+// definition and see whether the param is mutated inside.
+if (const auto *RefType = ParmType->getAs()) {
+  if (!RefType->getPointeeType().getQualifiers() &&
+  RefType->getPointeeType()->getAs()) {
+std::unique_ptr  =
+FuncParmAnalyzer[Func];
+if (!Analyzer)
+  Analyzer.reset(new FunctionParmMutationAnalyzer(*Func, Context));
+if (Analyzer->findMutation(Parm))
+  return Exp;
+continue;
+  }
+}
+// Not forwarding reference.
+return Exp;
+  }
+  return nullptr;
+}
+
+FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer(
+const FunctionDecl , ASTContext )
+: BodyAnalyzer(*Func.getBody(), Context) {
+  if (const auto *Ctor = dyn_cast()) {
+// CXXCtorInitializer might also mutate Param but they're not part of
+// function body, check them eagerly here since they're typically trivial.
+for (const CXXCtorInitializer *Init : Ctor->inits()) {
+  ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context);
+  for (const ParmVarDecl *Parm : Ctor->parameters()) {
+if (Results.find(Parm) != Results.end())
+  continue;
+if (const Stmt *S = InitAnalyzer.findDeclMutation(Parm))
+  Results[Parm] = S;
+  }
+}
+  }
+}
+
+const Stmt *
+FunctionParmMutationAnalyzer::findMutation(const ParmVarDecl *Parm) {
+  const auto Memoized = Results.find(Parm);
+  if (Memoized != Results.end())
+return 

r342271 - [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

2018-09-14 Thread Shuai Wang via cfe-commits
Author: shuaiwang
Date: Fri Sep 14 13:07:18 2018
New Revision: 342271

URL: http://llvm.org/viewvc/llvm-project?rev=342271=rev
Log:
[analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

Summary:
We used to treat an `Expr` mutated whenever it's passed as non-const
reference argument to a function. This results in false positives in
cases like this:
```
int x;
std::vector v;
v.emplace_back(x); // `x` is passed as non-const reference to `emplace_back`
```
In theory the false positives can be suppressed with
`v.emplace_back(std::as_const(x))` but that's considered overly verbose,
inconsistent with existing code and spammy as diags.

This diff handles such cases by following into the function definition
and see whether the argument is mutated inside.

Reviewers: lebedev.ri, JonasToth, george.karpenkov

Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, 
cfe-commits

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

Modified:
cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h?rev=342271=342270=342271=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h Fri Sep 14 
13:07:18 2018
@@ -17,6 +17,8 @@
 
 namespace clang {
 
+class FunctionParmMutationAnalyzer;
+
 /// Analyzes whether any mutative operations are applied to an expression 
within
 /// a given statement.
 class ExprMutationAnalyzer {
@@ -27,13 +29,13 @@ public:
   bool isMutated(const Decl *Dec) { return findDeclMutation(Dec) != nullptr; }
   bool isMutated(const Expr *Exp) { return findMutation(Exp) != nullptr; }
   const Stmt *findMutation(const Expr *Exp);
+  const Stmt *findDeclMutation(const Decl *Dec);
 
 private:
   bool isUnevaluated(const Expr *Exp);
 
   const Stmt *findExprMutation(ArrayRef Matches);
   const Stmt *findDeclMutation(ArrayRef Matches);
-  const Stmt *findDeclMutation(const Decl *Dec);
 
   const Stmt *findDirectMutation(const Expr *Exp);
   const Stmt *findMemberMutation(const Expr *Exp);
@@ -41,12 +43,32 @@ private:
   const Stmt *findCastMutation(const Expr *Exp);
   const Stmt *findRangeLoopMutation(const Expr *Exp);
   const Stmt *findReferenceMutation(const Expr *Exp);
+  const Stmt *findFunctionArgMutation(const Expr *Exp);
 
   const Stmt 
   ASTContext 
+  llvm::DenseMap>
+  FuncParmAnalyzer;
   llvm::DenseMap Results;
 };
 
+// A convenient wrapper around ExprMutationAnalyzer for analyzing function
+// params.
+class FunctionParmMutationAnalyzer {
+public:
+  FunctionParmMutationAnalyzer(const FunctionDecl , ASTContext );
+
+  bool isMutated(const ParmVarDecl *Parm) {
+return findMutation(Parm) != nullptr;
+  }
+  const Stmt *findMutation(const ParmVarDecl *Parm);
+
+private:
+  ExprMutationAnalyzer BodyAnalyzer;
+  llvm::DenseMap Results;
+};
+
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_ANALYSES_EXPRMUTATIONANALYZER_H

Modified: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp?rev=342271=342270=342271=diff
==
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp (original)
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp Fri Sep 14 13:07:18 2018
@@ -79,7 +79,8 @@ const Stmt *ExprMutationAnalyzer::findMu
  ::findArrayElementMutation,
  ::findCastMutation,
  ::findRangeLoopMutation,
- ::findReferenceMutation}) {
+ ::findReferenceMutation,
+ ::findFunctionArgMutation}) {
 if (const Stmt *S = (this->*Finder)(Exp))
   return Results[Exp] = S;
   }
@@ -192,10 +193,15 @@ const Stmt *ExprMutationAnalyzer::findDi
 
   // Used as non-const-ref argument when calling a function.
   // An argument is assumed to be non-const-ref when the function is 
unresolved.
+  // Instantiated template functions are not handled here but in
+  // findFunctionArgMutation which has additional smarts for handling 
forwarding
+  // references.
   const auto NonConstRefParam = forEachArgumentWithParam(
   equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(;
+  const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
   const auto AsNonConstRefArg = anyOf(
-  callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam),
+  callExpr(NonConstRefParam, NotInstantiated),
+  cxxConstructExpr(NonConstRefParam, NotInstantiated),

[PATCH] D52118: [Loopinfo] Remove one latch case in getLoopID. NFC.

2018-09-14 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur created this revision.
Meinersbur added reviewers: hfinkel, jdoerfert.

getLoopID has different control flow for two cases: If there is a single loop 
latch and for any other number of loop latches (0 and more than one). The 
latter case should return the same result if there is a single latch. We can 
save an iteration over the loop's basic blocks (which is what getLoopLatch 
does) by handling both cases with the same code.


Repository:
  rC Clang

https://reviews.llvm.org/D52118

Files:
  lib/Analysis/LoopInfo.cpp


Index: lib/Analysis/LoopInfo.cpp
===
--- lib/Analysis/LoopInfo.cpp
+++ lib/Analysis/LoopInfo.cpp
@@ -213,26 +213,21 @@
 
 MDNode *Loop::getLoopID() const {
   MDNode *LoopID = nullptr;
-  if (BasicBlock *Latch = getLoopLatch()) {
-LoopID = Latch->getTerminator()->getMetadata(LLVMContext::MD_loop);
-  } else {
-assert(!getLoopLatch() &&
-   "The loop should have no single latch at this point");
-// Go through the latch blocks and check the terminator for the metadata.
-SmallVector LatchesBlocks;
-getLoopLatches(LatchesBlocks);
-for (BasicBlock *BB : LatchesBlocks) {
-  TerminatorInst *TI = BB->getTerminator();
-  MDNode *MD = TI->getMetadata(LLVMContext::MD_loop);
-
-  if (!MD)
-return nullptr;
-
-  if (!LoopID)
-LoopID = MD;
-  else if (MD != LoopID)
-return nullptr;
-}
+
+  // Go through the latch blocks and check the terminator for the metadata.
+  SmallVector LatchesBlocks;
+  getLoopLatches(LatchesBlocks);
+  for (BasicBlock *BB : LatchesBlocks) {
+TerminatorInst *TI = BB->getTerminator();
+MDNode *MD = TI->getMetadata(LLVMContext::MD_loop);
+
+if (!MD)
+  return nullptr;
+
+if (!LoopID)
+  LoopID = MD;
+else if (MD != LoopID)
+  return nullptr;
   }
   if (!LoopID || LoopID->getNumOperands() == 0 ||
   LoopID->getOperand(0) != LoopID)


Index: lib/Analysis/LoopInfo.cpp
===
--- lib/Analysis/LoopInfo.cpp
+++ lib/Analysis/LoopInfo.cpp
@@ -213,26 +213,21 @@
 
 MDNode *Loop::getLoopID() const {
   MDNode *LoopID = nullptr;
-  if (BasicBlock *Latch = getLoopLatch()) {
-LoopID = Latch->getTerminator()->getMetadata(LLVMContext::MD_loop);
-  } else {
-assert(!getLoopLatch() &&
-   "The loop should have no single latch at this point");
-// Go through the latch blocks and check the terminator for the metadata.
-SmallVector LatchesBlocks;
-getLoopLatches(LatchesBlocks);
-for (BasicBlock *BB : LatchesBlocks) {
-  TerminatorInst *TI = BB->getTerminator();
-  MDNode *MD = TI->getMetadata(LLVMContext::MD_loop);
-
-  if (!MD)
-return nullptr;
-
-  if (!LoopID)
-LoopID = MD;
-  else if (MD != LoopID)
-return nullptr;
-}
+
+  // Go through the latch blocks and check the terminator for the metadata.
+  SmallVector LatchesBlocks;
+  getLoopLatches(LatchesBlocks);
+  for (BasicBlock *BB : LatchesBlocks) {
+TerminatorInst *TI = BB->getTerminator();
+MDNode *MD = TI->getMetadata(LLVMContext::MD_loop);
+
+if (!MD)
+  return nullptr;
+
+if (!LoopID)
+  LoopID = MD;
+else if (MD != LoopID)
+  return nullptr;
   }
   if (!LoopID || LoopID->getNumOperands() == 0 ||
   LoopID->getOperand(0) != LoopID)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r342267 - [NFC][clangd] silence pedantic extra '; ' warning

2018-09-14 Thread Jonas Toth via cfe-commits
Author: jonastoth
Date: Fri Sep 14 12:42:37 2018
New Revision: 342267

URL: http://llvm.org/viewvc/llvm-project?rev=342267=rev
Log:
[NFC][clangd] silence pedantic extra ';' warning

Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=342267=342266=342267=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Sep 14 
12:42:37 2018
@@ -671,7 +671,7 @@ TEST(CompletionTest, CompletionInPreambl
 )cpp")
  .Completions;
   EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
-};
+}
 
 TEST(CompletionTest, DynamicIndexMultiFile) {
   MockFSProvider FS;


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


[PATCH] D51789: [clang] Add the no_extern_template attribute

2018-09-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 165569.
ldionne added a comment.

Change no_extern_template to exclude_from_explicit_instantiation


Repository:
  rC Clang

https://reviews.llvm.org/D51789

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  
clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.dont_assume_extern_instantiation.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.diagnose_on_undefined_entity.cpp
  
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.explicit_instantiation.cpp
  
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
  
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp

Index: clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Test that we properly merge the exclude_from_explicit_instantiation
+// attribute on redeclarations.
+
+#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))
+
+template 
+struct Foo {
+  // Declaration without the attribute, definition with the attribute.
+  void func1();
+
+  // Declaration with the attribute, definition without the attribute.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void func2();
+
+  // Declaration with the attribute, definition with the attribute.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void func3();
+};
+
+template 
+EXCLUDE_FROM_EXPLICIT_INSTANTIATION void Foo::func1() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+void Foo::func2() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+EXCLUDE_FROM_EXPLICIT_INSTANTIATION void Foo::func3() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+struct Empty { };
+extern template struct Foo;
+
+int main() {
+  Foo foo;
+  foo.func1(); // expected-note{{in instantiation of}}
+  foo.func2(); // expected-note{{in instantiation of}}
+  foo.func3(); // expected-note{{in instantiation of}}
+}
Index: clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -Wno-unused-local-typedef -fsyntax-only -verify %s
+
+// Test that extern instantiation declarations cause members marked with
+// the exclude_from_explicit_instantiation attribute to be instantiated in
+// the current TU.
+
+#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))
+
+template 
+struct Foo {
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION inline void non_static_member_function1();
+
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void non_static_member_function2();
+
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION static inline void static_member_function1();
+
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION static void static_member_function2();
+
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION static int static_data_member;
+
+  struct EXCLUDE_FROM_EXPLICIT_INSTANTIATION member_class1 {
+static void static_member_function() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+  };
+
+  struct member_class2 {
+EXCLUDE_FROM_EXPLICIT_INSTANTIATION static void static_member_function() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+  };
+};
+
+template 
+inline void Foo::non_static_member_function1() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+void Foo::non_static_member_function2() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+inline void Foo::static_member_function1() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+void Foo::static_member_function2() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+int Foo::static_data_member = T::invalid; // expected-error{{no member named 'invalid' in 'Empty'}}
+
+struct Empty { };
+extern template struct Foo;
+
+int main() {
+  Foo foo;
+  foo.non_static_member_function1();   // expected-note{{in instantiation of}}
+  foo.non_static_member_function2();   // 

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 165568.
JonasToth added a comment.

- fix actually use clang-analyses correctly


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp

Index: test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,563 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void nested_scopes() {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  int np_local0 = 42;
+
+  {
+int p_local1 = 42;
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const'
+np_local0 *= 2;
+  }
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = _local0;
+  int *const p1_np_local0 = _local0;
+
+  int np_local1 = 42;
+  const int *const p0_np_local1 = _local1;
+  int *const p1_np_local1 = _local1;
+  *p1_np_local0 = 43;
+
+  int np_local2 = 42;
+  function_inout_pointer(_local2);
+
+  // Prevents const.
+  int np_local3 = 42;
+  out = _local3; // This returns and invalid address, its just about the AST
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+  const int *const p0_p_local1 = _local1;
+
+  int p_local2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared 'const'
+  function_in_pointer(_local2);
+}
+
+void function_inout_ref(int );
+void function_in_ref(const int );
+
+void some_reference_taking() {
+  int np_local0 = 42;
+  const int _np_local0 = np_local0;
+  int _np_local0 = np_local0;
+  r1_np_local0 = 43;
+  const int _np_local0 = r1_np_local0;
+
+  int np_local1 = 42;
+  function_inout_ref(np_local1);
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  const int _p_local0 = p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+  function_in_ref(p_local1);
+}
+
+double *non_const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared 'const'
+  double np_local0 = 

[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:381
+FunctionParmMutationAnalyzer::findMutation(const ParmVarDecl *Parm) {
+  const auto Memoized = Results.find(Parm);
+  if (Memoized != Results.end())

shuaiwang wrote:
> JonasToth wrote:
> > Please spell out the type here
> This type is a bit cumbersome to spell out as it's an iterator. I feel it's 
> fine to keep it auto.
Oh true, iterator can be `auto`ed I would say ;)


Repository:
  rC Clang

https://reviews.llvm.org/D52008



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


LLVM buildmaster will be restarted tonight

2018-09-14 Thread Galina Kistanova via cfe-commits
 Hello everyone,

LLVM buildmaster will be updated and restarted after 6PM Pacific time today.

Thanks

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


[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

Soonish it might be able to do so ;)

Am 14.09.2018 um 17:13 schrieb Dávid Bolvanský via Phabricator:

> xbolva00 added a comment.
> 
> Yeah, it would be super useful if Clang can add const to all places where 
> possible :) love this work, great!
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D45444


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

2018-09-14 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur created this revision.
Meinersbur added reviewers: hfinkel, amusman, ABataev, tyler.nowicki.
Meinersbur added a dependency: D52116: Introduce llvm.loop.parallel_accesses 
and llvm.access.group metadata..

Instead of generating llvm.mem.parallel_loop_access metadata, generate 
llvm.access.group on instructions and llvm.loop.parallel_accesses on loops. 
Minimize the number of access groups by only creating one for loops that are 
parallel.

This is clang part of https://reviews.llvm.org/D52116.


Repository:
  rC Clang

https://reviews.llvm.org/D52117

Files:
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  test/CodeGenCXX/pragma-loop-safety-nested.cpp
  test/CodeGenCXX/pragma-loop-safety-outer.cpp
  test/CodeGenCXX/pragma-loop-safety.cpp
  test/OpenMP/for_codegen.cpp
  test/OpenMP/for_simd_codegen.cpp
  test/OpenMP/loops_explicit_clauses_codegen.cpp
  test/OpenMP/ordered_codegen.cpp
  test/OpenMP/parallel_for_simd_codegen.cpp
  test/OpenMP/schedule_codegen.cpp
  test/OpenMP/simd_codegen.cpp
  test/OpenMP/simd_metadata.c
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/taskloop_simd_codegen.cpp

Index: test/OpenMP/taskloop_simd_codegen.cpp
===
--- test/OpenMP/taskloop_simd_codegen.cpp
+++ test/OpenMP/taskloop_simd_codegen.cpp
@@ -83,17 +83,17 @@
 // CHECK: [[LB_I32:%.+]] = trunc i64 [[LB_VAL]] to i32
 // CHECK: store i32 [[LB_I32]], i32* [[CNT:%.+]],
 // CHECK: br label
-// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP1:!.+]]
+// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.access.group
 // CHECK: [[VAL_I64:%.+]] = sext i32 [[VAL]] to i64
-// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
+// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.access.group
 // CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
 // CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: store i32 %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
+// CHECK: store i32 %{{.*}}!llvm.access.group
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
 // CHECK: add nsw i32 %{{.+}}, 1
-// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP1]]
-// CHECK: br label %{{.*}}!llvm.loop [[LOOP1]]
+// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.access.group
+// CHECK: br label %{{.*}}!llvm.loop
 // CHECK: ret i32 0
 
 // CHECK: define internal i32 [[TASK2]](
@@ -113,17 +113,17 @@
 // CHECK: [[LB_I32:%.+]] = trunc i64 [[LB_VAL]] to i32
 // CHECK: store i32 [[LB_I32]], i32* [[CNT:%.+]],
 // CHECK: br label
-// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP2:!.+]]
+// CHECK: [[VAL:%.+]] = load i32, i32* [[CNT]],{{.*}}!llvm.access.group
 // CHECK: [[VAL_I64:%.+]] = sext i32 [[VAL]] to i64
-// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
+// CHECK: [[UB_VAL:%.+]] = load i64, i64* [[UB]],{{.*}}!llvm.access.group
 // CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
 // CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: store i32 %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: load i32, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
+// CHECK: store i32 %{{.*}}!llvm.access.group
+// CHECK: load i32, i32* %{{.*}}!llvm.access.group
 // CHECK: add nsw i32 %{{.+}}, 1
-// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.mem.parallel_loop_access [[LOOP2]]
-// CHECK: br label %{{.*}}!llvm.loop [[LOOP2]]
+// CHECK: store i32 %{{.+}}, i32* %{{.*}}!llvm.access.group
+// CHECK: br label %{{.*}}!llvm.loop
 // CHECK: ret i32 0
 
 // CHECK: define internal i32 [[TASK3]](
@@ -142,7 +142,7 @@
 // CHECK: [[LB_VAL:%.+]] = load i64, i64* [[LB]],
 // CHECK: store i64 [[LB_VAL]], i64* [[CNT:%.+]],
 // CHECK: br label
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: br label %{{.*}}!llvm.loop
 // CHECK: ret i32 0
 
@@ -192,14 +192,14 @@
 // CHECK: [[CMP:%.+]] = icmp ule i64 [[VAL_I64]], [[UB_VAL]]
 // CHECK: br i1 [[CMP]], label %{{.+}}, label %{{.+}}
 // CHECK: load i32, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: store i32 %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: load i32, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // CHECK: add nsw i32 %{{.+}}, 1
 // CHECK: store i32 %{{.+}}, i32* %
-// CHECK-NOT: !llvm.mem.parallel_loop_access
+// CHECK-NOT: !llvm.access.group
 // 

[PATCH] D52113: Generate unique identifiers for Decl objects

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov created this revision.
george.karpenkov added reviewers: NoQ, rsmith, aprantl.

https://reviews.llvm.org/D52113

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp


Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -930,6 +930,13 @@
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
+int64_t Decl::getID() const {
+  Optional Out = getASTContext().getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Decl) == 0 && "Wrong alignment information");
+  return *Out / alignof(Decl);
+}
+
 const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
   QualType Ty;
   if (const auto *D = dyn_cast(this))
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1141,6 +1141,9 @@
 
   void dump(raw_ostream , bool Deserialize = false) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID() const;
+
   /// Looks through the Decl's underlying type to extract a FunctionType
   /// when possible. Will return null if the type underlying the Decl does not
   /// have a FunctionType.


Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -930,6 +930,13 @@
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
+int64_t Decl::getID() const {
+  Optional Out = getASTContext().getAllocator().identifyObject(this);
+  assert(Out && "Wrong allocator used");
+  assert(*Out % alignof(Decl) == 0 && "Wrong alignment information");
+  return *Out / alignof(Decl);
+}
+
 const FunctionType *Decl::getFunctionType(bool BlocksToo) const {
   QualType Ty;
   if (const auto *D = dyn_cast(this))
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -1141,6 +1141,9 @@
 
   void dump(raw_ostream , bool Deserialize = false) const;
 
+  /// \return Unique reproducible object identifier
+  int64_t getID() const;
+
   /// Looks through the Decl's underlying type to extract a FunctionType
   /// when possible. Will return null if the type underlying the Decl does not
   /// have a FunctionType.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r342261 - [clangd] Work around compiler macro expansion bugs(?) in completion tests

2018-09-14 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Fri Sep 14 11:49:16 2018
New Revision: 342261

URL: http://llvm.org/viewvc/llvm-project?rev=342261=rev
Log:
[clangd] Work around compiler macro expansion bugs(?) in completion tests

Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=342261=342260=342261=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Sep 14 
11:49:16 2018
@@ -662,15 +662,15 @@ TEST(CompletionTest, IndexSuppressesPrea
 // try to complete inside it, clang kicks our completion point just outside the
 // preamble, resulting in always getting top-level completions.
 TEST(CompletionTest, CompletionInPreamble) {
-  EXPECT_THAT(completions(R"cpp(
+  auto Results = completions(R"cpp(
 #ifnd^ef FOO_H_
 #define BAR_H_
 #include 
 int foo() {}
 #endif
 )cpp")
-  .Completions,
-  ElementsAre(Named("ifndef")));
+ .Completions;
+  EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
 };
 
 TEST(CompletionTest, DynamicIndexMultiFile) {


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


[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

2018-09-14 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment.

In https://reviews.llvm.org/D52008#1234828, @JonasToth wrote:

> The `std::move` as cast is a follow up patch?


Yes I'll send a follow up patch.




Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:381
+FunctionParmMutationAnalyzer::findMutation(const ParmVarDecl *Parm) {
+  const auto Memoized = Results.find(Parm);
+  if (Memoized != Results.end())

JonasToth wrote:
> Please spell out the type here
This type is a bit cumbersome to spell out as it's an iterator. I feel it's 
fine to keep it auto.


Repository:
  rC Clang

https://reviews.llvm.org/D52008



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


[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

2018-09-14 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang updated this revision to Diff 165552.
shuaiwang marked 8 inline comments as done.
shuaiwang added a comment.

Addressed review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D52008

Files:
  include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  lib/Analysis/ExprMutationAnalyzer.cpp
  unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -595,6 +595,96 @@
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
+TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) {
+  auto AST =
+  tooling::buildASTFromCode("template  void g(T&& t) { t = 10; }"
+"void f() { int x; g(x); }");
+  auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "void h(int&);"
+  "template  void g(Args&&... args) { h(args...); }"
+  "void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+  "void h(int&, int);"
+  "template  void g(Args&&... args) { h(args...); }"
+  "void f() { int x, y; g(x, y); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x, y)"));
+  Results = match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "void h(int, int&);"
+  "template  void g(Args&&... args) { h(args...); }"
+  "void f() { int x, y; g(y, x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(y, x)"));
+  Results = match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "struct S { template  S(T&& t) { t = 10; } };"
+  "void f() { int x; S s(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
+  AST = tooling::buildASTFromCode(
+  "struct S { template  S(T&& t) : m(++t) { } int m; };"
+  "void f() { int x; S s(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+}
+
+TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
+  auto AST = tooling::buildASTFromCode("template  void g(T&&) {}"
+   "void f() { int x; g(x); }");
+  auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode("template  void g(T&& t) { t; }"
+  "void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST =
+  tooling::buildASTFromCode("template  void g(Args&&...) {}"
+"void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST =
+  tooling::buildASTFromCode("template  void g(Args&&...) {}"
+"void f() { int y, x; g(y, x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "void h(int, int&);"
+  "template  void g(Args&&... args) { h(args...); }"
+  "void f() { int x, y; g(x, y); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "struct S { template  S(T&& t) { t; } };"
+  "void f() { int x; S s(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  "struct S { template  S(T&& t) : m(t) { } int m; };"
+  "void f() { int x; S s(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
 TEST(ExprMutationAnalyzerTest, ArrayElementModified) {
   const auto AST =
   tooling::buildASTFromCode("void f() { int x[2]; x[0] = 10; }");
Index: lib/Analysis/ExprMutationAnalyzer.cpp

[clang-tools-extra] r342252 - [modernize-use-transparent-functors] TypeLocs can be implicitly created, don't crash when encountering those.

2018-09-14 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Fri Sep 14 11:05:30 2018
New Revision: 342252

URL: http://llvm.org/viewvc/llvm-project?rev=342252=rev
Log:
[modernize-use-transparent-functors] TypeLocs can be implicitly created, don't 
crash when encountering those.

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-transparent-functors.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp?rev=342252=342251=342252=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp 
Fri Sep 14 11:05:30 2018
@@ -121,6 +121,8 @@ void UseTransparentFunctorsCheck::check(
 return;
 
   SourceLocation ReportLoc = FunctorLoc.getLocation();
+  if (ReportLoc.isInvalid())
+return;
   diag(ReportLoc, Message) << (FuncClass->getName() + "<>").str()
<< FixItHint::CreateRemoval(
   
FunctorTypeLoc.getArgLoc(0).getSourceRange());

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-transparent-functors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-transparent-functors.cpp?rev=342252=342251=342252=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-transparent-functors.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-transparent-functors.cpp 
Fri Sep 14 11:05:30 2018
@@ -104,4 +104,7 @@ int main() {
   std::set2 control;
 }
 
-
+struct ImplicitTypeLoc : std::set2> {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: prefer transparent functors
+  ImplicitTypeLoc() {}
+};


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


[PATCH] D51789: [clang] Add the no_extern_template attribute

2018-09-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

In https://reviews.llvm.org/D51789#1235113, @rsmith wrote:

> In https://reviews.llvm.org/D51789#1235100, @ldionne wrote:
>
> > In https://reviews.llvm.org/D51789#1235096, @rsmith wrote:
> >
> > > OK, so the semantics of this attribute are "explicit instantiation 
> > > declarations or definitions applied to the enclosing class do not apply 
> > > to this member", right? So it opts the member out of both `extern 
> > > template` and (non-`extern`) `template`, but only when applied to an 
> > > enclosing class.
> >
> >
> > Yes, but it also (obviously) opts the member out when applied to the member 
> > itself, not only to an enclosing class.
>
>
> Assuming I'm understanding you correctly, I don't think the current 
> implementation does that (nor does the documentation change say that), and I 
> think that's a feature not a bug. For example, given:
>
>   template struct A {
> [[clang::no_extern_template]] void f() {}
> void g() {}
>   };
>   template struct A; // instantiates A and definition of 
> A::g(), does not instantiate definition of A::f()
>   template void A::f(); // instantiates definition of A::f() 
> despite attribute
>
>
> ... the attribute affects the first explicit instantiation but not the second 
> (I think you're saying it should affect both, and make the second explicit 
> instantiation have no effect). I think the current behavior in this patch is 
> appropriate: making the attribute also affect the second case makes it 
> strictly less useful. [...]


Sorry, that's a misunderstanding. We're on the same page. What I meant is that 
if you apply the attribute to the entity itself but explicitly instantiate the 
enclosing class, then the entity is not instantiated. It's very obvious, but 
your previous comment:

> So it opts the member out of both `extern template` and (non-`extern`) 
> `template`, but only when applied to an enclosing class.

suggested that it only applied in the following case:

  template  struct Foo {
struct __attribute__((no_extern_template)) nested { static void f() { } };
  };
  
  template struct Foo; // Foo::nested::f not instantiated


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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


[PATCH] D43783: [OpenCL] Remove block invoke function from emitted block literal struct

2018-09-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In https://reviews.llvm.org/D43783#1235090, @Anastasia wrote:

> Ping! Do you still plan to do this? :)


Sorry I caught up in something else. Since there are some subsequent commits, 
it may take some efforts to revert it.


Repository:
  rC Clang

https://reviews.llvm.org/D43783



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


[PATCH] D51946: [analyzer] Remove PseudoConstantAnalysis

2018-09-14 Thread Shuai Wang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342246: Remove PseudoConstantAnalysis (authored by 
shuaiwang, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51946

Files:
  cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
  cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
  cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
  cfe/trunk/lib/Analysis/CMakeLists.txt
  cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp

Index: cfe/trunk/lib/Analysis/CMakeLists.txt
===
--- cfe/trunk/lib/Analysis/CMakeLists.txt
+++ cfe/trunk/lib/Analysis/CMakeLists.txt
@@ -23,7 +23,6 @@
   PostOrderCFGView.cpp
   PrintfFormatString.cpp
   ProgramPoint.cpp
-  PseudoConstantAnalysis.cpp
   ReachableCode.cpp
   ScanfFormatString.cpp
   ThreadSafety.cpp
Index: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
===
--- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
+++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
@@ -27,7 +27,6 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
-#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
 #include "clang/Analysis/BodyFarm.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/CFGStmtMap.h"
@@ -292,12 +291,6 @@
   return *PM;
 }
 
-PseudoConstantAnalysis *AnalysisDeclContext::getPseudoConstantAnalysis() {
-  if (!PCA)
-PCA.reset(new PseudoConstantAnalysis(getBody()));
-  return PCA.get();
-}
-
 AnalysisDeclContext *AnalysisDeclContextManager::getContext(const Decl *D) {
   if (const auto *FD = dyn_cast(D)) {
 // Calling 'hasBody' replaces 'FD' in place with the FunctionDecl
Index: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
===
--- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
+++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
@@ -40,7 +40,6 @@
 class LocationContext;
 class LocationContextManager;
 class ParentMap;
-class PseudoConstantAnalysis;
 class StackFrameContext;
 class Stmt;
 class VarDecl;
@@ -84,7 +83,6 @@
   bool builtCFG = false;
   bool builtCompleteCFG = false;
   std::unique_ptr PM;
-  std::unique_ptr PCA;
   std::unique_ptr CFA;
 
   llvm::BumpPtrAllocator A;
@@ -175,7 +173,6 @@
   bool isCFGBuilt() const { return builtCFG; }
 
   ParentMap ();
-  PseudoConstantAnalysis *getPseudoConstantAnalysis();
 
   using referenced_decls_iterator = const VarDecl * const *;
 
Index: cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
===
--- cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
+++ cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
@@ -1,226 +0,0 @@
-//== PseudoConstantAnalysis.cpp - Find Pseudoconstants in the AST-*- C++ -*-==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-// This file tracks the usage of variables in a Decl body to see if they are
-// never written to, implying that they constant. This is useful in static
-// analysis to see if a developer might have intended a variable to be const.
-//
-//===--===//
-
-#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
-#include "clang/AST/Decl.h"
-#include "clang/AST/Expr.h"
-#include "clang/AST/Stmt.h"
-#include "llvm/ADT/SmallPtrSet.h"
-#include 
-
-using namespace clang;
-
-typedef llvm::SmallPtrSet VarDeclSet;
-
-PseudoConstantAnalysis::PseudoConstantAnalysis(const Stmt *DeclBody) :
-  DeclBody(DeclBody), Analyzed(false) {
-  NonConstantsImpl = new VarDeclSet;
-  UsedVarsImpl = new VarDeclSet;
-}
-
-PseudoConstantAnalysis::~PseudoConstantAnalysis() {
-  delete (VarDeclSet*)NonConstantsImpl;
-  delete (VarDeclSet*)UsedVarsImpl;
-}
-
-// Returns true if the given ValueDecl is never written to in the given DeclBody
-bool PseudoConstantAnalysis::isPseudoConstant(const VarDecl *VD) {
-  // Only local and static variables can be pseudoconstants
-  if (!VD->hasLocalStorage() && !VD->isStaticLocal())
-return false;
-
-  if (!Analyzed) {
-RunAnalysis();
-Analyzed = true;
-  }
-
-  VarDeclSet *NonConstants = (VarDeclSet*)NonConstantsImpl;
-
-  return !NonConstants->count(VD);
-}
-
-// Returns true if the variable was used (self assignments don't count)
-bool PseudoConstantAnalysis::wasReferenced(const VarDecl *VD) {
-  if (!Analyzed) {
-RunAnalysis();
-Analyzed = true;
-  }
-
-  VarDeclSet *UsedVars = (VarDeclSet*)UsedVarsImpl;
-
-  return UsedVars->count(VD);
-}
-
-// Returns a Decl from a 

r342246 - Remove PseudoConstantAnalysis

2018-09-14 Thread Shuai Wang via cfe-commits
Author: shuaiwang
Date: Fri Sep 14 10:27:27 2018
New Revision: 342246

URL: http://llvm.org/viewvc/llvm-project?rev=342246=rev
Log:
Remove PseudoConstantAnalysis

Summary: It's not used anywhere for years. The last usage is removed in 
https://reviews.llvm.org/rL198476 in 2014.

Subscribers: mgorny, cfe-commits

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

Removed:
cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
Modified:
cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
cfe/trunk/lib/Analysis/CMakeLists.txt

Removed: cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h?rev=342245=auto
==
--- cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h 
(original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h (removed)
@@ -1,45 +0,0 @@
-//== PseudoConstantAnalysis.h - Find Pseudo-constants in the AST -*- C++ 
-*-==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-// This file tracks the usage of variables in a Decl body to see if they are
-// never written to, implying that they constant. This is useful in static
-// analysis to see if a developer might have intended a variable to be const.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_PSEUDOCONSTANTANALYSIS_H
-#define LLVM_CLANG_ANALYSIS_ANALYSES_PSEUDOCONSTANTANALYSIS_H
-
-#include "clang/AST/Stmt.h"
-
-namespace clang {
-
-class PseudoConstantAnalysis {
-public:
-  PseudoConstantAnalysis(const Stmt *DeclBody);
-  ~PseudoConstantAnalysis();
-
-  bool isPseudoConstant(const VarDecl *VD);
-  bool wasReferenced(const VarDecl *VD);
-
-private:
-  void RunAnalysis();
-  inline static const Decl *getDecl(const Expr *E);
-
-  // for storing the result of analyzed ValueDecls
-  void *NonConstantsImpl;
-  void *UsedVarsImpl;
-
-  const Stmt *DeclBody;
-  bool Analyzed;
-};
-
-}
-
-#endif

Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=342246=342245=342246=diff
==
--- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original)
+++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Fri Sep 14 10:27:27 
2018
@@ -40,7 +40,6 @@ class ImplicitParamDecl;
 class LocationContext;
 class LocationContextManager;
 class ParentMap;
-class PseudoConstantAnalysis;
 class StackFrameContext;
 class Stmt;
 class VarDecl;
@@ -84,7 +83,6 @@ class AnalysisDeclContext {
   bool builtCFG = false;
   bool builtCompleteCFG = false;
   std::unique_ptr PM;
-  std::unique_ptr PCA;
   std::unique_ptr CFA;
 
   llvm::BumpPtrAllocator A;
@@ -175,7 +173,6 @@ public:
   bool isCFGBuilt() const { return builtCFG; }
 
   ParentMap ();
-  PseudoConstantAnalysis *getPseudoConstantAnalysis();
 
   using referenced_decls_iterator = const VarDecl * const *;
 

Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=342246=342245=342246=diff
==
--- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
+++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Fri Sep 14 10:27:27 2018
@@ -27,7 +27,6 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
-#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
 #include "clang/Analysis/BodyFarm.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/CFGStmtMap.h"
@@ -292,12 +291,6 @@ ParentMap ::getParen
   return *PM;
 }
 
-PseudoConstantAnalysis *AnalysisDeclContext::getPseudoConstantAnalysis() {
-  if (!PCA)
-PCA.reset(new PseudoConstantAnalysis(getBody()));
-  return PCA.get();
-}
-
 AnalysisDeclContext *AnalysisDeclContextManager::getContext(const Decl *D) {
   if (const auto *FD = dyn_cast(D)) {
 // Calling 'hasBody' replaces 'FD' in place with the FunctionDecl

Modified: cfe/trunk/lib/Analysis/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CMakeLists.txt?rev=342246=342245=342246=diff
==
--- cfe/trunk/lib/Analysis/CMakeLists.txt (original)
+++ cfe/trunk/lib/Analysis/CMakeLists.txt Fri Sep 14 10:27:27 2018
@@ 

[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment.

This looks very useful!


Repository:
  rC Clang

https://reviews.llvm.org/D52008



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


[PATCH] D51946: [analyzer] Remove PseudoConstantAnalysis

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

Great, thanks! Seems that your code is a better version of this check.


Repository:
  rC Clang

https://reviews.llvm.org/D51946



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


[PATCH] D51789: [clang] Add the no_extern_template attribute

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In https://reviews.llvm.org/D51789#1235100, @ldionne wrote:

> In https://reviews.llvm.org/D51789#1235096, @rsmith wrote:
>
> > OK, so the semantics of this attribute are "explicit instantiation 
> > declarations or definitions applied to the enclosing class do not apply to 
> > this member", right? So it opts the member out of both `extern template` 
> > and (non-`extern`) `template`, but only when applied to an enclosing class.
>
>
> Yes, but it also (obviously) opts the member out when applied to the member 
> itself, not only to an enclosing class.


Assuming I'm understanding you correctly, I don't think the current 
implementation does that (nor does the documentation change say that), and I 
think that's a feature not a bug. For example, given:

  template struct A {
[[clang::no_extern_template]] void f() {}
void g() {}
  };
  template struct A; // instantiates A and definition of A::g(), 
does not instantiate definition of A::f()
  template void A::f(); // instantiates definition of A::f() despite 
attribute

... the attribute affects the first explicit instantiation but not the second 
(I think you're saying it should affect both, and make the second explicit 
instantiation have no effect). I think the current behavior in this patch is 
appropriate: making the attribute also affect the second case makes it strictly 
less useful. If you didn't want to actually explictly instantiate 
`A::f()`, you can just leave out that explicit instantiation. But the 
current implementation gives you extra capabilities:

  // A::g() is instantiated somewhere else, A::f() still needs to 
be implicitly instantiated when used
  extern template struct A;
  
  // A::f() and A::g() are both instantiated somewhere else
  extern template struct A;
  extern template void A::f();

You can't do that if an explicit instantiation of a member with the attribute 
has no effect.


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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


[PATCH] D52036: [Analyzer] Use diff_plist in tests, NFC

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

Thanks!
A substitution would probably need to be defined in a different file though.


https://reviews.llvm.org/D52036



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


[PATCH] D51789: [clang] Add the no_extern_template attribute

2018-09-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

In https://reviews.llvm.org/D51789#1235096, @rsmith wrote:

> In https://reviews.llvm.org/D51789#1234903, @ldionne wrote:
>
> > I think now's a good time to bikeshed the name of the attribute if you have 
> > other suggestions.
>
>
> OK, so the semantics of this attribute are "explicit instantiation 
> declarations or definitions applied to the enclosing class do not apply to 
> this member", right? So it opts the member out of both `extern template` and 
> (non-`extern`) `template`, but only when applied to an enclosing class.


Yes, but it also (obviously) opts the member out when applied to the member 
itself, not only to an enclosing class.

> Maybe something like `exclude_from_explicit_instantiation`? That's a little 
> longer than I'd like, but if it's going to be hidden behind a macro most of 
> the time I could live with it.

I like this a lot too. I thought about that one and was deterred by the length, 
but it may be acceptable. Realistically, it's also not an attribute we want 
people to use most of the time.

> Or maybe `no_transitive_instantiation`? That's less explicit about the 
> purpose of the attribute, but is a more comfortable length.

I'm not sure this one describes what the attribute is doing sufficiently 
closely.

Unless we get a better suggestion soonish, I'll change it to 
`exclude_from_explicit_instantiation` and update the diff.


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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


[PATCH] D51789: [clang] Add the no_extern_template attribute

2018-09-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In https://reviews.llvm.org/D51789#1234903, @ldionne wrote:

> I think now's a good time to bikeshed the name of the attribute if you have 
> other suggestions.


OK, so the semantics of this attribute are "explicit instantiation declarations 
or definitions applied to the enclosing class do not apply to this member", 
right? So it opts the member out of both `extern template` and (non-`extern`) 
`template`, but only when applied to an enclosing class.

Maybe something like `exclude_from_explicit_instantiation`? That's a little 
longer than I'd like, but if it's going to be hidden behind a macro most of the 
time I could live with it.
Or maybe `no_transitive_instantiation`? That's less explicit about the purpose 
of the attribute, but is a more comfortable length.


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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


[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins

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

LGTM!


https://reviews.llvm.org/D51411



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


[PATCH] D43783: [OpenCL] Remove block invoke function from emitted block literal struct

2018-09-14 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

Ping! Do you still plan to do this? :)


Repository:
  rC Clang

https://reviews.llvm.org/D43783



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


[PATCH] D52058: Add Parameters to DW_AT_name Attribute of Template Variables

2018-09-14 Thread Matthew Voss via Phabricator via cfe-commits
ormris added inline comments.



Comment at: lib/CodeGen/CGDebugInfo.cpp:3126
+  } else {
+templateParameters = nullptr;//llvm::DINodeArray().get();
+  }

JDevlieghere wrote:
> What's the meaning of this comment? 
Hmm... That should be removed.



Comment at: lib/CodeGen/CGDebugInfo.h:654
+   StringRef ,
+   llvm::MDTuple *,
+   llvm::DIScope *);

JDevlieghere wrote:
> s/templateParameters/TemplateParameters/ (same for the rest of this patch)
OK. Will fix.


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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


[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

2018-09-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment.

In https://reviews.llvm.org/D52078#1234667, @ilya-biryukov wrote:

> ~1% increase in memory usage seems totally fine. Actually surprised it's that 
> small.


Tested on a larger file: it's ~5% for `ClangdServer.cpp`. I think it's still 
worth it for the speedup.




Comment at: clangd/index/FileIndex.cpp:84
+Merged.insert(Macro);
+  for (const auto  : Collector.takeSymbols())
+Merged.insert(Sym);

ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > Why make an extra copy of the symbols? Why not add macros to the same 
> > > builder used in collector?
> > `index::indexTopLevelDecls` will re-`initialize` the collector. This is 
> > safe with the current implementation, but I can imagine it goes wrong when 
> > we do more cleanup in the initialization.
> Sorry, missed the comment about it.
> And if we do this after `indexTopLevelDecls`, than we'll be calling after the 
> `finish` call. **Sigh**...
> 
> Doing an extra copy of the symbols here is wasteful and makes the code more 
> complicated than it should be.
> 
> We have alternatives that could fix this:
> 1. Move `IndexMacro` logic into `index::indexTopLevelDecls`
> 2. Move macro collection to `SymbolCollector::finish()`
> 3. Extract a function that creates a symbol for macro definition, so we don't 
> need to call `handleMacroOccurence` and add can add extra macro symbols after 
> `SymbolCollector` finishes.
> 
> 
> (1) seems to be the cleanest option, WDYT?
(1) sounds good. D52098


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078



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


[PATCH] D52078: [clangd] Store preamble macros in dynamic index.

2018-09-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 165517.
ioeric added a comment.

- Move macro collection to indexTopLevelDecls.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078

Files:
  clangd/ClangdServer.cpp
  clangd/FindSymbols.cpp
  clangd/XRefs.cpp
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -51,7 +51,7 @@
 std::unique_ptr TestTU::index() const {
   auto AST = build();
   auto Content = indexAST(AST.getASTContext(), AST.getPreprocessorPtr(),
-  AST.getLocalTopLevelDecls());
+  /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
   return MemIndex::build(std::move(Content.first), std::move(Content.second));
 }
 
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -244,7 +244,7 @@
   Test.Filename = "test.cc";
   auto AST = Test.build();
   Dyn.update(Test.Filename, (), AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
 
   // Build static index for test.cc.
   Test.HeaderCode = HeaderCode;
@@ -254,7 +254,7 @@
   // Add stale refs for test.cc.
   StaticIndex.update(Test.Filename, (),
  StaticAST.getPreprocessorPtr(),
- StaticAST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, StaticAST.getLocalTopLevelDecls());
 
   // Add refs for test2.cc
   Annotations Test2Code(R"(class $Foo[[Foo]] {};)");
@@ -265,7 +265,7 @@
   StaticAST = Test2.build();
   StaticIndex.update(Test2.Filename, (),
  StaticAST.getPreprocessorPtr(),
- StaticAST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, StaticAST.getLocalTopLevelDecls());
 
   RefsRequest Request;
   Request.IDs = {Foo.ID};
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -15,6 +15,7 @@
 #include "index/FileIndex.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "gtest/gtest.h"
@@ -135,7 +136,8 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.update(File.Filename, (), AST.getPreprocessorPtr());
+  M.update(File.Filename, (), AST.getPreprocessorPtr(),
+   /*IndexMacros=*/true);
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
@@ -333,23 +335,38 @@
   Test.Filename = "test.cc";
   auto AST = Test.build();
   Index.update(Test.Filename, (), AST.getPreprocessorPtr(),
-   AST.getLocalTopLevelDecls());
+   /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
   // Add test2.cc
   TestTU Test2;
   Test2.HeaderCode = HeaderCode;
   Test2.Code = MainCode.code();
   Test2.Filename = "test2.cc";
   AST = Test2.build();
   Index.update(Test2.Filename, (), AST.getPreprocessorPtr(),
-   AST.getLocalTopLevelDecls());
+   /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
 
   EXPECT_THAT(getRefs(Index, Foo.ID),
   RefsAre({AllOf(RefRange(MainCode.range("foo")),
  FileURI("unittest:///test.cc")),
AllOf(RefRange(MainCode.range("foo")),
  FileURI("unittest:///test2.cc"))}));
 }
 
+TEST(FileIndexTest, CollectMacros) {
+  FileIndex M;
+  update(M, "f", "#define CLANGD 1");
+
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  bool SeenSymbol = false;
+  M.fuzzyFind(Req, [&](const Symbol ) {
+EXPECT_EQ(Sym.Name, "CLANGD");
+EXPECT_EQ(Sym.SymInfo.Kind, index::SymbolKind::Macro);
+SeenSymbol = true;
+  });
+  EXPECT_TRUE(SeenSymbol);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -223,12 +223,13 @@
 void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
 
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -276,11 +277,12 @@
 void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
   

r342240 - [clang-cl] Fix PR38934: failing to dllexport class template member w/ explicit instantiation and PCH

2018-09-14 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Fri Sep 14 08:18:30 2018
New Revision: 342240

URL: http://llvm.org/viewvc/llvm-project?rev=342240=rev
Log:
[clang-cl] Fix PR38934: failing to dllexport class template member w/ explicit 
instantiation and PCH

The code in ASTContext::DeclMustBeEmitted was supposed to handle this,
but didn't take into account that synthesized members such as operator=
might not get marked as template specializations, because they're
synthesized on the instantiation directly when handling the class-level
dllexport attribute.

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/test/CodeGen/pch-dllexport.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=342240=342239=342240=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Fri Sep 14 08:18:30 2018
@@ -9724,6 +9724,14 @@ bool ASTContext::DeclMustBeEmitted(const
 cast(D)->getTemplateSpecializationKind() ==
 TSK_ExplicitInstantiationDefinition;
 
+// Implicit member function definitions, such as operator= might not be
+// marked as template specializations, since they're not coming from a
+// template but synthesized directly on the class.
+IsExpInstDef |=
+isa(D) &&
+cast(D)->getParent()->getTemplateSpecializationKind() ==
+TSK_ExplicitInstantiationDefinition;
+
 if (getExternalSource()->DeclIsFromPCHWithObjectFile(D) && !IsExpInstDef)
   return false;
   }

Modified: cfe/trunk/test/CodeGen/pch-dllexport.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pch-dllexport.cpp?rev=342240=342239=342240=diff
==
--- cfe/trunk/test/CodeGen/pch-dllexport.cpp (original)
+++ cfe/trunk/test/CodeGen/pch-dllexport.cpp Fri Sep 14 08:18:30 2018
@@ -57,6 +57,13 @@ extern template void explicitInstantiati
 template  T __declspec(dllexport) variableTemplate;
 extern template int variableTemplate;
 
+namespace pr38934 {
+template  struct S {};
+extern template struct S;
+// The use here causes the S::operator= decl to go into the PCH.
+inline void use(S *a, S *b) { *a = *b; };
+}
+
 #else
 
 void use() {
@@ -81,4 +88,9 @@ template void __declspec(dllexport) expl
 template int __declspec(dllexport) variableTemplate;
 // PCHWITHOBJVARS: @"??$variableTemplate@H@@3HA" = weak_odr dso_local 
dllexport global
 
+// PR38934: Make sure S::operator= gets emitted. While it itself isn't a
+// template specialization, its parent is.
+template struct __declspec(dllexport) pr38934::S;
+// PCHWITHOBJ: define weak_odr dso_local dllexport x86_thiscallcc 
dereferenceable(1) %"struct.pr38934::S"* @"??4?$S@H@pr38934@@QAEAAU01@ABU01@@Z"
+
 #endif


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


[PATCH] D52098: [Index] Add an option to collect macros from preprocesor.

2018-09-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 165516.
ioeric added a comment.

- another cleanup...


Repository:
  rC Clang

https://reviews.llvm.org/D52098

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexingAction.cpp
  unittests/CMakeLists.txt
  unittests/Index/CMakeLists.txt
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- /dev/null
+++ unittests/Index/IndexTests.cpp
@@ -0,0 +1,132 @@
+//===--- IndexTests.cpp - Test indexing actions -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace index {
+
+struct TestSymbol {
+  std::string QName;
+  SymbolRoleSet Roles;
+  SymbolInfo Info;
+  // FIXME: add more information.
+};
+
+llvm::raw_ostream <<(llvm::raw_ostream , const TestSymbol ) {
+  return OS << S.QName;
+}
+
+namespace {
+class Indexer : public IndexDataConsumer {
+public:
+  bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
+   ArrayRef, SourceLocation,
+   ASTNodeInfo) override {
+const auto *ND = llvm::dyn_cast(D);
+if (!ND)
+  return true;
+TestSymbol S;
+S.QName = ND->getQualifiedNameAsString();
+S.Roles = Roles;
+S.Info = getSymbolInfo(ND);
+Symbols.push_back(std::move(S));
+return true;
+  }
+
+  bool handleMacroOccurence(const IdentifierInfo *Name, const MacroInfo *MI,
+SymbolRoleSet Roles, SourceLocation) override {
+TestSymbol S;
+S.QName = Name->getName();
+S.Roles = Roles;
+if (MI)
+  S.Info = getSymbolInfoForMacro(*MI);
+Symbols.push_back(std::move(S));
+return true;
+  }
+
+  std::vector Symbols;
+};
+
+class IndexAction : public ASTFrontendAction {
+public:
+  IndexAction(std::shared_ptr Index,
+  IndexingOptions Opts = IndexingOptions())
+  : Index(std::move(Index)), Opts(Opts) {}
+
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override {
+class Consumer : public ASTConsumer {
+  std::shared_ptr Index;
+  std::shared_ptr PP;
+  IndexingOptions Opts;
+
+public:
+  Consumer(std::shared_ptr Index, std::shared_ptr PP,
+   IndexingOptions Opts)
+  : Index(std::move(Index)), PP(std::move(PP)), Opts(Opts) {}
+
+  void HandleTranslationUnit(ASTContext ) override {
+std::vector DeclsToIndex(
+Ctx.getTranslationUnitDecl()->decls().begin(),
+Ctx.getTranslationUnitDecl()->decls().end());
+indexTopLevelDecls(Ctx, *PP, DeclsToIndex, *Index, Opts);
+  }
+};
+return llvm::make_unique(Index, CI.getPreprocessorPtr(), Opts);
+  }
+
+private:
+  std::shared_ptr Index;
+  IndexingOptions Opts;
+};
+
+using testing::Contains;
+using testing::Not;
+using testing::UnorderedElementsAre;
+
+MATCHER_P(QName, Name, "") { return arg.QName == Name; }
+
+TEST(IndexTest, Simple) {
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(new IndexAction(Index), "class X {}; void f() {}");
+  EXPECT_THAT(Index->Symbols, UnorderedElementsAre(QName("X"), QName("f")));
+}
+
+TEST(IndexTest, IndexPreprocessorMacros) {
+  std::string Code = "#define INDEX_MAC 1";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexMacrosInPreprocessor = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("INDEX_MAC")));
+
+  Opts.IndexMacrosInPreprocessor = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
+}
+
+} // namespace
+} // namespace index
+} // namespace clang
Index: unittests/Index/CMakeLists.txt
===
--- /dev/null
+++ unittests/Index/CMakeLists.txt
@@ -0,0 +1,18 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  Support
+  )
+
+add_clang_unittest(IndexTests
+  IndexTests.cpp
+  )
+
+target_link_libraries(IndexTests
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFrontend
+  clangIndex
+  clangLex
+  clangTooling
+  )
Index: unittests/CMakeLists.txt

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-09-14 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Yeah, it would be super useful if Clang can add const to all places where 
possible :) love this work, great!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D52097: [OPENMP] Move OMPClauseReader/Writer classes to ASTReader/Writer - NFC

2018-09-14 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rC Clang

https://reviews.llvm.org/D52097



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


[PATCH] D52098: [Index] Add an option to collect macros from preprocesor.

2018-09-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 165514.
ioeric added a comment.

- remove debug message.


Repository:
  rC Clang

https://reviews.llvm.org/D52098

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexingAction.cpp
  unittests/CMakeLists.txt
  unittests/Index/CMakeLists.txt
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- /dev/null
+++ unittests/Index/IndexTests.cpp
@@ -0,0 +1,132 @@
+//===--- IndexTests.cpp - Test indexing actions -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace index {
+
+struct TestSymbol {
+  std::string QName;
+  SymbolRoleSet Roles;
+  SymbolInfo Info;
+  // FIXME: add more information.
+};
+
+llvm::raw_ostream <<(llvm::raw_ostream , const TestSymbol ) {
+  return OS << S.QName;
+}
+
+namespace {
+class Indexer : public IndexDataConsumer {
+public:
+  bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
+   ArrayRef, SourceLocation,
+   ASTNodeInfo) override {
+const auto *ND = llvm::dyn_cast(D);
+if (!ND)
+  return true;
+TestSymbol S;
+S.QName = ND->getQualifiedNameAsString();
+S.Roles = Roles;
+S.Info = getSymbolInfo(ND);
+Symbols.push_back(std::move(S));
+return true;
+  }
+
+  bool handleMacroOccurence(const IdentifierInfo *Name, const MacroInfo *MI,
+SymbolRoleSet Roles, SourceLocation) override {
+TestSymbol S;
+S.QName = Name->getName();
+S.Roles = Roles;
+if (MI)
+  S.Info = getSymbolInfoForMacro(*MI);
+Symbols.push_back(std::move(S));
+return true;
+  }
+
+  std::vector Symbols;
+};
+
+class IndexAction : public ASTFrontendAction {
+public:
+  IndexAction(std::shared_ptr Index,
+  IndexingOptions Opts = IndexingOptions())
+  : Index(std::move(Index)), Opts(Opts) {}
+
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override {
+class Consumer : public ASTConsumer {
+  std::shared_ptr Index;
+  std::shared_ptr PP;
+  IndexingOptions Opts;
+
+public:
+  Consumer(std::shared_ptr Index, std::shared_ptr PP,
+   IndexingOptions Opts)
+  : Index(std::move(Index)), PP(std::move(PP)), Opts(Opts) {}
+
+  void HandleTranslationUnit(ASTContext ) override {
+std::vector DeclsToIndex(
+Ctx.getTranslationUnitDecl()->decls().begin(),
+Ctx.getTranslationUnitDecl()->decls().end());
+indexTopLevelDecls(Ctx, *PP, DeclsToIndex, *Index, Opts);
+  }
+};
+return llvm::make_unique(Index, CI.getPreprocessorPtr(), Opts);
+  }
+
+private:
+  std::shared_ptr Index;
+  IndexingOptions Opts;
+};
+
+using testing::Contains;
+using testing::Not;
+using testing::UnorderedElementsAre;
+
+MATCHER_P(QName, Name, "") { return arg.QName == Name; }
+
+TEST(IndexTest, Simple) {
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(new IndexAction(Index), "class X {}; void f() {}");
+  EXPECT_THAT(Index->Symbols, UnorderedElementsAre(QName("X"), QName("f")));
+}
+
+TEST(IndexTest, IndexPreprocessorMacros) {
+  std::string Code = "#define INDEX_MAC 1";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexMacrosInPreprocessor = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("INDEX_MAC")));
+
+  Opts.IndexMacrosInPreprocessor = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
+}
+
+} // namespace
+} // namespace index
+} // namespace clang
Index: unittests/Index/CMakeLists.txt
===
--- /dev/null
+++ unittests/Index/CMakeLists.txt
@@ -0,0 +1,18 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  Support
+  )
+
+add_clang_unittest(IndexTests
+  IndexTests.cpp
+  )
+
+target_link_libraries(IndexTests
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFrontend
+  clangIndex
+  clangLex
+  clangTooling
+  )
Index: unittests/CMakeLists.txt

[PATCH] D52098: [Index] Add an option to collect macros from preprocesor.

2018-09-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, mgorny.

Also added unit tests for the index library; lit+c-index-test is painful...


Repository:
  rC Clang

https://reviews.llvm.org/D52098

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexingAction.cpp
  unittests/CMakeLists.txt
  unittests/Index/CMakeLists.txt
  unittests/Index/IndexTests.cpp

Index: unittests/Index/IndexTests.cpp
===
--- /dev/null
+++ unittests/Index/IndexTests.cpp
@@ -0,0 +1,136 @@
+//===--- IndexTests.cpp - Test indexing actions -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace index {
+
+struct TestSymbol {
+  std::string QName;
+  SymbolRoleSet Roles;
+  SymbolInfo Info;
+  // FIXME: add more information.
+};
+
+llvm::raw_ostream <<(llvm::raw_ostream , const TestSymbol ) {
+  return OS << S.QName;
+}
+
+namespace {
+class Indexer : public IndexDataConsumer {
+public:
+  bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
+   ArrayRef, SourceLocation,
+   ASTNodeInfo) override {
+llvm::errs() << "Handle decl: \n";
+const auto *ND = llvm::dyn_cast(D);
+if (!ND)
+  return true;
+llvm::errs() << "ND: " << ND->getQualifiedNameAsString() << "\n";
+TestSymbol S;
+S.QName = ND->getQualifiedNameAsString();
+S.Roles = Roles;
+S.Info = getSymbolInfo(ND);
+Symbols.push_back(std::move(S));
+return true;
+  }
+
+  bool handleMacroOccurence(const IdentifierInfo *Name, const MacroInfo *MI,
+SymbolRoleSet Roles, SourceLocation) override {
+TestSymbol S;
+S.QName = Name->getName();
+S.Roles = Roles;
+if (MI)
+  S.Info = getSymbolInfoForMacro(*MI);
+Symbols.push_back(std::move(S));
+return true;
+  }
+
+  std::vector Symbols;
+};
+
+class IndexAction : public ASTFrontendAction {
+public:
+  IndexAction(std::shared_ptr Index,
+  IndexingOptions Opts = IndexingOptions())
+  : Index(std::move(Index)), Opts(Opts) {}
+
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override {
+class Consumer : public ASTConsumer {
+  std::shared_ptr Index;
+  std::shared_ptr PP;
+  IndexingOptions Opts;
+
+public:
+  Consumer(std::shared_ptr Index, std::shared_ptr PP,
+   IndexingOptions Opts)
+  : Index(std::move(Index)), PP(std::move(PP)), Opts(Opts) {}
+
+  void HandleTranslationUnit(ASTContext ) override {
+llvm::errs() << "Handle TU : " << Ctx.getTranslationUnitDecl() << "\n";
+Ctx.getTranslationUnitDecl()->dump();
+std::vector DeclsToIndex(
+Ctx.getTranslationUnitDecl()->decls().begin(),
+Ctx.getTranslationUnitDecl()->decls().end());
+indexTopLevelDecls(Ctx, *PP, DeclsToIndex, *Index, Opts);
+  }
+};
+return llvm::make_unique(Index, CI.getPreprocessorPtr(), Opts);
+  }
+
+private:
+  std::shared_ptr Index;
+  IndexingOptions Opts;
+};
+
+using testing::Contains;
+using testing::Not;
+using testing::UnorderedElementsAre;
+
+MATCHER_P(QName, Name, "") { return arg.QName == Name; }
+
+TEST(IndexTest, Simple) {
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(new IndexAction(Index), "class X {}; void f() {}");
+  EXPECT_THAT(Index->Symbols, UnorderedElementsAre(QName("X"), QName("f")));
+}
+
+TEST(IndexTest, IndexPreprocessorMacros) {
+  std::string Code = "#define INDEX_MAC 1";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexMacrosInPreprocessor = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("INDEX_MAC")));
+
+  Opts.IndexMacrosInPreprocessor = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
+}
+
+} // namespace
+} // namespace index
+} // namespace clang
Index: unittests/Index/CMakeLists.txt
===
--- /dev/null
+++ 

[PATCH] D52097: [OPENMP] Move OMPClauseReader/Writer classes to ASTReader/Writer - NFC

2018-09-14 Thread Patrick Lyster via Phabricator via cfe-commits
patricklyster created this revision.
patricklyster added reviewers: ABataev, Hahnfeld, RaviNarayanaswamy, mikerice, 
kkwli0, hfinkel, gtbercea.
patricklyster added projects: OpenMP, clang.
Herald added subscribers: cfe-commits, jfb, guansong.

Move declarations for `OMPClauseReader`, `OMPClauseWriter` to ASTReader.h and 
ASTWriter.h and move implementation to ASTReader.cpp and ASTWriter.cpp. This 
NFC will help generalize the serialization of OpenMP clauses and will be used 
in the future implementation of new OpenMP directives (e.g. `omp requires`).


Repository:
  rC Clang

https://reviews.llvm.org/D52097

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/StmtVisitor.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1803,485 +1803,6 @@
 }
 
 //===--===//
-// OpenMP Clauses.
-//===--===//
-
-namespace clang {
-class OMPClauseWriter : public OMPClauseVisitor {
-  ASTRecordWriter 
-public:
-  OMPClauseWriter(ASTRecordWriter ) : Record(Record) {}
-#define OPENMP_CLAUSE(Name, Class)\
-  void Visit##Class(Class *S);
-#include "clang/Basic/OpenMPKinds.def"
-  void writeClause(OMPClause *C);
-  void VisitOMPClauseWithPreInit(OMPClauseWithPreInit *C);
-  void VisitOMPClauseWithPostUpdate(OMPClauseWithPostUpdate *C);
-};
-}
-
-void OMPClauseWriter::writeClause(OMPClause *C) {
-  Record.push_back(C->getClauseKind());
-  Visit(C);
-  Record.AddSourceLocation(C->getBeginLoc());
-  Record.AddSourceLocation(C->getEndLoc());
-}
-
-void OMPClauseWriter::VisitOMPClauseWithPreInit(OMPClauseWithPreInit *C) {
-  Record.push_back(C->getCaptureRegion());
-  Record.AddStmt(C->getPreInitStmt());
-}
-
-void OMPClauseWriter::VisitOMPClauseWithPostUpdate(OMPClauseWithPostUpdate *C) {
-  VisitOMPClauseWithPreInit(C);
-  Record.AddStmt(C->getPostUpdateExpr());
-}
-
-void OMPClauseWriter::VisitOMPIfClause(OMPIfClause *C) {
-  VisitOMPClauseWithPreInit(C);
-  Record.push_back(C->getNameModifier());
-  Record.AddSourceLocation(C->getNameModifierLoc());
-  Record.AddSourceLocation(C->getColonLoc());
-  Record.AddStmt(C->getCondition());
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPFinalClause(OMPFinalClause *C) {
-  Record.AddStmt(C->getCondition());
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPNumThreadsClause(OMPNumThreadsClause *C) {
-  VisitOMPClauseWithPreInit(C);
-  Record.AddStmt(C->getNumThreads());
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPSafelenClause(OMPSafelenClause *C) {
-  Record.AddStmt(C->getSafelen());
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPSimdlenClause(OMPSimdlenClause *C) {
-  Record.AddStmt(C->getSimdlen());
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPCollapseClause(OMPCollapseClause *C) {
-  Record.AddStmt(C->getNumForLoops());
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPDefaultClause(OMPDefaultClause *C) {
-  Record.push_back(C->getDefaultKind());
-  Record.AddSourceLocation(C->getLParenLoc());
-  Record.AddSourceLocation(C->getDefaultKindKwLoc());
-}
-
-void OMPClauseWriter::VisitOMPProcBindClause(OMPProcBindClause *C) {
-  Record.push_back(C->getProcBindKind());
-  Record.AddSourceLocation(C->getLParenLoc());
-  Record.AddSourceLocation(C->getProcBindKindKwLoc());
-}
-
-void OMPClauseWriter::VisitOMPScheduleClause(OMPScheduleClause *C) {
-  VisitOMPClauseWithPreInit(C);
-  Record.push_back(C->getScheduleKind());
-  Record.push_back(C->getFirstScheduleModifier());
-  Record.push_back(C->getSecondScheduleModifier());
-  Record.AddStmt(C->getChunkSize());
-  Record.AddSourceLocation(C->getLParenLoc());
-  Record.AddSourceLocation(C->getFirstScheduleModifierLoc());
-  Record.AddSourceLocation(C->getSecondScheduleModifierLoc());
-  Record.AddSourceLocation(C->getScheduleKindLoc());
-  Record.AddSourceLocation(C->getCommaLoc());
-}
-
-void OMPClauseWriter::VisitOMPOrderedClause(OMPOrderedClause *C) {
-  Record.push_back(C->getLoopNumIterations().size());
-  Record.AddStmt(C->getNumForLoops());
-  for (Expr *NumIter : C->getLoopNumIterations())
-Record.AddStmt(NumIter);
-  for (unsigned I = 0, E = C->getLoopNumIterations().size(); I getLoopCunter(I));
-  Record.AddSourceLocation(C->getLParenLoc());
-}
-
-void OMPClauseWriter::VisitOMPNowaitClause(OMPNowaitClause *) {}
-
-void 

[PATCH] D51789: [clang] Add the no_extern_template attribute

2018-09-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

I think now's a good time to bikeshed the name of the attribute if you have 
other suggestions.


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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


[PATCH] D51789: [WIP][clang] Add the no_extern_template attribute

2018-09-14 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 165507.
ldionne added a comment.

Fix the tests and remove some warnings that I wasn't able to generate properly
(to avoid false positives).


Repository:
  rC Clang

https://reviews.llvm.org/D51789

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  
clang/test/CodeGenCXX/attr-no_extern_template.dont_assume_extern_instantiation.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-no_extern_template.diagnose_on_undefined_entity.cpp
  clang/test/SemaCXX/attr-no_extern_template.explicit_instantiation.cpp
  clang/test/SemaCXX/attr-no_extern_template.extern_declaration.cpp
  clang/test/SemaCXX/attr-no_extern_template.merge_redeclarations.cpp

Index: clang/test/SemaCXX/attr-no_extern_template.merge_redeclarations.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-no_extern_template.merge_redeclarations.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Test that we properly merge the no_extern_template attribute on
+// redeclarations.
+
+#define NO_EXTERN_TEMPLATE __attribute__((no_extern_template))
+
+template 
+struct Foo {
+  // Declaration without the attribute, definition with the attribute.
+  void func1();
+
+  // Declaration with the attribute, definition without the attribute.
+  NO_EXTERN_TEMPLATE void func2();
+
+  // Declaration with the attribute, definition with the attribute.
+  NO_EXTERN_TEMPLATE void func3();
+};
+
+template 
+NO_EXTERN_TEMPLATE void Foo::func1() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+void Foo::func2() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+NO_EXTERN_TEMPLATE void Foo::func3() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+struct Empty { };
+extern template struct Foo;
+
+int main() {
+  Foo foo;
+  foo.func1(); // expected-note{{in instantiation of}}
+  foo.func2(); // expected-note{{in instantiation of}}
+  foo.func3(); // expected-note{{in instantiation of}}
+}
Index: clang/test/SemaCXX/attr-no_extern_template.extern_declaration.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-no_extern_template.extern_declaration.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -Wno-unused-local-typedef -fsyntax-only -verify %s
+
+// Test that extern instantiation declarations cause members marked with
+// no_extern_template to be instantiated in the current TU.
+
+#define NO_EXTERN_TEMPLATE __attribute__((no_extern_template))
+
+template 
+struct Foo {
+  NO_EXTERN_TEMPLATE inline void non_static_member_function1();
+
+  NO_EXTERN_TEMPLATE void non_static_member_function2();
+
+  NO_EXTERN_TEMPLATE static inline void static_member_function1();
+
+  NO_EXTERN_TEMPLATE static void static_member_function2();
+
+  NO_EXTERN_TEMPLATE static int static_data_member;
+
+  struct NO_EXTERN_TEMPLATE member_class1 {
+static void static_member_function() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+  };
+
+  struct member_class2 {
+NO_EXTERN_TEMPLATE static void static_member_function() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+  };
+};
+
+template 
+inline void Foo::non_static_member_function1() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+void Foo::non_static_member_function2() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+inline void Foo::static_member_function1() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+void Foo::static_member_function2() {
+  using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template 
+int Foo::static_data_member = T::invalid; // expected-error{{no member named 'invalid' in 'Empty'}}
+
+struct Empty { };
+extern template struct Foo;
+
+int main() {
+  Foo foo;
+  foo.non_static_member_function1();   // expected-note{{in instantiation of}}
+  foo.non_static_member_function2();   // expected-note{{in instantiation of}}
+  Foo::static_member_function1();   // expected-note{{in instantiation of}}
+  Foo::static_member_function2();   // expected-note{{in instantiation of}}
+  (void)foo.static_data_member;// expected-note{{in instantiation of}}
+  Foo::member_class1::static_member_function(); // expected-note{{in instantiation of}}
+  

[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly

2018-09-14 Thread Louis Dionne via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342238: [clang] Make sure attributes on member classes are 
applied properly (authored by ldionne, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51997?vs=165375=165502#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51997

Files:
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/SemaCXX/PR38913.cpp


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1258,6 +1258,9 @@
   if (QualifierLoc)
 RecordInst->setQualifierInfo(QualifierLoc);
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs,
+  StartingScope);
+
   ClassTemplateDecl *Inst
 = ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(),
 D->getIdentifier(), InstParams, RecordInst);
@@ -1491,6 +1494,9 @@
   if (SubstQualifier(D, Record))
 return nullptr;
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
+  StartingScope);
+
   Record->setImplicit(D->isImplicit());
   // FIXME: Check against AS_none is an ugly hack to work around the issue that
   // the tag decls introduced by friend class declarations don't have an access
Index: test/SemaCXX/PR38913.cpp
===
--- test/SemaCXX/PR38913.cpp
+++ test/SemaCXX/PR38913.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+// PR38913
+// Check that we instantiate attributes on declarations for...
+
+// ...a member class of a class template specialization
+template struct A { struct __attribute__((abi_tag("ATAG"))) X { }; };
+A::X* a() { return 0; } // CHECK-DAG: @_Z1aB4ATAGv
+
+// ...a member class template
+template struct B { template struct 
__attribute__((abi_tag("BTAG"))) X { }; };
+B::X* b() { return 0; } // CHECK-DAG: @_Z1bB4BTAGv


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1258,6 +1258,9 @@
   if (QualifierLoc)
 RecordInst->setQualifierInfo(QualifierLoc);
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs,
+  StartingScope);
+
   ClassTemplateDecl *Inst
 = ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(),
 D->getIdentifier(), InstParams, RecordInst);
@@ -1491,6 +1494,9 @@
   if (SubstQualifier(D, Record))
 return nullptr;
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
+  StartingScope);
+
   Record->setImplicit(D->isImplicit());
   // FIXME: Check against AS_none is an ugly hack to work around the issue that
   // the tag decls introduced by friend class declarations don't have an access
Index: test/SemaCXX/PR38913.cpp
===
--- test/SemaCXX/PR38913.cpp
+++ test/SemaCXX/PR38913.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+// PR38913
+// Check that we instantiate attributes on declarations for...
+
+// ...a member class of a class template specialization
+template struct A { struct __attribute__((abi_tag("ATAG"))) X { }; };
+A::X* a() { return 0; } // CHECK-DAG: @_Z1aB4ATAGv
+
+// ...a member class template
+template struct B { template struct __attribute__((abi_tag("BTAG"))) X { }; };
+B::X* b() { return 0; } // CHECK-DAG: @_Z1bB4BTAGv
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r342238 - [clang] Make sure attributes on member classes are applied properly

2018-09-14 Thread Louis Dionne via cfe-commits
Author: ldionne
Date: Fri Sep 14 07:07:16 2018
New Revision: 342238

URL: http://llvm.org/viewvc/llvm-project?rev=342238=rev
Log:
[clang] Make sure attributes on member classes are applied properly

Summary:
Attributes on member classes of class templates and member class templates
of class templates are not currently instantiated. This was discovered by
Richard Smith here:

  http://lists.llvm.org/pipermail/cfe-dev/2018-September/059291.html

This commit makes sure that attributes are instantiated properly. This
commit does not fix the broken behavior for member partial and explicit
specializations of class templates.

PR38913

Reviewers: rsmith

Subscribers: dexonsmith, cfe-commits

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

Added:
cfe/trunk/test/SemaCXX/PR38913.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=342238=342237=342238=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Fri Sep 14 07:07:16 2018
@@ -1258,6 +1258,9 @@ Decl *TemplateDeclInstantiator::VisitCla
   if (QualifierLoc)
 RecordInst->setQualifierInfo(QualifierLoc);
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst, LateAttrs,
+  StartingScope);
+
   ClassTemplateDecl *Inst
 = ClassTemplateDecl::Create(SemaRef.Context, DC, D->getLocation(),
 D->getIdentifier(), InstParams, RecordInst);
@@ -1491,6 +1494,9 @@ Decl *TemplateDeclInstantiator::VisitCXX
   if (SubstQualifier(D, Record))
 return nullptr;
 
+  SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
+  StartingScope);
+
   Record->setImplicit(D->isImplicit());
   // FIXME: Check against AS_none is an ugly hack to work around the issue that
   // the tag decls introduced by friend class declarations don't have an access

Added: cfe/trunk/test/SemaCXX/PR38913.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR38913.cpp?rev=342238=auto
==
--- cfe/trunk/test/SemaCXX/PR38913.cpp (added)
+++ cfe/trunk/test/SemaCXX/PR38913.cpp Fri Sep 14 07:07:16 2018
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+// PR38913
+// Check that we instantiate attributes on declarations for...
+
+// ...a member class of a class template specialization
+template struct A { struct __attribute__((abi_tag("ATAG"))) X { }; };
+A::X* a() { return 0; } // CHECK-DAG: @_Z1aB4ATAGv
+
+// ...a member class template
+template struct B { template struct 
__attribute__((abi_tag("BTAG"))) X { }; };
+B::X* b() { return 0; } // CHECK-DAG: @_Z1bB4BTAGv


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


[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 165497.
JonasToth added a comment.

- update to ExprMutAnalyzer living in clang now


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp

Index: test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,563 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void nested_scopes() {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  int np_local0 = 42;
+
+  {
+int p_local1 = 42;
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const'
+np_local0 *= 2;
+  }
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = _local0;
+  int *const p1_np_local0 = _local0;
+
+  int np_local1 = 42;
+  const int *const p0_np_local1 = _local1;
+  int *const p1_np_local1 = _local1;
+  *p1_np_local0 = 43;
+
+  int np_local2 = 42;
+  function_inout_pointer(_local2);
+
+  // Prevents const.
+  int np_local3 = 42;
+  out = _local3; // This returns and invalid address, its just about the AST
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+  const int *const p0_p_local1 = _local1;
+
+  int p_local2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared 'const'
+  function_in_pointer(_local2);
+}
+
+void function_inout_ref(int );
+void function_in_ref(const int );
+
+void some_reference_taking() {
+  int np_local0 = 42;
+  const int _np_local0 = np_local0;
+  int _np_local0 = np_local0;
+  r1_np_local0 = 43;
+  const int _np_local0 = r1_np_local0;
+
+  int np_local1 = 42;
+  function_inout_ref(np_local1);
+
+  int p_local0 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  const int _p_local0 = p_local0;
+
+  int p_local1 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+  function_in_ref(p_local1);
+}
+
+double *non_const_pointer_return() {
+  double p_local0 = 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared 'const'
+  double np_local0 

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked 9 inline comments as done.
JonasToth added a comment.

I do consider the diagnostic thing as resolved given the lack of further 
comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714



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


[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

LG from my side, please rebase that patch on top of master. Currently the 
Release Notes would conflict (and please sort the release note alphabetically).

If the other reviewers do not say anything within a reasonable time it can be 
committed. Do you have commit rights?


https://reviews.llvm.org/D51332



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


[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

2018-09-14 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

And i forgot: Could you please run this over a real code base, if you know one 
that actually uses these types?
I assume not so many code bases actually use these.


https://reviews.llvm.org/D51332



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


  1   2   >