[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-08-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In D65410#1613555 , @hfinkel wrote:

> Thanks for starting on this. Can you go ahead and replace the sroa calls with 
> mem2reg calls for `O1` and then see what that does to the performance? That 
> strikes me as a major change, but certainly one that potentially makes sense, 
> so I'd rather we go ahead and test it now before we make decisions about 
> other adjustments.


I'll give it a shot. I think we'll want it in the long run, but happy to run it 
through the performance blender just to get an idea of what we're getting for 
our complexity.

> FWIW, I thought that we might run InstCombine less often (or maybe replace it 
> with InstSimplify, in some places). Did you try that?

I haven't. It's one part of pass ordering that we'll want to look at, but I 
figured some optimizing here could happen later. Happy to try a few things 
though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65410/new/

https://reviews.llvm.org/D65410



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


[PATCH] D66460: [analyzer] Remove BugReporter.BugTypes.

2019-08-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

> So we'll most likely make a clear separation between a basic bug report(er) 
> and a path-sensitive bug report(er), having them inherit from common bug 
> report(er) classes.

That sounds terrific! Clang-tidy this or that, the analyzer itself would 
benefit from this greatly, we do have syntactic only checks.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66460/new/

https://reviews.llvm.org/D66460



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


[PATCH] D66348: [ASTImporter] Do not look up lambda classes

2019-08-19 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

I am not enthusiastic about this solution but I need to think about it some 
more.

We can see that p0624r2 
 added 
assignable lambdas:

  bool f1() {
  auto x = []{} = {}; auto x2 = x;
  
  return x == x2;
  }
  
  bool f2() {
  auto x = []{} = {};
  auto xb = []{} = {};
  
  return x == xb;
  }

see godbolt 

So I don't think this is a long-term solution, although I don't know what clang 
is doing to make this work yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66348/new/

https://reviews.llvm.org/D66348



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


[PATCH] D66460: [analyzer] Remove BugReporter.BugTypes.

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.

I'm slowly cleaning up `BugReporter` as a preparation for porting clang-tidy to 
use it exclusively as discussed in 
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063092.html . Basically, 
we'll need to make a `BugReport` and a `BugReporter` that both can be compiled 
with `CLANG_ENABLE_STATIC_ANALYZER=OFF`: no "error nodes", no nothing. So we'll 
most likely make a clear separation between a basic bug report(er) and a 
path-sensitive bug report(er), having them inherit from common bug report(er) 
classes.

While figuring out which field/method goes where, i already made a couple of 
trivial commits (rC369319 , rC369320 
). This one, however, is relatively 
interesting.

The `BugTypes` field is basically unused, and it definitely doesn't need to be 
implemented as an //immutable// set (if at all). But once i removed it i 
started noticing a bunch of null dereferences in 
`generateDiagnosticForConsumerMap()`:

  3060std::unique_ptr Out =
  3061generatePathDiagnostics(consumers, bugReports);
  3062   
  3063if (Out->empty())   // <== crash: Out is null!
  3064  return Out;

This was fun to debug because it's obvious by looking at 
`PathSensitiveBugReporter::generatePathDiagnostics()` that it never returns 
null:

  2643 std::unique_ptr
  2644 PathSensitiveBugReporter::generatePathDiagnostics(...) {
  ...
  2649   auto Out = llvm::make_unique();
  ...
  2658   return Out;
  2659 }

The debugger was also sure that we're dealing with a path-sensitive bug 
reporter here. The answer turned out to be related to this tiny discussion 
. Or, rather, here's a 
report that's worth a thousand words:

F9827911: report-f2e935.html 

So i decided to avoid the destructor minefield entirely, even if it means 
calling flush manually.

Also, ugh, why is CloneChecker subscribed to `check::EndOfTranslationUnit` 
which is a path-sensitive callback? :/


Repository:
  rC Clang

https://reviews.llvm.org/D66460

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -609,6 +609,7 @@
   // After all decls handled, run checkers on the entire TranslationUnit.
   checkerMgr->runCheckersOnEndOfTranslationUnit(TU, *Mgr, BR);
 
+  BR.FlushReports();
   RecVisitorBR = nullptr;
 }
 
@@ -766,6 +767,9 @@
 if (SyntaxCheckTimer)
   SyntaxCheckTimer->stopTimer();
   }
+
+  BR.FlushReports();
+
   if ((Mode & AM_Path) && checkerMgr->hasPathSensitiveCheckers()) {
 RunPathSensitiveChecks(D, IMode, VisitedCallees);
 if (IMode != ExprEngine::Inline_Minimal)
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2221,7 +2221,9 @@
 }
 
 BugReporter::~BugReporter() {
-  FlushReports();
+  // Make sure reports are flushed.
+  assert(StrBugTypes.empty() &&
+ "Destroying BugReporter before diagnostics are emitted!");
 
   // Free the bug reports we are tracking.
   for (const auto I : EQClassesVector)
@@ -2229,9 +2231,6 @@
 }
 
 void BugReporter::FlushReports() {
-  if (BugTypes.isEmpty())
-return;
-
   // We need to flush reports in deterministic order to ensure the order
   // of the reports is consistent between runs.
   for (const auto EQ : EQClassesVector)
@@ -2242,9 +2241,6 @@
   // FIXME: There are leaks from checkers that assume that the BugTypes they
   // create will be destroyed by the BugReporter.
   llvm::DeleteContainerSeconds(StrBugTypes);
-
-  // Remove all references to the BugType objects.
-  BugTypes = F.getEmptySet();
 }
 
 //===--===//
@@ -2658,10 +2654,6 @@
   return Out;
 }
 
-void BugReporter::Register(const BugType *BT) {
-  BugTypes = F.add(BugTypes, BT);
-}
-
 void BugReporter::emitReport(std::unique_ptr R) {
   if (const ExplodedNode *E = R->getErrorNode()) {
 // An error node must either be a sink or have a tag, otherwise
@@ -2692,8 +2684,6 @@
   R->Profile(ID);
 
   // Lookup the equivance class.  If there isn't one, create it.
-  const BugType& BT = R->getBugType();
-  Register(&BT);
   void *InsertPos;
   BugReportEquivClass* EQ = EQClasses.FindNodeOr

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-19 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 216046.
aganea marked 4 inline comments as done.
aganea added a comment.

As requested.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDeclCXX.cpp
  test/CodeGenCXX/debug-info-atexit-stub.cpp
  test/CodeGenCXX/debug-info-global-ctor-dtor.cpp


Index: test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
===
--- test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
+++ test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
@@ -30,24 +30,24 @@
 template A FooTpl::sdm_tpl;
 
 // CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init",{{.*}} line: 
15,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor_glob",{{.*}} line: 15,{{.*}} 
DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor_glob",{{.*}}: DISPFlagLocalToUnit 
| DISPFlagDefinition
 // CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init.1",{{.*}} line: 
16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 // CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} line: 
16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor_array",{{.*}} line: 16,{{.*}} 
DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor__ZZ3foovE4stat",{{.*}} line: 
19,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor_array",{{.*}}: 
DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor__ZZ3foovE4stat",{{.*}} 
DISPFlagLocalToUnit | DISPFlagDefinition
 // CHECK-NOKEXT: !DISubprogram({{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 
 // CHECK-KEXT: !DISubprogram({{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 
 // CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'glob'",{{.*}} 
line: 15,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 
'glob'",{{.*}} line: 15,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 
'glob'",{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
 // CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'array'",{{.*}} 
line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
 // CHECK-MSVC: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} line: 
16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 
'array'",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 
'stat'",{{.*}} line: 19,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 
'array'",{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 
'stat'",{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
 
 // MSVC does weird stuff when templates are involved, so we don't match 
exactly,
 // but these names are reasonable.
 // FIXME: These should not be marked DISPFlagLocalToUnit.
 // CHECK-MSVC: !DISubprogram(name: "FooTpl::`dynamic initializer for 
'sdm_tpl'",{{.*}} line: 29,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "FooTpl::`dynamic atexit destructor 
for 'sdm_tpl'",{{.*}} line: 29,{{.*}}: DISPFlagLocalToUnit | 
DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "FooTpl::`dynamic atexit destructor 
for 'sdm_tpl'",{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
Index: test/CodeGenCXX/debug-info-atexit-stub.cpp
===
--- test/CodeGenCXX/debug-info-atexit-stub.cpp
+++ test/CodeGenCXX/debug-info-atexit-stub.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm %s -gcodeview -debug-info-kind=limited -o - | 
FileCheck %s
+
+struct a {
+  ~a();
+};
+template  struct c : a {
+  c(void (b::*)());
+};
+struct B {
+  virtual void e();
+};
+c *d() { static c f(&B::e); return &f; }
+
+// CHECK: define internal void @"??__Ff@?1??d@@YAPEAU?$c@UBXZ@YAXXZ"()
+// CHECK-SAME: !dbg ![[SUBPROGRAM:[0-9]+]] {
+// CHECK: call void @"??1?$c@UBQEAA@XZ"(%struct.c* 
@"?f@?1??d@@YAPEAU?$c@UBXZ@4U2@A"), !dbg ![[LOCATION:[0-9]+]]
+// CHECK-NEXT: ret void, !dbg ![[LOCATION]]
+// CHECK: ![[SUBPROGRAM]] = distinct !DISubprogram(name: "`dynamic atexit 
destructor for 'f'"
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK: ![[LOCATION]] = !DILocation(line: 0, scope: ![[SUBPROGRAM]])
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -247,6 +247,8 @@
 
   CGF.StartFunction(GlobalDecl(&VD, DynamicInitKind::AtExit),
 CGM.getContext().VoidTy, fn, FI, Functi

[PATCH] D65453: [analyzer] Improve the accuracy of the Clang call graph analysis

2019-08-19 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL369321: [CallGraph] Take into accound calls that aren't 
within any function bodies. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65453?vs=215999&id=216040#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65453/new/

https://reviews.llvm.org/D65453

Files:
  cfe/trunk/include/clang/Analysis/CallGraph.h
  cfe/trunk/lib/Analysis/CallGraph.cpp
  cfe/trunk/test/Analysis/cxx-callgraph.cpp
  cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp

Index: cfe/trunk/include/clang/Analysis/CallGraph.h
===
--- cfe/trunk/include/clang/Analysis/CallGraph.h
+++ cfe/trunk/include/clang/Analysis/CallGraph.h
@@ -131,6 +131,7 @@
 
   bool shouldWalkTypesOfTypeLocs() const { return false; }
   bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
 
 private:
   /// Add the given declaration to the call graph.
Index: cfe/trunk/test/Analysis/cxx-callgraph.cpp
===
--- cfe/trunk/test/Analysis/cxx-callgraph.cpp
+++ cfe/trunk/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp
@@ -1,5 +1,6 @@
 // FIXME: Figure out how to use %clang_analyze_cc1 with our lit.local.cfg.
 // RUN: %clang_cc1 -analyze -triple x86_64-unknown-linux-gnu \
+// RUN: -analyze-function "test()" \
 // RUN: -analyzer-checker=core \
 // RUN: -analyzer-dump-egraph=%t.dot %s
 // RUN: %exploded_graph_rewriter %t.dot | FileCheck %s
Index: cfe/trunk/lib/Analysis/CallGraph.cpp
===
--- cfe/trunk/lib/Analysis/CallGraph.cpp
+++ cfe/trunk/lib/Analysis/CallGraph.cpp
@@ -79,6 +79,34 @@
 VisitChildren(CE);
   }
 
+  void VisitLambdaExpr(LambdaExpr *LE) {
+if (CXXMethodDecl *MD = LE->getCallOperator())
+  G->VisitFunctionDecl(MD);
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (FunctionDecl *FD = E->getOperatorNew())
+  addCalledDecl(FD);
+VisitChildren(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+CXXConstructorDecl *Ctor = E->getConstructor();
+if (FunctionDecl *Def = Ctor->getDefinition())
+  addCalledDecl(Def);
+VisitChildren(E);
+  }
+
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -143,13 +171,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r369321 - [CallGraph] Take into accound calls that aren't within any function bodies.

2019-08-19 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Mon Aug 19 19:22:37 2019
New Revision: 369321

URL: http://llvm.org/viewvc/llvm-project?rev=369321&view=rev
Log:
[CallGraph] Take into accound calls that aren't within any function bodies.

This patch improves Clang call graph analysis by adding in expressions
that are not found in regular function bodies, such as default arguments
or member initializers.

Patch by Joshua Cranmer!

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

Added:
cfe/trunk/test/Analysis/cxx-callgraph.cpp
Modified:
cfe/trunk/include/clang/Analysis/CallGraph.h
cfe/trunk/lib/Analysis/CallGraph.cpp

cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp

Modified: cfe/trunk/include/clang/Analysis/CallGraph.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CallGraph.h?rev=369321&r1=369320&r2=369321&view=diff
==
--- cfe/trunk/include/clang/Analysis/CallGraph.h (original)
+++ cfe/trunk/include/clang/Analysis/CallGraph.h Mon Aug 19 19:22:37 2019
@@ -131,6 +131,7 @@ public:
 
   bool shouldWalkTypesOfTypeLocs() const { return false; }
   bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
 
 private:
   /// Add the given declaration to the call graph.

Modified: cfe/trunk/lib/Analysis/CallGraph.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CallGraph.cpp?rev=369321&r1=369320&r2=369321&view=diff
==
--- cfe/trunk/lib/Analysis/CallGraph.cpp (original)
+++ cfe/trunk/lib/Analysis/CallGraph.cpp Mon Aug 19 19:22:37 2019
@@ -79,6 +79,34 @@ public:
 VisitChildren(CE);
   }
 
+  void VisitLambdaExpr(LambdaExpr *LE) {
+if (CXXMethodDecl *MD = LE->getCallOperator())
+  G->VisitFunctionDecl(MD);
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (FunctionDecl *FD = E->getOperatorNew())
+  addCalledDecl(FD);
+VisitChildren(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+CXXConstructorDecl *Ctor = E->getConstructor();
+if (FunctionDecl *Def = Ctor->getDefinition())
+  addCalledDecl(Def);
+VisitChildren(E);
+  }
+
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -143,13 +171,20 @@ bool CallGraph::includeInGraph(const Dec
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {

Added: cfe/trunk/test/Analysis/cxx-callgraph.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-callgraph.cpp?rev=369321&view=auto
==
--- cfe/trunk/test/Analysis/cxx-callgraph.cpp (added)
+++ cfe/trunk/test/Analysis/cxx-callgraph.cpp Mon Aug 19 19:22:37 2019
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | 
FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}

Modified: 
cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp?rev=369321&r1=369320&r2=369321&view=diff
==
--- 
cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_construction.cpp 
(original)
+++ 
cfe/trunk/test/Analysis/exploded-graph-rewriter/objects_under_con

r369320 - [analyzer] NFC: Rename GRBugReporter to PathSensitiveBugReporter.

2019-08-19 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Mon Aug 19 19:15:50 2019
New Revision: 369320

URL: http://llvm.org/viewvc/llvm-project?rev=369320&view=rev
Log:
[analyzer] NFC: Rename GRBugReporter to PathSensitiveBugReporter.

The GR prefix is super ancient.

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

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=369320&r1=369319&r2=369320&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Mon 
Aug 19 19:15:50 2019
@@ -405,7 +405,7 @@ public:
 /// The base class is used for generating path-insensitive
 class BugReporter {
 public:
-  enum Kind { BaseBRKind, GRBugReporterKind };
+  enum Kind { BasicBRKind, PathSensitiveBRKind };
 
 private:
   using BugTypesTy = llvm::ImmutableSet;
@@ -437,7 +437,7 @@ protected:
 
 public:
   BugReporter(BugReporterData& d)
-  : BugTypes(F.getEmptySet()), kind(BaseBRKind), D(d) {}
+  : BugTypes(F.getEmptySet()), kind(BasicBRKind), D(d) {}
   virtual ~BugReporter();
 
   /// Generate and flush diagnostics for all bug reports.
@@ -504,14 +504,14 @@ private:
 };
 
 /// GRBugReporter is used for generating path-sensitive reports.
-class GRBugReporter : public BugReporter {
+class PathSensitiveBugReporter : public BugReporter {
   ExprEngine& Eng;
 
 public:
-  GRBugReporter(BugReporterData& d, ExprEngine& eng)
-  : BugReporter(d, GRBugReporterKind), Eng(eng) {}
+  PathSensitiveBugReporter(BugReporterData& d, ExprEngine& eng)
+  : BugReporter(d, PathSensitiveBRKind), Eng(eng) {}
 
-  ~GRBugReporter() override = default;
+  ~PathSensitiveBugReporter() override = default;
 
   /// getGraph - Get the exploded graph created by the analysis engine
   ///  for the analyzed method or function.
@@ -534,7 +534,7 @@ public:
 
   /// classof - Used by isa<>, cast<>, and dyn_cast<>.
   static bool classof(const BugReporter* R) {
-return R->getKind() == GRBugReporterKind;
+return R->getKind() == PathSensitiveBRKind;
   }
 };
 
@@ -551,18 +551,19 @@ public:
 };
 
 class BugReporterContext {
-  GRBugReporter &BR;
+  PathSensitiveBugReporter &BR;
   NodeMapClosure NMC;
 
   virtual void anchor();
 
 public:
-  BugReporterContext(GRBugReporter &br, InterExplodedGraphMap &Backmap)
+  BugReporterContext(PathSensitiveBugReporter &br,
+ InterExplodedGraphMap &Backmap)
   : BR(br), NMC(Backmap) {}
 
   virtual ~BugReporterContext() = default;
 
-  GRBugReporter& getBugReporter() { return BR; }
+  PathSensitiveBugReporter& getBugReporter() { return BR; }
 
   const ExplodedGraph &getGraph() const { return BR.getGraph(); }
 

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=369320&r1=369319&r2=369320&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Mon 
Aug 19 19:15:50 2019
@@ -145,9 +145,9 @@ private:
   ObjCNoReturn ObjCNoRet;
 
   /// The BugReporter associated with this engine.  It is important that
-  ///  this object be placed at the very end of member variables so that its
-  ///  destructor is called before the rest of the ExprEngine is destroyed.
-  GRBugReporter BR;
+  /// this object be placed at the very end of member variables so that its
+  /// destructor is called before the rest of the ExprEngine is destroyed.
+  PathSensitiveBugReporter BR;
 
   /// The functions which have been analyzed through inlining. This is owned by
   /// AnalysisConsumer. It can be null.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=369320&r1=369319&r2=369320&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Mon Aug 19 19:15:50 2019
@@ -215,7 +215,8 @@ public:
   /// a PathDiagnosticBuilder able to construct bug reports for different
   /// consumers. Returns None if no valid report is found.
   static Optional
-  findValidReport(ArrayRef &bugReports, GRBugReporter &Reporter);
+  findValidReport(ArrayRef &bugReports,
+  PathSensitiveBugReporter &Reporter);
 
   PathDiagnosticBuilder(
   BugReporterContext BRC, std::u

r369319 - [analyzer] NFC: Drop support for extra text attached to bug reports.

2019-08-19 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Mon Aug 19 19:15:47 2019
New Revision: 369319

URL: http://llvm.org/viewvc/llvm-project?rev=369319&view=rev
Log:
[analyzer] NFC: Drop support for extra text attached to bug reports.

It was introduced in 2011 but never used since then.

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=369319&r1=369318&r2=369319&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Mon 
Aug 19 19:15:47 2019
@@ -88,7 +88,6 @@ public:
   using VisitorList = SmallVector, 8>;
   using visitor_iterator = VisitorList::iterator;
   using visitor_range = llvm::iterator_range;
-  using ExtraTextList = SmallVector;
   using NoteList = SmallVector, 4>;
 
 protected:
@@ -106,7 +105,6 @@ protected:
   const ExplodedNode *ErrorNode = nullptr;
   SmallVector Ranges;
   const SourceRange ErrorNodeRange;
-  ExtraTextList ExtraText;
   NoteList Notes;
 
   /// A (stack of) a set of symbols that are registered with this
@@ -288,17 +286,6 @@ public:
 return Notes;
   }
 
-  /// This allows for addition of meta data to the diagnostic.
-  ///
-  /// Currently, only the HTMLDiagnosticClient knows how to display it.
-  void addExtraText(StringRef S) {
-ExtraText.push_back(S);
-  }
-
-  virtual const ExtraTextList &getExtraText() {
-return ExtraText;
-  }
-
   /// Return the "definitive" location of the reported bug.
   ///
   ///  While a bug can span an entire path, usually there is a specific

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=369319&r1=369318&r2=369319&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Mon Aug 19 19:15:47 2019
@@ -2878,11 +2878,6 @@ void BugReporter::FlushReport(BugReportE
 Pieces.push_front(*I);
 }
 
-// Get the meta data.
-const BugReport::ExtraTextList &Meta = report->getExtraText();
-for (const auto &i : Meta)
-  PD->addMeta(i);
-
 updateExecutedLinesWithDiagnosticPieces(*PD);
 Consumer->HandlePathDiagnostic(std::move(PD));
   }


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


[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Also thanks, everything makes sense now!

Do we already have a test that will cover the necessity for having a map from 
regions to cast results? Eg.:

  void foo(Shape *C, Shape *T) {
if (isa(S) && !isa(T))
  clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
  }




Comment at: clang/test/Analysis/cast-value-state-dump.cpp:30
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' 
fails}}
+// expected-note@-2 {{Taking false branch}}

We're not assuming it, right? We already know it's gonna fail because we 
already know that it's a circle.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:19
+public:
+  enum CastKind { Success, Fail };
+

I suggest `enum CastResult { Success, Failure }` ("a fail" is slang, also 
"result" because it's basically a result).

Also TODO: The success information should ideally go straight into the dynamic 
type map.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:31-32
+
+  bool isSucceeds() const { return Kind == CastKind::Success; }
+  bool isFails() const { return Kind == CastKind::Fail; }
+

`succeeds()`, `fails()` (valid English).



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:28
 /// symbol to its most likely type.
-struct DynamicTypeMap {};
+REGISTER_MAP_WITH_PROGRAMSTATE(DynamicTypeMap, const clang::ento::MemRegion *,
+   clang::ento::DynamicTypeInfo)

Can we move these macros into the cpp file so that they were only accessed by 
the fancy accessors?

Also i don't remember whether these macros do actually work correctly in 
headers. The original code was doing the trait manually because it had 
`GDMIndex()` defined in the cpp file, but if we put these macros into the 
header we'll have a static variable defined in multiple translation units, 
which may cause it to have different addresses in different dynamically loaded 
libraries that include this header.

You might need to merge your `checkDeadSymbols()` into the existing 
`checkDeadSymbols()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93
+static QualType getRecordType(QualType Ty) {
+  Ty = Ty.getCanonicalType();
+
+  if (Ty->isPointerType())
+return getRecordType(Ty->getPointeeType());
 
+  if (Ty->isReferenceType())

Most of the time we should know exactly how many pointer/reference types we 
need to unwrap. I really prefer we hard-assert this knowledge instead of 
blindly unwrapping as many pointer/reference types as we want. Because every 
time we have an unexpected number of unwraps it's an indication that something 
went horribly wrong. So it's good to have the extra sanity checking.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:115
+
+  const DynamicCastInfo *Cast = getDynamicCastInfo(State, CastFromTy, 
CastToTy);
+

It would have been much easier for me to read if this variable was called 
`CastInfo`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:118
+  // We assume that every checked cast succeeds.
+  bool IsCastSucceeds;
+  if (Cast)

Just `CastSucceeds` (valid English).



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:145-152
+if (!IsCheckedCast)
+  Out << (IsKnownCast ? "Dynamic cast" : "Assuming dynamic cast");
+else
+  Out << "Checked cast";
 
-Out << "to '" << CastToName << "' "
-<< (!IsNullReturn ? "succeeds" : "fails");
+Out << " from '" << CastFromTy->getAsCXXRecordDecl()->getNameAsString()
+<< "' to '" << CastToTy->getAsCXXRecordDecl()->getNameAsString()

To think: in D66423 you added this fancy feature when you dump the current 
dynamic type of the object instead of the static type. I think this is super 
cool and we should do it here as well, because the dynamic type is pretty much 
the only thing that you won't be able to figure out by looking at the source.

We could give the same message as for `isa`, say, `Assuming the object is a 
'Circle'` etc.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp:83-86
+  CastSet Set = F.getEmptySet();
+
+  if (const CastSet *TempSet = State->get(MR))
+Set = *TempSet;

My favorite way of writing this stuff:
```lang=c++
const CastSet *SetPtr = State->get(MR);
CastSet Set = SetPtr ? *SetPTr : F.getEmptySet();
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[clang-tools-extra] r369316 - Fix typo. "piont" => "point"

2019-08-19 Thread Richard Trieu via cfe-commits
Author: rtrieu
Date: Mon Aug 19 17:28:21 2019
New Revision: 369316

URL: http://llvm.org/viewvc/llvm-project?rev=369316&view=rev
Log:
Fix typo.  "piont" => "point"

Found by Chris Morris (cwmorris).

Modified:
clang-tools-extra/trunk/clang-tidy/abseil/DurationConversionCastCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-conversion-cast.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/abseil/DurationConversionCastCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationConversionCastCheck.cpp?rev=369316&r1=369315&r2=369316&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationConversionCastCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationConversionCastCheck.cpp 
Mon Aug 19 17:28:21 2019
@@ -70,7 +70,7 @@ void DurationConversionCastCheck::check(
 llvm::StringRef NewFuncName = getDurationInverseForScale(*Scale).first;
 
 diag(MatchedCast->getBeginLoc(), "duration should be converted directly to 
"
- "a floating-piont number rather than "
+ "a floating-point number rather than "
  "through a type cast")
 << FixItHint::CreateReplacement(
MatchedCast->getSourceRange(),

Modified: 
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-conversion-cast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-conversion-cast.cpp?rev=369316&r1=369315&r2=369316&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-conversion-cast.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-conversion-cast.cpp 
Mon Aug 19 17:28:21 2019
@@ -11,37 +11,37 @@ void f() {
   // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to an integer rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToInt64Hours(d1);
   x = static_cast(absl::ToInt64Hours(d1));
-  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-piont number rather than through a type cast 
[abseil-duration-conversion-cast]
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-point number rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToDoubleHours(d1);
   i = static_cast(absl::ToDoubleMinutes(d1));
   // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to an integer rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToInt64Minutes(d1);
   x = static_cast(absl::ToInt64Minutes(d1));
-  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-piont number rather than through a type cast 
[abseil-duration-conversion-cast]
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-point number rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToDoubleMinutes(d1);
   i = static_cast(absl::ToDoubleSeconds(d1));
   // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to an integer rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToInt64Seconds(d1);
   x = static_cast(absl::ToInt64Seconds(d1));
-  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-piont number rather than through a type cast 
[abseil-duration-conversion-cast]
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-point number rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToDoubleSeconds(d1);
   i = static_cast(absl::ToDoubleMilliseconds(d1));
   // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to an integer rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToInt64Milliseconds(d1);
   x = static_cast(absl::ToInt64Milliseconds(d1));
-  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-piont number rather than through a type cast 
[abseil-duration-conversion-cast]
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to a floating-point number rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES: absl::ToDoubleMilliseconds(d1);
   i = static_cast(absl::ToDoubleMicroseconds(d1));
   // CHECK-MESSAGES: [[@LINE-1]]:7: warning: duration should be converted 
directly to an integer rather than through a type cast 
[abseil-duration-conversion-cast]
   // CHECK-FIXES:

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment.

In D66325#1636091 , @NoQ wrote:

> Yay! I understand the rough idea and it looks perfectly reasonable to start 
> with. Please add FIXMEs/TODOs on how we eventually want to support more 
> complicated inheritance hierarchies.


I have not decided yet. This only one successful cast allowance is the smallest 
possible solution, then we need a very great design to do better without 
exponential path-splitting. I have added a tiny TODO section.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[PATCH] D66404: [CFG] Make destructor calls more accurate

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

I have a few nitpicks but i still love this patch, thank you! It picks up the 
work exactly where i dropped it a year or so ago.

> Respect C++17 copy elision; previously it would generate destructor calls for 
> elided temporaries, including in initialization and return statements.

I think the root cause of these redundant destructors was the confusing AST 
that contains a `CXXBindTemporaryExpr` even when there's not much of a 
temporary. See also a very loosely related discussion in 
http://lists.llvm.org/pipermail/cfe-dev/2018-March/057475.html




Comment at: lib/Analysis/CFG.cpp:2102
 case Stmt::CompoundStmtClass:
-  return VisitCompoundStmt(cast(S));
+  return VisitCompoundStmt(cast(S), 
/*ExternallyDestructed=*/false);
 

This goes over the 80 characters limit :)



Comment at: lib/Analysis/CFG.cpp:2290-2295
+if (BuildOpts.AddTemporaryDtors) {
+  TempDtorContext Context;
+  VisitForTemporaryDtors(EC->getSubExpr(),
+ /*ExternallyDestructed=*/true, Context);
+}
+return Visit(EC->getSubExpr(), asc);

This looks like a copy-paste from `VisitExprWithCleanups` with the 
`ExternallyDestructed` flag flipped. Could you double-check if the update to 
`asc` that's present in `VisitExprWithCleanups` is relevant here as well? I've 
actually no idea what do these do and we don't seem to have any tests for them, 
so feel free to ignore, but it might make sense to at least deduplicate the 
code.



Comment at: test/Analysis/cfg-rich-constructors.cpp:415
 
-// FIXME: There should be no temporary destructor in C++17.
 // CHECK:  return_stmt_with_dtor::D returnTemporary()

Yay!! I'm very happy these are sorted out.



Comment at: test/Analysis/cfg-rich-constructors.mm:63
-// CXX17-NEXT: 6: ~D() (Temporary object destructor)
-// CXX17-NEXT: 7: [B1.5].~D() (Implicit destructor)
 void returnObjectFromMessage(E *e) {

Whoops, looks like i forgot to add a `FIXME` here. This update is correct, 
thanks!



Comment at: test/Analysis/more-dtors-cfg-output.cpp:3
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -std=c++14 
-DCXX2A=0 -fblocks -Wall -Wno-unused -Werror %s > %t.14 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -std=c++2a 
-DCXX2A=1 -fblocks -Wall -Wno-unused -Werror %s > %t.17 2>&1
+// RUN: FileCheck --input-file=%t.14 -check-prefixes=CHECK,CXX14 
-implicit-check-not=destructor %s

Did you mean `t.20`? :)



Comment at: test/Analysis/more-dtors-cfg-output.cpp:50
+// CXX14: (CXXConstructExpr{{.*}}, struct Foo)
+// CXX14: ~Foo() (Temporary object destructor)
+// CHECK: ~Foo() (Implicit destructor)

Maybe also add `CXX20-NOT` so that to make sure that the destructor *isn't* 
there? (i think this applies to a lot of other tests)



Comment at: test/Analysis/more-dtors-cfg-output.cpp:93
+
+void elided_lambda_capture_init() {
+  // The copy from get_foo() into the lambda should be elided.  Should call

Pls mention somehow that this language feature is called "generalized lambda 
captures" because it's fairly hard to google for :)



Comment at: test/Analysis/more-dtors-cfg-output.cpp:233
+
+// FIXME: Here are some cases that are currently handled wrongly:
+

I'm afraid that nobody will notice this comment when more items will be added 
to this test file. Having a `FIXME` in tests themselves is sufficient.



Comment at: test/Analysis/more-dtors-cfg-output.cpp:251-252
+void default_ctor_with_default_arg() {
+  // FIXME: The CFG records a construction of the array type but a destruction
+  // of the base type, which is inconsistent.  It really should should be
+  // emitting a loop, which should also contain the construction/destruction of

Is it just mismatching dump styles or is it an actual error?



Comment at: test/Analysis/more-dtors-cfg-output.cpp:252-254
+  // of the base type, which is inconsistent.  It really should should be
+  // emitting a loop, which should also contain the construction/destruction of
+  // default arguments.

These should most likely be two separate loops: default arguments are destroyed 
immediately after the constructor of `qux[2]`, but elements of `qux` should not 
be destroyed before the end of the scope.



Comment at: test/Analysis/more-dtors-cfg-output.cpp:273
+#if CXX2A
+// Boilerplate needed to test co_return:
+

Feel free to move this into 
`test/Analysis/inputs/system-header-simulator-cxx.h`, we could always use more 
mocks in there.



Comment at: test/Analysis/temporaries.cpp:836
-#else
-// FIXME: Destructor called twice in C++17?
-clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 216023.
Charusso edited the summary of this revision.
Charusso added a comment.

- Use a set factory to store a dynamic cast information set per memory region.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l  \{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// RUN:  -analyzer-output=text -verify %s 2>&1 | FileCheck %s
+
+void clang_analyzer_printState();
+
+namespace llvm {
+template 
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y &Value);
+} // namespace llvm
+
+namespace clang {
+struct Shape {};
+class Triangle : public Shape {};
+class Circle : public Shape {};
+class Square : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
+
+void evalNonNullParamNonNullReturnReference(const Shape &S) {
+  const auto *C = dyn_cast_or_null(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(S)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  clang_analyzer_printState();
+
+  // CHECK:  "dynamic_types": [
+  // CHECK-NEXT:   { "region": "SymRegion{reg_$0}", "dyn_type": "const class clang::Circle", "sub_classable": true }
+  // CHECK-NEXT: ],
+  // CHECK-NEXT: "dynamic_casts": [
+  // CHECK:{ "region": "SymRegion{reg_$0}", "casts": [
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class clang::Circle", "kind": "success" },
+  // CHECK-NEXT: { "from": "struct clang::Shape", "to": "class clang::Square", "kind": "fail" }
+  // CHECK-NEXT:   ]}
+
+  (void)(1 / !C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+}
Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -1,14 +1,7 @@
 // RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -verify=logic %s
-// RUN: %clang_analyze_cc1 \
 // RUN:  -analyzer-ch

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

2019-08-19 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

In D63932#1610182 , @ostannard wrote:

> > Partial linking will indeed prevent dropping the virtual functions, but it 
> > should not prevent clearing the pointer to the virtual function in the 
> > vtable. The linker should then be able to drop the virtual function body as 
> > part of `--gc-sections` during the final link.
>
> If partial linking isn't doing internalisation, I'd expect that to prevent a 
> lot of other LTO optimisations, not just VFE. Is this a common use-case which 
> we need to care about?


It isn't that common, but it seems worth doing if it can be done easily.

That said, I note that it does appear that your implementation will end up 
preserving the pointer in the vtable in this case because you're relying on the 
use list to make decisions about what to GC. So it doesn't seem easy to do at 
this point, but if for example we made this compatible with ThinLTO at some 
point we would probably not be able to rely on the use list, and the resulting 
changes to this feature might make this easier to do.

>> I think I would favour something closer to your first suggestion, but 
>> instead of telling GlobalDCE specifically about this, we represent the 
>> "post-link" flag in the IR (e.g. as a module flag) in order to keep the IR 
>> self-contained. LTO would then be taught to add this flag at the right time, 
>> and the logic inside GlobalDCE would be:
>> 
>> - If post-link flag not set, allow VFE if linkage <= TranslationUnit.
>> - If post-link flag set, allow VFE if linkage <= LinkageUnit.
>> 
>>   This would also help address a correctness issue with the CFI and WPD 
>> passes, which is that it is currently unsound to run them at compile time. 
>> If we let them run at compile time, we would in principle be able to do CFI 
>> and WPD on internal linkage types without LTO.
> 
> Ok, sounds reasonable, though I suspect WPD and CFI will need a slightly 
> different definition of type visibility - they care about the possibility of 
> unseen code adding new derived classes, so the visibility of base classes 
> isn't important, and they might be able to make use of the final specifier.

Internal linkage types use distinct metadata (i.e. metadata that can't be 
merged with other TUs), so there's no possibility of a new derived class being 
added. I suspect that we could make these passes check the distinctness of 
metadata if the post-link flag is not set, but that probably deserves more 
thought.




Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:409
+  F->replaceAllUsesWith(
+  
ConstantPointerNull::get(PointerType::getUnqual(F->getFunctionType(;
+}

Could be just `ConstantPointerNull::get(F->getType())`.



Comment at: llvm/test/LTO/ARM/lto-linking-metadata.ll:2
+; RUN: opt %s -o %t1.bc
+; RUN: llvm-lto %t1.bc -o %t1.save.opt -save-merged-module -O1 
--exported-symbol=foo
+; RUN: llvm-dis < %t1.save.opt.merged.bc | FileCheck %s

Could you add a test for the new LTO API as well as the legacy one?



Comment at: 
llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll:30
+; pointer of type "int (B::*q)(int)", so the call could only be dispatched to
+; or B::foo. It can't be dispatched to A::bar or B::bar as the function pointer
+; does not match, and it can't be dispatched to A::foo as the object type

Remove word "or".



Comment at: 
llvm/test/Transforms/GlobalDCE/virtual-functions-visibility-post-lto.ll:13
+@_ZTS1A = hidden constant [3 x i8] c"1A\00", align 1
+@_ZTI1A = hidden constant { i8*, i8* } { i8* bitcast (i8** getelementptr 
inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* 
getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1A, i32 0, i32 0) }, align 8
+

Could you simplify this test (and others) by removing the RTTI, please? We 
should also have a test showing that RTTI is preserved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63932/new/

https://reviews.llvm.org/D63932



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


[PATCH] D52524: Add -Wpoison-system-directories warning

2019-08-19 Thread Denis Nikitin via Phabricator via cfe-commits
denik updated this revision to Diff 216021.
denik added a comment.

Removed check for libraries.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/lib/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/include/c++/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/lib/gcc/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/include/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/lib/.keep
  clang/test/Frontend/warning-poison-system-directories.c


Index: clang/test/Frontend/warning-poison-system-directories.c
===
--- /dev/null
+++ clang/test/Frontend/warning-poison-system-directories.c
@@ -0,0 +1,27 @@
+// System directory and sysroot option causes warning.
+// RUN: %clang -Wpoison-system-directories -target x86_64 -I/usr/include 
--sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-cxx-isystem/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c 
-o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-iquote/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-isystem/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+
+// Missing target but included sysroot still causes the warning.
+// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.2.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.2.stderr %s
+
+// With -Werror the warning causes the failure.
+// RUN: not %clang -Werror=poison-system-directories -target x86_64 
-I/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 
2> %t.3.stderr
+// RUN: FileCheck -check-prefix=ERROR < %t.3.stderr %s
+
+// Cros target without sysroot causes no warning.
+// RUN: %clang -Wpoison-system-directories -Werror -target x86_64 
-I/usr/include -c -o - %s
+
+// By default the warning is off.
+// RUN: %clang -Werror -target x86_64 -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s
+
+// WARN: warning: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Wpoison-system-directories]
+
+// ERROR: error: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Werror,-Wpoison-system-directories]
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -137,6 +137,15 @@
   SmallString<256> MappedPathStorage;
   StringRef MappedPathStr = Path.toStringRef(MappedPathStorage);
 
+  // If use system headers while cross-compiling, emit the warning.
+  if (HasSysroot) {
+if (MappedPathStr.startswith("/usr/include") ||
+MappedPathStr.startswith("/usr/local/include")) {
+  Headers.getDiags().Report(diag::warn_poison_system_directories)
+  << MappedPathStr.str();
+}
+  }
+
   // Compute the DirectoryLookup type.
   SrcMgr::CharacteristicKind Type;
   if (Group == Quoted || Group == Angled || Group == IndexHeaderMap) {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1067,6 +1067,10 @@
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
 
+// A warning group for warnings about including system headers when
+// cross-compiling.
+def PoisonSystemDirectories : DiagGroup<"poison-system-directories">;
+
 def NoDeref : DiagGroup<"noderef">;
 
 // A group for cross translation unit static analysis related warnings.
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -304,4 +304,9 @@
 "no analyzer checkers or packages are associated with '%0'">;
 def note_suggest_disabling_all_checkers : Note<
 "use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;
+
+// Poison system directories.
+def warn_poison_system_directories : Warning <
+  "include location '%0' is unsafe

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

2019-08-19 Thread Anton Afanasyev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL369308: [Support][Time profiler] Make FE codegen blocks to 
be inside frontend blocks (authored by anton-afanasyev, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D63325?vs=206187&id=216011#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/test/Driver/check-time-trace-sections.cpp
  cfe/trunk/test/Driver/check-time-trace-sections.py
  llvm/trunk/lib/Support/TimeProfiler.cpp


Index: llvm/trunk/lib/Support/TimeProfiler.cpp
===
--- llvm/trunk/lib/Support/TimeProfiler.cpp
+++ llvm/trunk/lib/Support/TimeProfiler.cpp
@@ -58,8 +58,8 @@
 auto &E = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+// Only include sections longer or equal to TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: cfe/trunk/test/Driver/check-time-trace-sections.py
===
--- cfe/trunk/test/Driver/check-time-trace-sections.py
+++ cfe/trunk/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", events)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside any Frontend section!")
+
+if not all([all([is_before(frontend, backend) for frontend in frontends])
+for backend in backends]):
+sys.exit("Not all Frontend section are before all Backend sections!")
Index: cfe/trunk/test/Driver/check-time-trace-sections.cpp
===
--- cfe/trunk/test/Driver/check-time-trace-sections.cpp
+++ cfe/trunk/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -229,6 +230,7 @@
 
 void HandleTranslationUnit(ASTContext &C) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;


Index: llvm/trunk/lib/Support/TimeProfiler.cpp
===
--- llvm/trunk/lib/Support/TimeProfiler.cpp
+++ llvm/trunk/lib/Support/TimeProfiler.cpp
@@ -58,8 +58,8 @@
 auto &E = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+// Only include sections longer or equal to TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: cfe/trunk/test/Driver/check-time-trace-sections.py
===
--- cfe/trunk/test/Driver/check-time-trace-sections.py
+++ cfe/trunk/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]

r369308 - [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

2019-08-19 Thread Anton Afanasyev via cfe-commits
Author: anton-afanasyev
Date: Mon Aug 19 15:58:26 2019
New Revision: 369308

URL: http://llvm.org/viewvc/llvm-project?rev=369308&view=rev
Log:
[Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

Summary:
Add `Frontend` time trace entry to `HandleTranslationUnit()` function.
Add test to check all codegen blocks are inside frontend blocks.
Also, change `--time-trace-granularity` option a bit to make sure very small
time blocks are outputed to json-file when using `--time-trace-granularity=0`.

This fixes http://llvm.org/pr41969

Reviewers: russell.gallop, lebedev.ri, thakis

Reviewed By: russell.gallop

Subscribers: vsapsai, aras-p, lebedev.ri, hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added:
cfe/trunk/test/Driver/check-time-trace-sections.cpp
cfe/trunk/test/Driver/check-time-trace-sections.py
Modified:
cfe/trunk/lib/CodeGen/CodeGenAction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=369308&r1=369307&r2=369308&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Mon Aug 19 15:58:26 2019
@@ -38,6 +38,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -229,6 +230,7 @@ namespace clang {
 
 void HandleTranslationUnit(ASTContext &C) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;

Added: cfe/trunk/test/Driver/check-time-trace-sections.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/check-time-trace-sections.cpp?rev=369308&view=auto
==
--- cfe/trunk/test/Driver/check-time-trace-sections.cpp (added)
+++ cfe/trunk/test/Driver/check-time-trace-sections.cpp Mon Aug 19 15:58:26 2019
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }

Added: cfe/trunk/test/Driver/check-time-trace-sections.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/check-time-trace-sections.py?rev=369308&view=auto
==
--- cfe/trunk/test/Driver/check-time-trace-sections.py (added)
+++ cfe/trunk/test/Driver/check-time-trace-sections.py Mon Aug 19 15:58:26 2019
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", events)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside any Frontend section!")
+
+if not all([all([is_before(frontend, backend) for frontend in frontends])
+for backend in backends]):
+sys.exit("Not all Frontend section are before all Backend sections!")


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


[PATCH] D66068: cmake: Make building clang-shlib optional

2019-08-19 Thread Jan Vesely via Phabricator via cfe-commits
jvesely added a comment.

Hi,

sorry for the delay. I fully understand the need to reduce the number of 
options. Having always used SHARED_LIBS build I remember weekly shared build 
breakages.
That said, forcing everyone to build one huge library effectively makes debug 
builds unusable in any practical way.
Having a usable debug build would also obsolete the with_asserts option.
What is the use-case of one big shared lib that split libraries can't do?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66068/new/

https://reviews.llvm.org/D66068



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


[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Emm, so you're saving successes and failures of all casts //regardless of 
> > > which object is getting casted//? That's definitely not sufficient. If 
> > > `X` is a `Stmt` that isn't an `Expr`, you can't automatically infer that 
> > > `Y` is a `Stmt` that isn't an `Expr` for any object `Y` other than `X` . 
> > > This information needs to be per-object. Then you'll need to clean it up 
> > > in `checkDeadSymbols`.
> > I have two implementations, the other is set factory based on memory 
> > regions. Is it would be a good idea to go through all the regions every 
> > time and all their casts to know every possible cast-assumption? I have 
> > made it, and I felt like it is overcomplicated as the DynamicTypeMap could 
> > be used for asking what is the type now and whether we have better 
> > precision and update that information therefore we could update the 
> > cast-set as well.
> It's about correctness, we don't have much choice here. The current data 
> structure simply cannot work correctly because there's a "one vs many" 
> problem in it: for every pair of types (T₁, T₂) there are *many* possible 
> outcomes of a cast from T₁ to T₂ (depending on the object that's being 
> casted) but the current data structure only has room for *one* such outcome. 
> Your data structure is basically saying "Oh, this shape turned out to be a 
> circle, let's from now on forever believe that triangles don't exist" (?)
Well, let me swap then with the set factory. I think I could handle that in the 
current shape, but I cannot say no to your clear idea. Thanks for the factory 
idea! It does the same at the moment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[PATCH] D65453: [analyzer] Improve the accuracy of the Clang call graph analysis

2019-08-19 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.

Thanks! I'll try to land again.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65453/new/

https://reviews.llvm.org/D65453



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


[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

Charusso wrote:
> NoQ wrote:
> > Emm, so you're saving successes and failures of all casts //regardless of 
> > which object is getting casted//? That's definitely not sufficient. If `X` 
> > is a `Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is 
> > a `Stmt` that isn't an `Expr` for any object `Y` other than `X` . This 
> > information needs to be per-object. Then you'll need to clean it up in 
> > `checkDeadSymbols`.
> I have two implementations, the other is set factory based on memory regions. 
> Is it would be a good idea to go through all the regions every time and all 
> their casts to know every possible cast-assumption? I have made it, and I 
> felt like it is overcomplicated as the DynamicTypeMap could be used for 
> asking what is the type now and whether we have better precision and update 
> that information therefore we could update the cast-set as well.
It's about correctness, we don't have much choice here. The current data 
structure simply cannot work correctly because there's a "one vs many" problem 
in it: for every pair of types (T₁, T₂) there are *many* possible outcomes of a 
cast from T₁ to T₂ (depending on the object that's being casted) but the 
current data structure only has room for *one* such outcome. Your data 
structure is basically saying "Oh, this shape turned out to be a circle, let's 
from now on forever believe that triangles don't exist" (?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:183
+  if (Body)
+DRE = dyn_cast(Body);
+

A body of a function is always either a `CompoundStmt` or (shockingly) a 
`CXXTryStmt`. This cast will always fail.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:188
+
+  QualType CastToTy = DRE->getTemplateArgs()->getArgument().getAsType();
+  QualType CastFromTy = getRecordType(Call.parameters()[0]->getType());

I suspect that `DRE` may still be null (eg., when calling `isa` through 
function pointer).

I think you should just lookup `Call.getDecl()`'s template arguments instead. 
It's going to be the declaration of the specific instantiation, so it'll have 
all the arguments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:216
+  const NoteTag *Tag = C.getNoteTag(
+  [=](BugReport &) -> std::string {
+SmallString<128> Msg;

Can you use the overload without the bug report parameter? Cause you don't seem 
to be using it anyway.



Comment at: clang/test/Analysis/cast-value-notes.cpp:5-27
 namespace llvm {
 template 
 const X *cast(Y Value);
 
 template 
 const X *dyn_cast(Y *Value);
 template 

Szelethus wrote:
> Hmm, we may want to move these eventually to `clang/test/Inputs/`, since this 
> isn't the only LLVM checker. Thought that said, I want to see the other LLVM 
> checker gone, like, purged from the codebase for being the abomination that 
> it is.
> 
> Feel free to leave these here for know, just thinking aloud :)
I agree, extracting common definitions into a system header simulator usually 
ends up being worth it pretty quickly.



Comment at: clang/test/Analysis/cast-value-notes.cpp:89
+  if (isa(C)) {
+// expected-note@-1 {{'C' with type 'Circle' is not the instance of 
'Triangle'}}
+// expected-note@-2 {{Taking false branch}}

I suggest: `'C' is a 'Circle', not a 'Triangle'`.



Comment at: clang/test/Analysis/cast-value-notes.cpp:95
+  if (isa(C)) {
+// expected-note@-1 {{'C' with type 'Circle' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}

Just `'C' is a 'Circle'`.



Comment at: clang/test/Analysis/cast-value-notes.cpp:111
+  if (isa(C)) {
+// expected-note@-1 {{Assuming 'C' with type 'Circle' is not the instance 
of 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}

This test looks incorrect. We shouldn't be assuming this, we already know that 
it's not a triangle because we know that it's a circle.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66423/new/

https://reviews.llvm.org/D66423



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


[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

NoQ wrote:
> Emm, so you're saving successes and failures of all casts //regardless of 
> which object is getting casted//? That's definitely not sufficient. If `X` is 
> a `Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is a 
> `Stmt` that isn't an `Expr` for any object `Y` other than `X` . This 
> information needs to be per-object. Then you'll need to clean it up in 
> `checkDeadSymbols`.
I have two implementations, the other is set factory based on memory regions. 
Is it would be a good idea to go through all the regions every time and all 
their casts to know every possible cast-assumption? I have made it, and I felt 
like it is overcomplicated as the DynamicTypeMap could be used for asking what 
is the type now and whether we have better precision and update that 
information therefore we could update the cast-set as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[PATCH] D66446: [clang][IFS] Adding new Interface Stubs format.

2019-08-19 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi updated this revision to Diff 216004.
plotfi added a comment.

Adding ObjectFileFormat


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66446/new/

https://reviews.llvm.org/D66446

Files:
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -68,6 +68,8 @@
 return std::make_unique();
   case GenerateInterfaceTBEExpV1:
 return std::make_unique();
+  case GenerateInterfaceIfsExpV1:
+return std::make_unique();
   case InitOnly:   return std::make_unique();
   case ParseSyntaxOnly:return std::make_unique();
   case ModuleFileInfo: return std::make_unique();
Index: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -354,9 +354,55 @@
   OS.flush();
 };
 
+auto writeIfsV1 =
+[this](const llvm::Triple &T, const MangledSymbols &Symbols,
+   const ASTContext &context, StringRef Format,
+   raw_ostream &OS) -> void {
+  OS << "--- !" << Format << "\n";
+  OS << "IfsVersion: 1.0\n";
+  OS << "Triple: " << T.str() << "\n";
+  OS << "ObjectFileFormat: " << "ELF" << "\n"; // TODO: For now, just ELF.
+  OS << "Symbols:\n";
+  for (const auto &E : Symbols) {
+const MangledSymbol &Symbol = E.second;
+for (auto Name : Symbol.Names) {
+  OS << "  "
+ << (Symbol.ParentName.empty() || Instance.getLangOpts().CPlusPlus
+ ? ""
+ : (Symbol.ParentName + "."))
+ << Name << ": { Type: ";
+  switch (Symbol.Type) {
+  default:
+llvm_unreachable(
+"clang -emit-iterface-stubs: Unexpected symbol type.");
+  case llvm::ELF::STT_NOTYPE:
+OS << "NoType";
+break;
+  case llvm::ELF::STT_OBJECT: {
+auto VD = cast(E.first)->getType();
+OS << "Object, Size: "
+   << context.getTypeSizeInChars(VD).getQuantity();
+break;
+  }
+  case llvm::ELF::STT_FUNC:
+OS << "Func";
+break;
+  }
+  if (Symbol.Binding == llvm::ELF::STB_WEAK)
+OS << ", Weak: true";
+  OS << " }\n";
+}
+  }
+  OS << "...\n";
+  OS.flush();
+};
+
 if (Format == "experimental-yaml-elf-v1")
   writeIfoYaml(Instance.getTarget().getTriple(), Symbols, context, Format,
*OS);
+else if (Format == "experimental-ifs-v1")
+  writeIfsV1(Instance.getTarget().getTriple(), Symbols, context, Format,
+ *OS);
 else
   writeIfoElfAbiYaml(Instance.getTarget().getTriple(), Symbols, context,
  Format, *OS);
@@ -376,3 +422,10 @@
   return std::make_unique(
   CI, InFile, "experimental-tapi-elf-v1");
 }
+
+std::unique_ptr
+GenerateInterfaceIfsExpV1Action::CreateASTConsumer(CompilerInstance &CI,
+   StringRef InFile) {
+  return std::make_unique(
+  CI, InFile, "experimental-ifs-v1");
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1737,11 +1737,14 @@
 frontend::GenerateInterfaceYAMLExpV1)
   .Case("experimental-tapi-elf-v1",
 frontend::GenerateInterfaceTBEExpV1)
+  .Case("experimental-ifs-v1",
+frontend::GenerateInterfaceIfsExpV1)
   .Default(llvm::None);
   if (!ProgramAction)
 Diags.Report(diag::err_drv_invalid_value)
 << "Must specify a valid interface stub format type using "
 << "-interface-stub-version=";
   Opts.ProgramAction = *ProgramAction;
   break;
@@ -3185,6 +3188,7 @@
   case frontend::GeneratePCH:
   case frontend::GenerateInterfaceYAMLExpV1:
   case frontend::GenerateInterfaceTBEExpV1:
+  case frontend::GenerateInterfaceIfsExpV1:
   case frontend::ParseSyntaxOnly:
   case frontend::ModuleFileInfo:
   case frontend::VerifyPCH:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Yay! I understand the rough idea and it looks perfectly reasonable to start 
with. Please add FIXMEs/TODOs on how we eventually want to support more 
complicated inheritance hierarchies.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32
+/// A set of dynamic cast informations.
+REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo)
 

Emm, so you're saving successes and failures of all casts //regardless of which 
object is getting casted//? That's definitely not sufficient. If `X` is a 
`Stmt` that isn't an `Expr`, you can't automatically infer that `Y` is a `Stmt` 
that isn't an `Expr` for any object `Y` other than `X` . This information needs 
to be per-object. Then you'll need to clean it up in `checkDeadSymbols`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325



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


[PATCH] D66446: [clang][IFS] Adding new Interface Stubs format.

2019-08-19 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi created this revision.
plotfi added a reviewer: compnerd.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
plotfi marked an inline comment as done.
plotfi added inline comments.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:363
+  OS << "IfsVersion: 1.0\n";
+  OS << "Triple: " << T.str() << "\n";
+  OS << "Symbols:\n";

This is missing ObjectFileFormat: ELF


After posting llvm-ifs, I made some progress in hardening up how I think the 
format for Interface Stubs should look.
There are a number of things I think the TBE format was missing (no endianness, 
no info about the Object Format because it assumes ELF), so I have added those 
and broken off from being as similar to the TBE schema. I think in a subsequent 
commit I can drop the other formats.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66446

Files:
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -68,6 +68,8 @@
 return std::make_unique();
   case GenerateInterfaceTBEExpV1:
 return std::make_unique();
+  case GenerateInterfaceIfsExpV1:
+return std::make_unique();
   case InitOnly:   return std::make_unique();
   case ParseSyntaxOnly:return std::make_unique();
   case ModuleFileInfo: return std::make_unique();
Index: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -354,9 +354,54 @@
   OS.flush();
 };
 
+auto writeIfsV1 =
+[this](const llvm::Triple &T, const MangledSymbols &Symbols,
+   const ASTContext &context, StringRef Format,
+   raw_ostream &OS) -> void {
+  OS << "--- !" << Format << "\n";
+  OS << "IfsVersion: 1.0\n";
+  OS << "Triple: " << T.str() << "\n";
+  OS << "Symbols:\n";
+  for (const auto &E : Symbols) {
+const MangledSymbol &Symbol = E.second;
+for (auto Name : Symbol.Names) {
+  OS << "  "
+ << (Symbol.ParentName.empty() || Instance.getLangOpts().CPlusPlus
+ ? ""
+ : (Symbol.ParentName + "."))
+ << Name << ": { Type: ";
+  switch (Symbol.Type) {
+  default:
+llvm_unreachable(
+"clang -emit-iterface-stubs: Unexpected symbol type.");
+  case llvm::ELF::STT_NOTYPE:
+OS << "NoType";
+break;
+  case llvm::ELF::STT_OBJECT: {
+auto VD = cast(E.first)->getType();
+OS << "Object, Size: "
+   << context.getTypeSizeInChars(VD).getQuantity();
+break;
+  }
+  case llvm::ELF::STT_FUNC:
+OS << "Func";
+break;
+  }
+  if (Symbol.Binding == llvm::ELF::STB_WEAK)
+OS << ", Weak: true";
+  OS << " }\n";
+}
+  }
+  OS << "...\n";
+  OS.flush();
+};
+
 if (Format == "experimental-yaml-elf-v1")
   writeIfoYaml(Instance.getTarget().getTriple(), Symbols, context, Format,
*OS);
+else if (Format == "experimental-ifs-v1")
+  writeIfsV1(Instance.getTarget().getTriple(), Symbols, context, Format,
+ *OS);
 else
   writeIfoElfAbiYaml(Instance.getTarget().getTriple(), Symbols, context,
  Format, *OS);
@@ -376,3 +421,10 @@
   return std::make_unique(
   CI, InFile, "experimental-tapi-elf-v1");
 }
+
+std::unique_ptr
+GenerateInterfaceIfsExpV1Action::CreateASTConsumer(CompilerInstance &CI,
+   StringRef InFile) {
+  return std::make_unique(
+  CI, InFile, "experimental-ifs-v1");
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1737,11 +1737,14 @@
 frontend::GenerateInterfaceYAMLExpV1)
   .Case("experimental-tapi-elf-v1",
 frontend::GenerateInterfaceTBEExpV1)
+  .Case("experimental-ifs-v1",
+frontend::GenerateInterfaceIfsExpV1)
   .Default(llvm::None);
   if (!ProgramAction)
 Diags.Report(diag::err_drv_invalid_value)
 << "Must specify a valid inter

[PATCH] D66446: [clang][IFS] Adding new Interface Stubs format.

2019-08-19 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi marked an inline comment as done.
plotfi added inline comments.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:363
+  OS << "IfsVersion: 1.0\n";
+  OS << "Triple: " << T.str() << "\n";
+  OS << "Symbols:\n";

This is missing ObjectFileFormat: ELF


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66446/new/

https://reviews.llvm.org/D66446



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


[PATCH] D65453: [analyzer] Improve the accuracy of the Clang call graph analysis

2019-08-19 Thread Joshua Cranmer via Phabricator via cfe-commits
jcranmer-intel updated this revision to Diff 215999.
jcranmer-intel added a comment.

I think there are use cases for having a callgraph that errs on the side of 
adding edges that might not exist, but I'm happy enough to leave that for a 
later patch.

This does remind me that we need a real clang IR that we can leverage for this 
sort of thing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65453/new/

https://reviews.llvm.org/D65453

Files:
  clang/include/clang/Analysis/CallGraph.h
  clang/lib/Analysis/CallGraph.cpp
  clang/test/Analysis/cxx-callgraph.cpp

Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -79,6 +79,34 @@
 VisitChildren(CE);
   }
 
+  void VisitLambdaExpr(LambdaExpr *LE) {
+if (CXXMethodDecl *MD = LE->getCallOperator())
+  G->VisitFunctionDecl(MD);
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (FunctionDecl *FD = E->getOperatorNew())
+  addCalledDecl(FD);
+VisitChildren(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+CXXConstructorDecl *Ctor = E->getConstructor();
+if (FunctionDecl *Def = Ctor->getDefinition())
+  addCalledDecl(Def);
+VisitChildren(E);
+  }
+
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -143,13 +171,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {
Index: clang/include/clang/Analysis/CallGraph.h
===
--- clang/include/clang/Analysis/CallGraph.h
+++ clang/include/clang/Analysis/CallGraph.h
@@ -131,6 +131,7 @@
 
   bool shouldWalkTypesOfTypeLocs() const { return false; }
   bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
 
 private:
   /// Add the given declaration to the call graph.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-19 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl added inline comments.



Comment at: test/CodeGen/debug-info-no-location.cpp:4
+// CHECK-NEXT: ret void, !dbg !51
+// CHECK: !48 = distinct !DISubprogram(name: "`dynamic atexit destructor for 
'f'", scope: !3, file: !3, line: 16, type: !49, scopeLine: 16, spFlags: 
DISPFlagLocalToUnit | DISPFlagDefinition, unit: !27, retainedNodes: !29)
+// CHECK: !51 = !DILocation(line: 16, scope: !48)

rnk wrote:
> aprantl wrote:
> > Please don't hardcode metadata numbers in the tests, this is bound to break 
> > almost immediately.
> > 
> > It seems like this function is compiler-generated and thus should be marked 
> > as artificial?
> I think using an artificial location (line 0) is consistent with other calls 
> to StartFunction in CGDeclCXX.
I was referring to the `DIFlag::Artificial` that translates into 
`DISubprogram::isArtificial()` here. The line 0 is orthogonal, but also nice to 
have!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328



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


[PATCH] D66394: clang-cl: Enable /Zc:twoPhase by default if targeting MSVC 2017 update 3 or newer

2019-08-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm! I can't believe we didn't notice this for a year. Not long ago a Firefox 
developer filed a bug for a crash with `-fdelayed-template-parsing`, and if 
this works for them, we won't have to debug it. :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66394/new/

https://reviews.llvm.org/D66394



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


[PATCH] D66437: Sema: Create a no-op implicit cast for lvalue function conversions.

2019-08-19 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc created this revision.
pcc added a reviewer: rsmith.
Herald added a project: clang.

This fixes an assertion failure in the case where an implicit conversion for a
function call involves an lvalue function conversion, and makes the AST for
initializations involving implicit lvalue function conversions more accurate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66437

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCXX/implicit-function-conversion.cpp

Index: clang/test/CodeGenCXX/implicit-function-conversion.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/implicit-function-conversion.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-unknown-linux -std=c++17 | FileCheck %s
+
+double a(double) noexcept;
+int b(double (&)(double));
+
+// CHECK: call i32 @_Z1bRFddE(double (double)* @_Z1ad)
+int c = b(a);
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -4301,7 +4301,8 @@
QualType OrigT1, QualType OrigT2,
bool &DerivedToBase,
bool &ObjCConversion,
-   bool &ObjCLifetimeConversion) {
+   bool &ObjCLifetimeConversion,
+   bool &FunctionConversion) {
   assert(!OrigT1->isReferenceType() &&
 "T1 must be the pointee type of the reference type");
   assert(!OrigT2->isReferenceType() && "T2 cannot be a reference type");
@@ -4331,15 +4332,16 @@
Context.canBindObjCObjectType(UnqualT1, UnqualT2))
 ObjCConversion = true;
   else if (UnqualT2->isFunctionType() &&
-   IsFunctionConversion(UnqualT2, UnqualT1, ConvertedT2))
+   IsFunctionConversion(UnqualT2, UnqualT1, ConvertedT2)) {
 // C++1z [dcl.init.ref]p4:
 //   cv1 T1" is reference-compatible with "cv2 T2" if [...] T2 is "noexcept
 //   function" and T1 is "function"
 //
 // We extend this to also apply to 'noreturn', so allow any function
 // conversion between function types.
+FunctionConversion = true;
 return Ref_Compatible;
-  else
+  } else
 return Ref_Incompatible;
 
   // At this point, we know that T1 and T2 are reference-related (at
@@ -4420,6 +4422,7 @@
   bool DerivedToBase = false;
   bool ObjCConversion = false;
   bool ObjCLifetimeConversion = false;
+  bool FunctionConversion = false;
 
   // If we are initializing an rvalue reference, don't permit conversion
   // functions that return lvalues.
@@ -4432,12 +4435,13 @@
 
   if (!ConvTemplate &&
   S.CompareReferenceRelationship(
-DeclLoc,
-Conv->getConversionType().getNonReferenceType()
-  .getUnqualifiedType(),
-DeclType.getNonReferenceType().getUnqualifiedType(),
-DerivedToBase, ObjCConversion, ObjCLifetimeConversion) ==
-  Sema::Ref_Incompatible)
+  DeclLoc,
+  Conv->getConversionType()
+  .getNonReferenceType()
+  .getUnqualifiedType(),
+  DeclType.getNonReferenceType().getUnqualifiedType(),
+  DerivedToBase, ObjCConversion, ObjCLifetimeConversion,
+  FunctionConversion) == Sema::Ref_Incompatible)
 continue;
 } else {
   // If the conversion function doesn't return a reference type,
@@ -4541,11 +4545,11 @@
   bool DerivedToBase = false;
   bool ObjCConversion = false;
   bool ObjCLifetimeConversion = false;
+  bool FunctionConversion = false;
   Expr::Classification InitCategory = Init->Classify(S.Context);
-  Sema::ReferenceCompareResult RefRelationship
-= S.CompareReferenceRelationship(DeclLoc, T1, T2, DerivedToBase,
- ObjCConversion, ObjCLifetimeConversion);
-
+  Sema::ReferenceCompareResult RefRelationship = S.CompareReferenceRelationship(
+  DeclLoc, T1, T2, DerivedToBase, ObjCConversion, ObjCLifetimeConversion,
+  FunctionConversion);
 
   // C++0x [dcl.init.ref]p5:
   //   A reference to type "cv1 T1" is initialized by an expression
@@ -4972,9 +4976,10 @@
   bool dummy1 = false;
   bool dummy2 = false;
   bool dummy3 = false;
+  bool dummy4 = false;
   Sema::ReferenceCompareResult RefRelationship =
   S.CompareReferenceRelationship(From->getBeginLoc(), T1, T2, dummy1,
- dummy2, dummy3);
+ dummy2, dummy3, dummy4);
 
   if (RefRelationship >= Sema::Ref_Related) {
 return TryReferenceInit(S, Init, ToType, /*FIXME*/ From->getBeginLoc(),
Index: clang/lib/Sema/SemaInit.cpp
===

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

2019-08-19 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

In D66364#1635814 , @aaron.ballman 
wrote:

> [ ...]
>
> Adding some libc++ maintainers to see if they have opinions.
>
> `__extension__` is one option. Could we get away with push/pop disabling of 
> the diagnostic? Or perhaps this is a situation where we should not diagnose 
> use within a system header in the first place, because that's part of the 
> implementation?


I just learned about `__extension__`, but from my perspective it makes sense to 
mark uses of `_Atomic` with `__extension__` (or disable the warning with a 
`#pragma`) inside libc++ if we're using something non-standard for the current 
dialect. I don't think Clang should bend its back for libc++ in this case.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66364/new/

https://reviews.llvm.org/D66364



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


[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

>> suggested source-code fixit of #define ALPHA_OFFSET 0x3

No, there is a note “replace with 0x2 ^ ALPHA_OFFSET” to silence it.

While we can suggest as you wrote:
#define ALPHA_OFFSET 0x3

I really dont think it is worth to do.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

In D66397#1635796 , @xbolva00 wrote:

> Now, Chromium disabled this warning :( until we “fix” it.


If Chromium is willing to make build-system changes to add `-Wno-xor-as-pow`, 
then surely they should also be willing to make the suggested source-code fixit 
of `#define ALPHA_OFFSET 0x3` where hexadecimal indicates an intentional 
bitwise operation. Let's give them a little time to react to that suggestion. :)
(A heavier workaround would be to use `xor` instead of `^`.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I'd be happy to take this patch, address the comments, and land it, if you 
don't want to deal with all the nits.




Comment at: test/CodeGen/debug-info-no-location.cpp:1
+// RUN: %clang -cc1 -emit-llvm %s -gcodeview -debug-info-kind=limited -o - | 
FileCheck %s
+// CHECK: call void @"??1?$c@UBQEAA@XZ"(%struct.c* 
@"?f@?1??d@@YAPEAU?$c@UBXZ@4U2@A"), !dbg !51

I think the `%clang_cc1` substitution is preferred, it sets the resource 
directory.



Comment at: test/CodeGen/debug-info-no-location.cpp:4
+// CHECK-NEXT: ret void, !dbg !51
+// CHECK: !48 = distinct !DISubprogram(name: "`dynamic atexit destructor for 
'f'", scope: !3, file: !3, line: 16, type: !49, scopeLine: 16, spFlags: 
DISPFlagLocalToUnit | DISPFlagDefinition, unit: !27, retainedNodes: !29)
+// CHECK: !51 = !DILocation(line: 16, scope: !48)

aprantl wrote:
> Please don't hardcode metadata numbers in the tests, this is bound to break 
> almost immediately.
> 
> It seems like this function is compiler-generated and thus should be marked 
> as artificial?
I think using an artificial location (line 0) is consistent with other calls to 
StartFunction in CGDeclCXX.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328



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


[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

2019-08-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Whoa, the test cases look AMAZING! I admit that I haven't read much of the 
other code just yet, but I like the results in any case.




Comment at: clang/test/Analysis/cast-value-notes.cpp:5-27
 namespace llvm {
 template 
 const X *cast(Y Value);
 
 template 
 const X *dyn_cast(Y *Value);
 template 

Hmm, we may want to move these eventually to `clang/test/Inputs/`, since this 
isn't the only LLVM checker. Thought that said, I want to see the other LLVM 
checker gone, like, purged from the codebase for being the abomination that it 
is.

Feel free to leave these here for know, just thinking aloud :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66423/new/

https://reviews.llvm.org/D66423



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


[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: mclow.lists, ldionne.
aaron.ballman added a comment.
Herald added a subscriber: dexonsmith.

In D66364#1635795 , @rsmith wrote:

> In D66364#1633981 , @aaron.ballman 
> wrote:
>
> > My motivation is for portability. _Thread_local (and all the rest) do not 
> > exist in C99 or earlier (or C++), so having some way to warn users of that 
> > is useful. I agree that we should be consistent and go with all or none, 
> > but my preference is for all (esp since this is a -pedantic warning class).
>
>
> OK, if the motivation is to catch cases where people thought they were 
> writing portable C99 code, but they weren't then I can see this being a 
> useful warning. And that suggests that we should warn on all C11 `_Keywords` 
> when used in C99 or earlier (or in C++). And I suppose it's reasonable to 
> split the hair between "this is code that someone might think is valid C99" 
> (eg, use of `_Static_assert`) and "this is code that is using language 
> extensions that no-one is likely to think is valid C99" (eg, use of 
> `__builtin_*`).
>
> That said, this direction will presumably mean that we start to reject (eg) 
> libc++ when built with `-Wsystem-headers -pedantic-errors` (because it uses 
> `_Atomic` to implement `std::atomic`), which doesn't seem ideal to me. Do you 
> have any thoughts on that? Maybe we should change libc++ to use 
> `__extension__` in those instances?


Adding some libc++ maintainers to see if they have opinions.

`__extension__` is one option. Could we get away with push/pop disabling of the 
diagnostic? Or perhaps this is a situation where we should not diagnose use 
within a system header in the first place, because that's part of the 
implementation?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66364/new/

https://reviews.llvm.org/D66364



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


[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

2019-08-19 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.



In D66267#1632344 , @Charusso wrote:

> I would upstream my hotfix of nullability without any tests as the comment 
> says the intention and also we have plenty of tests of 
> `TrackConstraintBRVisitor`, none of them changed.


Sounds fair.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66267/new/

https://reviews.llvm.org/D66267



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


[PATCH] D66290: [clang] Pragma vectorize_width() implies vectorize(enable)

2019-08-19 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

I think it would be slightly better to split off the change to disable 
vectorization via `llvm.loop.vectorize.enable=false` instead of width=1. This 
changes the behaviour from "disable vectorization, but allow interleaving in 
the vectoriser" to "disable the vectoriser". IMO this change makes sense, but 
it is probably better/safer to do it separately.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66290/new/

https://reviews.llvm.org/D66290



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


[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a subscriber: hans.
xbolva00 added a comment.

>> I think it's too soon to disable any macros.

It is hard to say (this was discussed a lot).

@tkanis or @hans what do you think?  Maybe you could do experiments for us. 
Comment the code which disables this warning in macros and try it on Chromium - 
let’s see how many false positives/negatives it could produce.  This could 
really resolve “macro” question.

But please let’s not repeat same “macro yes/no” arguments from last review.

Anyway, I could make -Wxor-used-as-pow-in-macro so everybody would be happy. Is 
it solution?

Now, Chromium disabled this warning :( until we “fix” it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

2019-08-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D66364#1633981 , @aaron.ballman 
wrote:

> My motivation is for portability. _Thread_local (and all the rest) do not 
> exist in C99 or earlier (or C++), so having some way to warn users of that is 
> useful. I agree that we should be consistent and go with all or none, but my 
> preference is for all (esp since this is a -pedantic warning class).


OK, if the motivation is to catch cases where people thought they were writing 
portable C99 code, but they weren't then I can see this being a useful warning. 
And that suggests that we should warn on all C11 `_Keywords` when used in C99 
or earlier (or in C++). And I suppose it's reasonable to split the hair between 
"this is code that someone might think is valid C99" (eg, use of 
`_Static_assert`) and "this is code that is using language extensions that 
no-one is likely to think is valid C99" (eg, use of `__builtin_*`).

That said, this direction will presumably mean that we start to reject (eg) 
libc++ when built with `-Wsystem-headers -pedantic-errors` (because it uses 
`_Atomic` to implement `std::atomic`), which doesn't seem ideal to me. Do you 
have any thoughts on that? Maybe we should change libc++ to use `__extension__` 
in those instances?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66364/new/

https://reviews.llvm.org/D66364



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


[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D66397#1635745 , @xbolva00 wrote:

> I think it is too soon to jump and disable all macros. We still catch all 
> motivating cases, it found bugs in Chromium.


I think it's too soon to disable any macros. The workaround for Chromium is 
trivial and makes the code more clear.

> Chromium's case makes sense but on the other hand, LHS does not make much 
> sense to be a macro. #define TWO 2  and res =   TWO ^ 32? And even for this 
> case, I think the user should rework it to #define TWO 0x2.

I guess I don't see the distinction. If the user should rework `TWO` to be 
`0x2` when on the LHS, the same holds true for the RHS, to me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


[PATCH] D66365: [clang-tidy] [readability-convert-member-functions-to-static] ignore functions that hide base class methods

2019-08-19 Thread Matthias Gehre via Phabricator via cfe-commits
mgehre added a comment.

I struggle myself to fully understand the motivation behind the bug report. 
This is certainly not code that I would write.
So I don't really care about that kind of code and I didn't want to be in the 
way of other people's way of writing C++.

Alternatively, we could ask the bug reporter to use `// NOLINT` to support his 
specific case. I will try to get some clarification
from the reporter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66365/new/

https://reviews.llvm.org/D66365



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


Re: r369217 - [Diagnostics] Diagnose misused xor as pow

2019-08-19 Thread Dávid Bolvanský via cfe-commits
Yes,
#define ALPHA_OFFSET 0x3

should turn off the diagnostic. (We skip hex decimals).


Dňa 19. 8. 2019 o 20:06 užívateľ Arthur O'Dwyer  
napísal:

>> On Mon, Aug 19, 2019 at 1:53 PM Nico Weber via cfe-commits 
>>  wrote:
> 
>> Hi,
>> 
>> this results in a false positive on webrtc, on this code:
>> 
>> https://cs.chromium.org/chromium/src/third_party/libwebp/src/enc/picture_csp_enc.c?q=picture_csp_enc.c&sq=package:chromium&dr&l=1002
>> 
>> The code does this:
>> 
>> #ifdef WORDS_BIGENDIAN
>> #define ALPHA_OFFSET 0   // uint32_t 0xff00 is 0xff,00,00,00 in memory
>> #else
>> #define ALPHA_OFFSET 3   // uint32_t 0xff00 is 0x00,00,00,ff in memory
>> #endif
>> 
>> // ...
>> 
>> const uint8_t* const argb = (const uint8_t*)picture->argb;
>> const uint8_t* const a = argb + (0 ^ ALPHA_OFFSET);
>> const uint8_t* const r = argb + (1 ^ ALPHA_OFFSET);
>> const uint8_t* const g = argb + (2 ^ ALPHA_OFFSET);
>> const uint8_t* const b = argb + (3 ^ ALPHA_OFFSET);
>> 
>> The idea is to get bytes 0, 1, 2, 3 as a, r, g, b if ALPHA_OFFSET is 0, or 
>> bytes 3, 2, 1, 0 if ALPHA_OFFSET is 3.
>> 
>> Maybe this shouldn't fire if the rhs is a macro?
> 
> Does it show a fix-it that suggests replacing
> #define ALPHA_OFFSET 3
> with
> #define ALPHA_OFFSET 0x3
> ? That would be the quickest way to shut up the warning, if I understand 
> correctly. The use of hexadecimal or octal or binary indicates unambiguously 
> that you mean to do a bitwise operation (xor), not an arithmetic one (pow).
> 
> If the use of a macro here prevents the hex workaround from working, then I'd 
> call that a bug in the diagnostic.
> 
> It would be great if Clang could do what humans do instinctively, and see 
> that this "2 ^ FOO" is actually part of a larger pattern — "0 ^ FOO, 1 ^ FOO, 
> 2 ^ FOO, 3 ^ FOO" — and therefore it probably isn't wrong unless the three 
> surrounding lines are also wrong. However, I'm pretty sure that Clang isn't 
> set up to discover "patterns" like this in general.  It sees "2 ^ 
> LITERAL_EXPONENT" and gives a warning regardless of the surrounding lines.
> 
> –Arthur
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52524: Add -Wno-poison-system-directories flag

2019-08-19 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added reviewers: rtrieu, aaron.ballman.
manojgupta added a comment.

Thanks,

Adding a few more reviewers since I am not very familiar with this part of 
clang.
Please also update the patch description as suggested by @thakis


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524



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


[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

I think it is too soon to jump and disable all macros. We still catch all 
motivating cases, it found bugs in Chromium.

Chromium's case makes sense but on the other hand, LHS does not make much sense 
to be a macro. #define TWO 2  and res =   TWO ^ 32? And even for this case, I 
think the user should rework it to #define TWO 0x2.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


Re: r369281 - Implement P1668R1

2019-08-19 Thread Leonard Chan via cfe-commits
👍 Thanks

On Mon, Aug 19, 2019 at 11:34 AM Keane, Erich  wrote:

> Yeah, sorry about that.  I fixed it in 369284.
>
>
>
> *From:* Leonard Chan [mailto:leonardc...@google.com]
> *Sent:* Monday, August 19, 2019 11:33 AM
> *To:* Keane, Erich 
> *Cc:* cfe-commits cfe 
> *Subject:* Re: r369281 - Implement P1668R1
>
>
>
> Not sure if this was caught by upstream bots already, but we're seeing a
> failing test ob our x64 bots:
>
>
>
> ```
>
> FAIL: Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp (9574 of 15387)
>  TEST 'Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp'
> FAILED 
> Script:
> --
> : 'RUN: at line 1';
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
> -internal-isystem
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
> -nostdsysteminc -std=c++1z -verify -fsyntax-only -fblocks
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
> -fcxx-exceptions
> : 'RUN: at line 2';
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
> -internal-isystem
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
> -nostdsysteminc -std=c++2a -verify -fsyntax-only -fblocks
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
> -fcxx-exceptions
> : 'RUN: at line 3';
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
> -internal-isystem
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
> -nostdsysteminc -std=c++1z -verify -fsyntax-only -fblocks
> -fdelayed-template-parsing
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
> -fcxx-exceptions
> : 'RUN: at line 4';
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
> -internal-isystem
> /b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
> -nostdsysteminc -std=c++14 -verify -fsyntax-only -fblocks
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
> -DCPP14_AND_EARLIER -fcxx-exceptions
> --
> Exit Code: 1
>
> Command Output (stderr):
> --
> error: 'error' diagnostics expected but not seen:
>   File
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
> Line 26 (directive at
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp:28):
> use of this statement in a constexpr function is a C++2a extension
> error: 'warning' diagnostics seen but not expected:
>   File
> /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
> Line 26: use of this statement in a constexpr function is a C++2a extension
> 2 errors generated.
>
> --
>
> 
> Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
> Testing Time: 79.76s
> 
> Failing Tests (1):
> Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp
>
> ```
>
>
>
> Could you look into this? Thanks.
>
>
>
> On Mon, Aug 19, 2019 at 10:39 AM Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: erichkeane
> Date: Mon Aug 19 10:39:59 2019
> New Revision: 369281
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369281&view=rev
> Log:
> Implement P1668R1
>
> Allow inline assembly statements in unexecuted branches of constexpr
> functions.
>
> Modified:
> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
> cfe/trunk/test/Lexer/cxx-features.cpp
> cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
>
> Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=369281&r1=369280&r2=369281&view=diff
>
> ==
> --- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
> +++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Mon Aug 19 10:39:59 2019
> @@ -480,6 +480,7 @@ static void InitializeCPlusPlusFeatureTe
>  Builder.defineMacro("__cpp_user_defined_literals", "200809L");
>  Builder.defineMacro("__cpp_lambdas", "200907L");
>  Builder.defineMacro("__cpp_constexpr",
> +LangOpts.CPlusPlus2a ? "201907L" :
>  LangOpts.CPlusPlus17 ? "201603L" :
>  LangOpts.CPlusPlus14 ? "201304L" : "200704");
>  Builder.defineMacro("__cpp_range_based_for",
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=369281&r1=369280&r2=369281&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Aug 19 10:39:59 2019
> @@ -1995,6 +1995,9 @@ CheckConstexprFunctionStmt(Sema &SemaRef
>  return false;
>  return true;
>
> +  case Stmt::GCCAsmStmtClass:
> +  case Stmt::MSAsmStmtClass:
> + 

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D66397#1635715 , @xbolva00 wrote:

> I agree what @tkanis suggested and be silent if RHS is macro as real world 
> code shows it. Opinions?
>
> @jfb @aaron.ballman


I suspect we can come up with examples where the macro on either lhs or rhs is 
sensible and other examples where its senseless. The existing test cases 
already have:

  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10; did you mean 
'1 << 8' (256)?}}
  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << 8"
  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this 
warning}}
  
  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8; did you mean 
'1 << TEN' (1024)?}}
  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << TEN"
  // expected-note@-2 {{replace expression with '0x2 ^ TEN' to silence this 
warning}}

showing that we expect to warn when macros are involved. If we decide that 
macros should silence the warning, I would expect any use of a macro in the 
expression to silence it, not just a RHS macro.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


[PATCH] D52524: Add -Wno-poison-system-directories flag

2019-08-19 Thread Denis Nikitin via Phabricator via cfe-commits
denik added a comment.

Manoj, please check updated diff.




Comment at: clang/test/Frontend/warning-poison-system-directories.c:12
+// Missing target but included sysroot still causes the warning.
+// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.2.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.2.stderr %s

manojgupta wrote:
> Thanks, Can you also add a test for  -Werror=poison-system-directories .
Added the case with -Werror= and expected failure. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524



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


[PATCH] D52524: Add -Wno-poison-system-directories flag

2019-08-19 Thread Denis Nikitin via Phabricator via cfe-commits
denik updated this revision to Diff 215958.
denik marked 3 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/lib/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/include/c++/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/lib/gcc/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/include/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/lib/.keep
  clang/test/Frontend/warning-poison-system-directories.c


Index: clang/test/Frontend/warning-poison-system-directories.c
===
--- /dev/null
+++ clang/test/Frontend/warning-poison-system-directories.c
@@ -0,0 +1,27 @@
+// System directory and sysroot option causes warning.
+// RUN: %clang -Wpoison-system-directories -target x86_64 -I/usr/include 
--sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-cxx-isystem/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c 
-o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-iquote/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-isystem/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+
+// Missing target but included sysroot still causes the warning.
+// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.2.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.2.stderr %s
+
+// With -Werror the warning causes the failure.
+// RUN: not %clang -Werror=poison-system-directories -target x86_64 
-I/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 
2> %t.3.stderr
+// RUN: FileCheck -check-prefix=ERROR < %t.3.stderr %s
+
+// Cros target without sysroot causes no warning.
+// RUN: %clang -Wpoison-system-directories -Werror -target x86_64 
-I/usr/include -c -o - %s
+
+// By default the warning is off.
+// RUN: %clang -Werror -target x86_64 -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s
+
+// WARN: warning: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Wpoison-system-directories]
+
+// ERROR: error: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Werror,-Wpoison-system-directories]
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -137,6 +137,18 @@
   SmallString<256> MappedPathStorage;
   StringRef MappedPathStr = Path.toStringRef(MappedPathStorage);
 
+  // If use system headers/libraries while cross-compiling,
+  // emit the warning.
+  if (HasSysroot) {
+if (MappedPathStr.startswith("/usr/include") ||
+MappedPathStr.startswith("/usr/local/include") ||
+MappedPathStr.startswith("/lib") ||
+MappedPathStr.startswith("/usr/local/lib")) {
+  Headers.getDiags().Report(diag::warn_poison_system_directories)
+  << MappedPathStr.str();
+}
+  }
+
   // Compute the DirectoryLookup type.
   SrcMgr::CharacteristicKind Type;
   if (Group == Quoted || Group == Angled || Group == IndexHeaderMap) {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1066,6 +1066,10 @@
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
 
+// A warning group for warnings about including system headers when
+// cross-compiling.
+def PoisonSystemDirectories : DiagGroup<"poison-system-directories">;
+
 def NoDeref : DiagGroup<"noderef">;
 
 // A group for cross translation unit static analysis related warnings.
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -304,4 +304,9 @@
 "no analyzer checkers or packages are associated with '%0'">;
 def note_suggest_disabling_all_checkers : Note<
 "use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;
+
+// Poison s

RE: r369281 - Implement P1668R1

2019-08-19 Thread Keane, Erich via cfe-commits
Yeah, sorry about that.  I fixed it in 369284.

From: Leonard Chan [mailto:leonardc...@google.com]
Sent: Monday, August 19, 2019 11:33 AM
To: Keane, Erich 
Cc: cfe-commits cfe 
Subject: Re: r369281 - Implement P1668R1

Not sure if this was caught by upstream bots already, but we're seeing a 
failing test ob our x64 bots:

```
FAIL: Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp (9574 of 15387)
 TEST 'Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp' FAILED 

Script:
--
: 'RUN: at line 1';   
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include 
-nostdsysteminc -std=c++1z -verify -fsyntax-only -fblocks 
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp 
-fcxx-exceptions
: 'RUN: at line 2';   
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include 
-nostdsysteminc -std=c++2a -verify -fsyntax-only -fblocks 
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp 
-fcxx-exceptions
: 'RUN: at line 3';   
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include 
-nostdsysteminc -std=c++1z -verify -fsyntax-only -fblocks 
-fdelayed-template-parsing 
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp 
-fcxx-exceptions
: 'RUN: at line 4';   
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1 
-internal-isystem 
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include 
-nostdsysteminc -std=c++14 -verify -fsyntax-only -fblocks 
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp 
-DCPP14_AND_EARLIER -fcxx-exceptions
--
Exit Code: 1

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen:
  File /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp 
Line 26 (directive at 
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp:28): 
use of this statement in a constexpr function is a C++2a extension
error: 'warning' diagnostics seen but not expected:
  File /b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp 
Line 26: use of this statement in a constexpr function is a C++2a extension
2 errors generated.

--


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

Failing Tests (1):
Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp
```

Could you look into this? Thanks.

On Mon, Aug 19, 2019 at 10:39 AM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Mon Aug 19 10:39:59 2019
New Revision: 369281

URL: http://llvm.org/viewvc/llvm-project?rev=369281&view=rev
Log:
Implement P1668R1

Allow inline assembly statements in unexecuted branches of constexpr
functions.

Modified:
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/Lexer/cxx-features.cpp
cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Mon Aug 19 10:39:59 2019
@@ -480,6 +480,7 @@ static void InitializeCPlusPlusFeatureTe
 Builder.defineMacro("__cpp_user_defined_literals", "200809L");
 Builder.defineMacro("__cpp_lambdas", "200907L");
 Builder.defineMacro("__cpp_constexpr",
+LangOpts.CPlusPlus2a ? "201907L" :
 LangOpts.CPlusPlus17 ? "201603L" :
 LangOpts.CPlusPlus14 ? "201304L" : "200704");
 Builder.defineMacro("__cpp_range_based_for",

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Aug 19 10:39:59 2019
@@ -1995,6 +1995,9 @@ CheckConstexprFunctionStmt(Sema &SemaRef
 return false;
 return true;

+  case Stmt::GCCAsmStmtClass:
+  case Stmt::MSAsmStmtClass:
+// C++2a allows inline assembly statements.
   case Stmt::CXXTryStmtClass:
 if (Cxx2aLoc.isInvalid())
   Cxx2aLoc = S->getBeginLoc();

Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.spe

Re: r369281 - Implement P1668R1

2019-08-19 Thread Leonard Chan via cfe-commits
Not sure if this was caught by upstream bots already, but we're seeing a
failing test ob our x64 bots:

```
FAIL: Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp (9574 of 15387)
 TEST 'Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp'
FAILED 
Script:
--
: 'RUN: at line 1';
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
-internal-isystem
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
-nostdsysteminc -std=c++1z -verify -fsyntax-only -fblocks
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
-fcxx-exceptions
: 'RUN: at line 2';
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
-internal-isystem
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
-nostdsysteminc -std=c++2a -verify -fsyntax-only -fblocks
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
-fcxx-exceptions
: 'RUN: at line 3';
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
-internal-isystem
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
-nostdsysteminc -std=c++1z -verify -fsyntax-only -fblocks
-fdelayed-template-parsing
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
-fcxx-exceptions
: 'RUN: at line 4';
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/bin/clang -cc1
-internal-isystem
/b/s/w/ir/k/recipe_cleanup/clangAGxiPQ/llvm_build_dir/lib/clang/10.0.0/include
-nostdsysteminc -std=c++14 -verify -fsyntax-only -fblocks
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
-DCPP14_AND_EARLIER -fcxx-exceptions
--
Exit Code: 1

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen:
  File
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Line 26 (directive at
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp:28):
use of this statement in a constexpr function is a C++2a extension
error: 'warning' diagnostics seen but not expected:
  File
/b/s/w/ir/k/llvm-project/clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
Line 26: use of this statement in a constexpr function is a C++2a extension
2 errors generated.

--


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

Failing Tests (1):
Clang :: SemaCXX/cxx1z-constexpr-lambdas.cpp
```

Could you look into this? Thanks.

On Mon, Aug 19, 2019 at 10:39 AM Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Mon Aug 19 10:39:59 2019
> New Revision: 369281
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369281&view=rev
> Log:
> Implement P1668R1
>
> Allow inline assembly statements in unexecuted branches of constexpr
> functions.
>
> Modified:
> cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
> cfe/trunk/test/Lexer/cxx-features.cpp
> cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
>
> Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=369281&r1=369280&r2=369281&view=diff
>
> ==
> --- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
> +++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Mon Aug 19 10:39:59 2019
> @@ -480,6 +480,7 @@ static void InitializeCPlusPlusFeatureTe
>  Builder.defineMacro("__cpp_user_defined_literals", "200809L");
>  Builder.defineMacro("__cpp_lambdas", "200907L");
>  Builder.defineMacro("__cpp_constexpr",
> +LangOpts.CPlusPlus2a ? "201907L" :
>  LangOpts.CPlusPlus17 ? "201603L" :
>  LangOpts.CPlusPlus14 ? "201304L" : "200704");
>  Builder.defineMacro("__cpp_range_based_for",
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=369281&r1=369280&r2=369281&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Aug 19 10:39:59 2019
> @@ -1995,6 +1995,9 @@ CheckConstexprFunctionStmt(Sema &SemaRef
>  return false;
>  return true;
>
> +  case Stmt::GCCAsmStmtClass:
> +  case Stmt::MSAsmStmtClass:
> +// C++2a allows inline assembly statements.
>case Stmt::CXXTryStmtClass:
>  if (Cxx2aLoc.isInvalid())
>Cxx2aLoc = S->getBeginLoc();
>
> Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp?rev=369281&r1=369280&r2=369281&view=diff
>
> ==
> --- cfe/trunk/t

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 215957.
xbolva00 added a comment.

Do not warn if RHS is macro.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -10,8 +10,10 @@
 #define XOR(x, y) (x ^ y)
 #define TWO 2
 #define TEN 10
+#define IOP 64
 #define TWO_ULL 2ULL
 #define EPSILON 10 ^ -300
+#define ALPHA_OFFSET 3
 
 #define flexor 7
 
@@ -48,9 +50,7 @@
   res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18; did you mean '1 << 16' (65536)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << 16"
   // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
-  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8; did you mean '1 << TEN' (1024)?}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << TEN"
-  // expected-note@-2 {{replace expression with '0x2 ^ TEN' to silence this warning}}
+  res = 2 ^ TEN;
   res = 0x2 ^ 16;
   res = 2 xor 16;
 
@@ -70,7 +70,21 @@
   res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; did you mean '1LL << 32'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL << 32"
   // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
-  res = 2 ^ 64;
+  res = (2 ^ 64) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean '-1LL'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"-1LL"
+  // expected-note@-2 {{replace expression with '0x2 ^ 64' to silence this warning}}
+#define ULLONG_MAX 18446744073709551615ULL
+  res = (2 ^ 64) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean 'ULLONG_MAX'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"ULLONG_MAX"
+  // expected-note@-2 {{replace expression with '0x2 ^ 64' to silence this warning}}
+  res = (2 ^ 64) - 2;
+  res = (2 ^ IOP) - 1;
+  res = (2 ^ 65) - 1;
+  res = (2 + 64) - 1;
+  res = (3 ^ 64) - 1;
+  res = (2 ^ 8) - 1; // expected-warning {{result of '2 ^ 8' is 10; did you mean '1 << 8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:15}:"1 << 8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
 
   res = EPSILON;
   res = 10 ^ 0; // expected-warning {{result of '10 ^ 0' is 10; did you mean '1e0'?}}
@@ -102,4 +116,5 @@
   res = 10 ^ 5_0b;
   res = 10_0X ^ 5;
 #endif
+  res = res + (2 ^ ALPHA_OFFSET);
 }
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9422,6 +9422,133 @@
 << RHSExpr->getSourceRange();
 }
 
+static void diagnoseXorMisusedAsPow(Sema &S, const ExprResult &LHS,
+const ExprResult &RHS,
+const SourceLocation Loc,
+const Expr *SubLHS = nullptr,
+const Expr *SubRHS = nullptr) {
+  // Do not diagnose xor in macros.
+  if (Loc.isMacroID())
+return;
+
+  // Do not diagnose if xor's rhs is macro.
+  if (RHS.get()->getBeginLoc().isMacroID())
+return;
+
+  bool Negative = false;
+  const auto *LHSInt = dyn_cast(LHS.get());
+  const auto *RHSInt = dyn_cast(RHS.get());
+
+  if (!LHSInt)
+return;
+  if (!RHSInt) {
+// Check negative literals.
+if (const auto *UO = dyn_cast(RHS.get())) {
+  if (UO->getOpcode() != UO_Minus)
+return;
+  RHSInt = dyn_cast(UO->getSubExpr());
+  if (!RHSInt)
+return;
+  Negative = true;
+} else {
+  return;
+}
+  }
+
+  if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth())
+return;
+
+  CharSourceRange ExprRange = CharSourceRange::getCharRange(
+  LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation()));
+  llvm::StringRef ExprStr =
+  Lexer::getSourceText(ExprRange, S.getSourceManager(), S.getLangOpts());
+
+  CharSourceRange XorRange =
+  CharSourceRange::getCharRange(Loc, S.getLocForEndOfToken(Loc));
+  llvm::StringRef XorStr =
+  Lexer::getSourceText(XorRange, S.getSourceManager(), S.getLangOpts());
+  // Do not diagnose if xor keyword/macro is used.
+  if (XorStr == "xor")
+return;
+
+  const llvm::APInt &LeftSideValue = LHSInt->getValue();
+  const llvm::APInt &RightSideValue = RHSInt->getValue();
+  const llvm::APInt XorValue = LeftSideValue ^ RightSideValue;
+
+  std::string LHSStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(LHSInt->getSourceRange()),
+  S.getSourceManager(), S.getLangOpts());
+  std::string RHSStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(RHSInt->getSourceRange()),
+  S.getSourceManager(), S.getLangOpts());
+
+  int64_t RightSideIntValue = Rig

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a subscriber: jfb.
xbolva00 added a comment.

I agree what @tkanis suggested and be silent if RHS is macro as real world code 
shows it. Opinions?

@jfb @aaron.ballman


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-19 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

"debug-info-no-location.cpp" is an extremely generic name for a very specific 
case.  "debug-info-atexit-stub.cpp" would be better.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66328/new/

https://reviews.llvm.org/D66328



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


[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

From post commit discussion, Nico Weber:

1. Hi,
2.
3. this results in a false positive on webrtc, on this code:
4.
5. 
https://cs.chromium.org/chromium/src/third_party/libwebp/src/enc/picture_csp_enc.c?q=picture_csp_enc.c&sq=package:chromium&dr&l=1002
6.
7. The code does this:
8.
9. #ifdef WORDS_BIGENDIAN
10. #define ALPHA_OFFSET 0   // uint32_t 0xff00 is 0xff,00,00,00 in memory
11. #else
12. #define ALPHA_OFFSET 3   // uint32_t 0xff00 is 0x00,00,00,ff in memory
13. #endif
14.
15. // ...
16.
17. const uint8_t* const argb = (const uint8_t*)picture->argb;
18. const uint8_t* const a = argb + (0 ^ ALPHA_OFFSET);
19. const uint8_t* const r = argb + (1 ^ ALPHA_OFFSET);
20. const uint8_t* const g = argb + (2 ^ ALPHA_OFFSET);
21. const uint8_t* const b = argb + (3 ^ ALPHA_OFFSET);
22.
23. The idea is to get bytes 0, 1, 2, 3 as a, r, g, b if ALPHA_OFFSET is 0, or 
bytes 3, 2, 1, 0 if ALPHA_OFFSET is 3.
24.
25. Maybe this shouldn't fire if the rhs is a macro?
26.
27. (On all of Chromium, this fired 3 times; in the other 2 cases the rhs was a 
literal as well, and those two were true positives.)




Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66397/new/

https://reviews.llvm.org/D66397



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


r369286 - Fix poorly formatted HTML in the cxx_status.html file caused by adding

2019-08-19 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Mon Aug 19 11:14:22 2019
New Revision: 369286

URL: http://llvm.org/viewvc/llvm-project?rev=369286&view=rev
Log:
Fix poorly formatted HTML in the cxx_status.html file caused by adding
1668.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=369286&r1=369285&r2=369286&view=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Mon Aug 19 11:14:22 2019
@@ -1003,14 +1003,15 @@ as the draft C++2a standard evolves.
   

 http://wg21.link/p1331r2";>P1331R2
-No
+No
   
   
 http://wg21.link/p1668r1";>P1668R1
-SVN
+SVN
   
   
 http://wg21.link/p0784r7";>P0784R7
+No
   
 
   Prohibit aggregates with user-declared constructors


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


r369284 - Fix test where diagnostics changed in P1668 implementation

2019-08-19 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Mon Aug 19 11:08:52 2019
New Revision: 369284

URL: http://llvm.org/viewvc/llvm-project?rev=369284&view=rev
Log:
Fix test where diagnostics changed in P1668 implementation

Modified:
cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Modified: cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp?rev=369284&r1=369283&r2=369284&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp Mon Aug 19 11:08:52 2019
@@ -25,7 +25,7 @@ namespace ns1 {
 namespace ns2 {
   auto L = [](int I) constexpr { if (I == 5) asm("non-constexpr");  };
 #if __cpp_constexpr < 201907L
-  //expected-error@-2{{use of this statement in a constexpr function is a 
C++2a extension}}
+  //expected-warning@-2{{use of this statement in a constexpr function is a 
C++2a extension}}
 #endif
 } // end ns1
 
@@ -71,9 +71,16 @@ namespace ns2 {
   static_assert(L(3.14) == 3.14);
 }
 namespace ns3 {
-  auto L = [](auto a) { asm("non-constexpr"); return a; }; 
//expected-note{{declared here}}
+  auto L = [](auto a) { asm("non-constexpr"); return a; };
   constexpr int I =  //expected-error{{must be initialized by a constant 
expression}}
-  L(3); //expected-note{{non-constexpr function}}
+  L(3);
+#if __cpp_constexpr < 201907L
+//expected-note@-2{{non-constexpr function}}
+//expected-note@-5{{declared here}}
+#else
+//expected-note@-7{{subexpression not valid in a constant expression}}
+//expected-note@-6{{in call to}}
+#endif
 } 
 
 } // end ns test_constexpr_call
@@ -170,7 +177,7 @@ static_assert(I == 12);
 namespace contained_lambdas_call_operator_is_not_constexpr {
 constexpr auto f(int i) {
   double d = 3.14;
-  auto L = [=](auto a) { //expected-note{{declared here}}
+  auto L = [=](auto a) {
 int Isz = sizeof(i);
 asm("hello");
 return sizeof(i) + sizeof(a) + sizeof(d); 
@@ -181,8 +188,14 @@ constexpr auto f(int i) {
 constexpr auto L = f(3);
 
 constexpr auto M =  // expected-error{{must be initialized by}} 
-L("abc"); //expected-note{{non-constexpr function}}
-
+L("abc");
+#if __cpp_constexpr < 201907L
+//expected-note@-2{{non-constexpr function}}
+//expected-note@-14{{declared here}}
+#else
+//expected-note@-14{{subexpression not valid in a constant expression}}
+//expected-note@-6{{in call to}}
+#endif
 } // end ns contained_lambdas_call_operator_is_not_constexpr
 
 


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


Re: r369217 - [Diagnostics] Diagnose misused xor as pow

2019-08-19 Thread Arthur O'Dwyer via cfe-commits
On Mon, Aug 19, 2019 at 1:53 PM Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi,
>
> this results in a false positive on webrtc, on this code:
>
>
> https://cs.chromium.org/chromium/src/third_party/libwebp/src/enc/picture_csp_enc.c?q=picture_csp_enc.c&sq=package:chromium&dr&l=1002
>
> The code does this:
>
> #ifdef WORDS_BIGENDIAN
> #define ALPHA_OFFSET 0   // uint32_t 0xff00 is 0xff,00,00,00 in memory
> #else
> #define ALPHA_OFFSET 3   // uint32_t 0xff00 is 0x00,00,00,ff in memory
> #endif
>
> // ...
>
> const uint8_t* const argb = (const uint8_t*)picture->argb;
> const uint8_t* const a = argb + (0 ^ ALPHA_OFFSET);
> const uint8_t* const r = argb + (1 ^ ALPHA_OFFSET);
> const uint8_t* const g = argb + (2 ^ ALPHA_OFFSET);
> const uint8_t* const b = argb + (3 ^ ALPHA_OFFSET);
>
> The idea is to get bytes 0, 1, 2, 3 as a, r, g, b if ALPHA_OFFSET is 0, or
> bytes 3, 2, 1, 0 if ALPHA_OFFSET is 3.
>
> Maybe this shouldn't fire if the rhs is a macro?
>

Does it show a fix-it that suggests replacing
#define ALPHA_OFFSET 3
with
#define ALPHA_OFFSET 0x3
? That would be the quickest way to shut up the warning, if I understand
correctly. The use of hexadecimal or octal or binary indicates
unambiguously that you mean to do a bitwise operation (xor), not an
arithmetic one (pow).

If the use of a macro here *prevents* the hex workaround from working, then
I'd call that a bug in the diagnostic.

It would be great if Clang could do what humans do instinctively, and see
that this "2 ^ FOO" is actually part of a larger pattern — "0 ^ FOO, 1 ^
FOO, 2 ^ FOO, 3 ^ FOO" — and therefore it probably isn't wrong unless the
three surrounding lines are also wrong. However, I'm pretty sure that Clang
isn't set up to discover "patterns" like this in general.  It sees "2 ^
LITERAL_EXPONENT" and gives a warning regardless of the surrounding lines.

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


r369282 - Update cxx_status.html with P1668 status.

2019-08-19 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Mon Aug 19 10:57:27 2019
New Revision: 369282

URL: http://llvm.org/viewvc/llvm-project?rev=369282&view=rev
Log:
Update cxx_status.html with P1668 status.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=369282&r1=369281&r2=369282&view=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Mon Aug 19 10:57:27 2019
@@ -1007,6 +1007,7 @@ as the draft C++2a standard evolves.
   
   
 http://wg21.link/p1668r1";>P1668R1
+SVN
   
   
 http://wg21.link/p0784r7";>P0784R7


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


Re: r369217 - [Diagnostics] Diagnose misused xor as pow

2019-08-19 Thread Nico Weber via cfe-commits
Hi,

this results in a false positive on webrtc, on this code:

https://cs.chromium.org/chromium/src/third_party/libwebp/src/enc/picture_csp_enc.c?q=picture_csp_enc.c&sq=package:chromium&dr&l=1002

The code does this:

#ifdef WORDS_BIGENDIAN
#define ALPHA_OFFSET 0   // uint32_t 0xff00 is 0xff,00,00,00 in memory
#else
#define ALPHA_OFFSET 3   // uint32_t 0xff00 is 0x00,00,00,ff in memory
#endif

// ...

const uint8_t* const argb = (const uint8_t*)picture->argb;
const uint8_t* const a = argb + (0 ^ ALPHA_OFFSET);
const uint8_t* const r = argb + (1 ^ ALPHA_OFFSET);
const uint8_t* const g = argb + (2 ^ ALPHA_OFFSET);
const uint8_t* const b = argb + (3 ^ ALPHA_OFFSET);

The idea is to get bytes 0, 1, 2, 3 as a, r, g, b if ALPHA_OFFSET is 0, or
bytes 3, 2, 1, 0 if ALPHA_OFFSET is 3.

Maybe this shouldn't fire if the rhs is a macro?

(On all of Chromium, this fired 3 times; in the other 2 cases the rhs was a
literal as well, and those two were true positives.)

On Sun, Aug 18, 2019 at 3:13 PM David Bolvansky via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: xbolva00
> Date: Sun Aug 18 12:14:14 2019
> New Revision: 369217
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369217&view=rev
> Log:
> [Diagnostics] Diagnose misused xor as pow
>
> Summary:
> Motivation:
> https://twitter.com/jfbastien/status/1139298419988549632
> https://twitter.com/mikemx7f/status/1139335901790625793
> https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E&search=Search
>
> Reviewers: jfb, rsmith, regehr, aaron.ballman
>
> Reviewed By: aaron.ballman
>
> Subscribers: lebedev.ri, Quuxplusone, erik.pilkington, riccibruno,
> dexonsmith, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D63423
>
> Added:
> cfe/trunk/test/SemaCXX/warn-xor-as-pow.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaExpr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=369217&r1=369216&r2=369217&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Sun Aug 18 12:14:14
> 2019
> @@ -512,6 +512,7 @@ def CompareDistinctPointerType : DiagGro
>  def GNUUnionCast : DiagGroup<"gnu-union-cast">;
>  def GNUVariableSizedTypeNotAtEnd :
> DiagGroup<"gnu-variable-sized-type-not-at-end">;
>  def Varargs : DiagGroup<"varargs">;
> +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">;
>
>  def Unsequenced : DiagGroup<"unsequenced">;
>  // GCC name for -Wunsequenced
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=369217&r1=369216&r2=369217&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Aug 18
> 12:14:14 2019
> @@ -3326,6 +3326,15 @@ def warn_address_of_reference_bool_conve
>"code; pointer may be assumed to always convert to true">,
>InGroup;
>
> +def warn_xor_used_as_pow_base_extra : Warning<
> +  "result of '%0' is %1; did you mean '%2' (%3)?">,
> +  InGroup;
> +def warn_xor_used_as_pow_base : Warning<
> +  "result of '%0' is %1; did you mean '%2'?">,
> +  InGroup;
> +def note_xor_used_as_pow_silence : Note<
> +  "replace expression with '%0' to silence this warning">;
> +
>  def warn_null_pointer_compare : Warning<
>  "comparison of %select{address of|function|array}0 '%1' %select{not
> |}2"
>  "equal to a null pointer is always %select{true|false}2">,
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=369217&r1=369216&r2=369217&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sun Aug 18 12:14:14 2019
> @@ -11011,6 +11011,107 @@ QualType Sema::CheckVectorCompareOperand
>return GetSignedVectorType(vType);
>  }
>
> +static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult
> &RHS,
> +SourceLocation Loc) {
> +  // Do not diagnose macros.
> +  if (Loc.isMacroID())
> +return;
> +
> +  bool Negative = false;
> +  const auto *LHSInt = dyn_cast(LHS.get());
> +  const auto *RHSInt = dyn_cast(RHS.get());
> +
> +  if (!LHSInt)
> +return;
> +  if (!RHSInt) {
> +// Check negative literals.
> +if (const auto *UO = dyn_cast(RHS.get())) {
> +  if (UO->getOpcode() != UO_Minus)
> +return;
> +  RHSInt = dyn_cast(UO->getSubExpr());
> +  if 

[PATCH] D52524: Add -Wno-poison-system-directories flag

2019-08-19 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added inline comments.



Comment at: clang/test/Frontend/warning-poison-system-directories.c:12
+// Missing target but included sysroot still causes the warning.
+// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.2.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.2.stderr %s

Thanks, Can you also add a test for  -Werror=poison-system-directories .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524



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


[PATCH] D52524: Add -Wno-poison-system-directories flag

2019-08-19 Thread Denis Nikitin via Phabricator via cfe-commits
denik marked 2 inline comments as done.
denik added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1071
+// cross-compiling.
+def PoisonSystemDirectories : DiagGroup<"poison-system-directories">;
+

manojgupta wrote:
> Please verify that the warning is not enabled by default.
Added DefaultIgnore flag. Now the warning is off by default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524



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


r369281 - Implement P1668R1

2019-08-19 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Mon Aug 19 10:39:59 2019
New Revision: 369281

URL: http://llvm.org/viewvc/llvm-project?rev=369281&view=rev
Log:
Implement P1668R1

Allow inline assembly statements in unexecuted branches of constexpr
functions.

Modified:
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/Lexer/cxx-features.cpp
cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Mon Aug 19 10:39:59 2019
@@ -480,6 +480,7 @@ static void InitializeCPlusPlusFeatureTe
 Builder.defineMacro("__cpp_user_defined_literals", "200809L");
 Builder.defineMacro("__cpp_lambdas", "200907L");
 Builder.defineMacro("__cpp_constexpr",
+LangOpts.CPlusPlus2a ? "201907L" :
 LangOpts.CPlusPlus17 ? "201603L" :
 LangOpts.CPlusPlus14 ? "201304L" : "200704");
 Builder.defineMacro("__cpp_range_based_for",

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Aug 19 10:39:59 2019
@@ -1995,6 +1995,9 @@ CheckConstexprFunctionStmt(Sema &SemaRef
 return false;
 return true;
 
+  case Stmt::GCCAsmStmtClass:
+  case Stmt::MSAsmStmtClass:
+// C++2a allows inline assembly statements.
   case Stmt::CXXTryStmtClass:
 if (Cxx2aLoc.isInvalid())
   Cxx2aLoc = S->getBeginLoc();

Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp (original)
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp Mon Aug 19 
10:39:59 2019
@@ -136,9 +136,13 @@ constexpr int AllowedStmtsCXX11() {
 }
 
 //  or a compound-statement that does not contain [CXX1Y]
-constexpr int DisallowedStmtsCXX1Y_1() {
+constexpr int DisallowedStmtsCXX1Y_1(bool b) {
   //  - an asm-definition
-  asm("int3"); // expected-error {{statement not allowed in constexpr 
function}}
+  if (b)
+asm("int3");
+#if !defined(CXX2A)
+  // expected-error@-2 {{use of this statement in a constexpr function is a 
C++2a extension}}
+#endif
   return 0;
 }
 constexpr int DisallowedStmtsCXX1Y_2() {

Modified: cfe/trunk/test/Lexer/cxx-features.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/cxx-features.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/test/Lexer/cxx-features.cpp (original)
+++ cfe/trunk/test/Lexer/cxx-features.cpp Mon Aug 19 10:39:59 2019
@@ -203,7 +203,7 @@
 #error "wrong value for __cpp_lambdas"
 #endif
 
-#if check(constexpr, 0, 200704, 201304, 201603, 201603)
+#if check(constexpr, 0, 200704, 201304, 201603, 201907)
 #error "wrong value for __cpp_constexpr"
 #endif
 

Modified: cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp?rev=369281&r1=369280&r2=369281&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp Mon Aug 19 10:39:59 2019
@@ -23,7 +23,10 @@ namespace ns1 {
 } // end ns1
 
 namespace ns2 {
-  auto L = [](int I) constexpr { asm("non-constexpr");  }; 
//expected-error{{not allowed in constexpr function}}
+  auto L = [](int I) constexpr { if (I == 5) asm("non-constexpr");  };
+#if __cpp_constexpr < 201907L
+  //expected-error@-2{{use of this statement in a constexpr function is a 
C++2a extension}}
+#endif
 } // end ns1
 
 // This is not constexpr until C++20, as the requirements on constexpr


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


[PATCH] D52524: Add -Wno-poison-system-directories flag

2019-08-19 Thread Denis Nikitin via Phabricator via cfe-commits
denik updated this revision to Diff 215945.
denik added a comment.

Changed Wpoison-system-directories warning to be disabled by default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52524/new/

https://reviews.llvm.org/D52524

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/lib/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/include/c++/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/lib/gcc/.keep
  
clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/include/.keep
  clang/test/Frontend/Inputs/sysroot_x86_64_cross_linux_tree/usr/local/lib/.keep
  clang/test/Frontend/warning-poison-system-directories.c


Index: clang/test/Frontend/warning-poison-system-directories.c
===
--- /dev/null
+++ clang/test/Frontend/warning-poison-system-directories.c
@@ -0,0 +1,21 @@
+// System directory and sysroot option causes warning.
+// RUN: %clang -Wpoison-system-directories -target x86_64 -I/usr/include 
--sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-cxx-isystem/usr/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree -c 
-o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-iquote/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+// RUN: %clang -Wpoison-system-directories -target x86_64 
-isystem/usr/local/include --sysroot %S/Inputs/sysroot_x86_64_cross_linux_tree 
-c -o - %s 2> %t.1.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.1.stderr %s
+
+// Missing target but included sysroot still causes the warning.
+// RUN: %clang -Wpoison-system-directories -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s 2> %t.2.stderr
+// RUN: FileCheck -check-prefix=WARN < %t.2.stderr %s
+
+// Cros target without sysroot causes no warning.
+// RUN: %clang -Wpoison-system-directories -Werror -target x86_64 
-I/usr/include -c -o - %s
+
+// By default the warning is off.
+// RUN: %clang -Werror -target x86_64 -I/usr/include --sysroot 
%S/Inputs/sysroot_x86_64_cross_linux_tree -c -o - %s
+
+// WARN: warning: include location {{[^ ]+}} is unsafe for cross-compilation 
[-Wpoison-system-directories]
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -137,6 +137,18 @@
   SmallString<256> MappedPathStorage;
   StringRef MappedPathStr = Path.toStringRef(MappedPathStorage);
 
+  // If use system headers/libraries while cross-compiling,
+  // emit the warning.
+  if (HasSysroot) {
+if (MappedPathStr.startswith("/usr/include") ||
+MappedPathStr.startswith("/usr/local/include") ||
+MappedPathStr.startswith("/lib") ||
+MappedPathStr.startswith("/usr/local/lib")) {
+  Headers.getDiags().Report(diag::warn_poison_system_directories)
+  << MappedPathStr.str();
+}
+  }
+
   // Compute the DirectoryLookup type.
   SrcMgr::CharacteristicKind Type;
   if (Group == Quoted || Group == Angled || Group == IndexHeaderMap) {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1066,6 +1066,10 @@
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
 
+// A warning group for warnings about including system headers when
+// cross-compiling.
+def PoisonSystemDirectories : DiagGroup<"poison-system-directories">;
+
 def NoDeref : DiagGroup<"noderef">;
 
 // A group for cross translation unit static analysis related warnings.
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -304,4 +304,9 @@
 "no analyzer checkers or packages are associated with '%0'">;
 def note_suggest_disabling_all_checkers : Note<
 "use -analyzer-disable-all-checks to disable all static analyzer 
checkers">;
+
+// Poison system directories.
+def warn_poison_system_directories : Warning <
+  "include location '%0' is unsafe for cross-compilation">,
+  InGroup, DefaultIgnore;
 }


Index: clang/test/Frontend/warning-poison-system-directories.c
===
--- /dev/null
+++ clang/test/Frontend/warni

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

2019-08-19 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.
Herald added a reviewer: jdoerfert.

I had some time to work on this finally late last week. I decided the most 
straightforward thing was to implement the necessary interface changes to the 
TLI analysis to make it require a Function (without any changes yet to how that 
analysis operates). See D66428  that I just 
mailed for review. That takes care of the most widespread changes needed for 
this migration, and afterwards we can change the analysis to look at the 
function attributes and make a truly per-function TLI.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61634/new/

https://reviews.llvm.org/D61634



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


[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

2019-08-19 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth updated this revision to Diff 215940.
paulkirth added a comment.

Fix missing context in prior diff


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66324/new/

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-no-warning-without-flag.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll

Index: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
@@ -138,7 +138,7 @@
   %conv1 = sext i32 %conv to i64
   %expval = call i64 @llvm.expect.i64(i64 %conv1, i64 0)
   %tobool = icmp ne i64 %expval, 0
-; CHECK: !prof !1
+; CHECK: !prof !2
 ; CHECK-NOT: @llvm.expect
   br i1 %tobool, label %if.then, label %if.end
 
@@ -165,7 +165,7 @@
   %tmp = load i32, i32* %x.addr, align 4
   %conv = sext i32 %tmp to i64
   %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1)
-; CHECK: !prof !2
+; CHECK: !prof !4
 ; CHECK-NOT: @llvm.expect
   switch i64 %expval, label %sw.epilog [
 i64 1, label %sw.bb
@@ -194,7 +194,7 @@
   %tmp = load i32, i32* %x.addr, align 4
   %conv = sext i32 %tmp to i64
   %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1)
-; CHECK: !prof !3
+; CHECK: !prof !5
 ; CHECK-NOT: @llvm.expect
   switch i64 %expval, label %sw.epilog [
 i64 2, label %sw.bb
@@ -278,7 +278,7 @@
   %t7 = call i64 @llvm.expect.i64(i64 %t6, i64 0)
   %t8 = icmp ne i64 %t7, 0
   %t9 = select i1 %t8, i32 1, i32 2
-; CHECK: select{{.*}}, !prof !1
+; CHECK: select{{.*}}, !prof !2
   ret i32 %t9
 }
 
@@ -286,6 +286,6 @@
 declare i1 @llvm.expect.i1(i1, i1) nounwind readnone
 
 ; CHECK: !0 = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2000}
-; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000, i32 1}
-; CHECK: !3 = !{!"branch_weights", i32 2000, i32 1, i32 1}
+; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}
+; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}
Index: llvm/test/ThinLTO/X86/lazyload_metadata.ll
===
--- llvm/test/ThinLTO/X86/lazyload_metadata.ll
+++ llvm/test/ThinLTO/X86/lazyload_metadata.ll
@@ -10,13 +10,13 @@
 ; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc \
 ; RUN:  -o /dev/null -stats \
 ; RUN:  2>&1 | FileCheck %s -check-prefix=LAZY
-; LAZY: 61 bitcode-reader  - Number of Metadata records loaded
+; LAZY: 63 bitcode-reader  - Number of Metadata records loaded
 ; LAZY: 2 bitcode-reader  - Number of MDStrings loaded
 
 ; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc \
 ; RUN:  -o /dev/null -disable-ondemand-mds-loading -stats \
 ; RUN:  2>&1 | FileCheck %s -check-prefix=NOTLAZY
-; NOTLAZY: 70 bitcode-reader  - Number of Metadata records loaded
+; NOTLAZY: 72 bitcode-reader  - Number of Metadata records loaded
 ; NOTLAZY: 7 bitcode-reader  - Number of MDStrings loaded
 
 
Index: llvm/lib/Transforms/Utils/MisExpect.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Utils/MisExpect.cpp
@@ -0,0 +1,125 @@
+//===--- MisExpect.cpp - Check Use of __builtin_expect() with PGO data ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This contains c

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

2019-08-19 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth updated this revision to Diff 215938.
paulkirth edited the summary of this revision.
paulkirth added a comment.

- Update CodeGenOptions to remove -fmisexpect
- Access the LLVMContext directly
- Add -line-tables-only for more accurate diagnostics

Fixes various issues with tests and cleans up some residual code from the 
original frontend implementation


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66324/new/

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/lib/Transforms/Utils/MisExpect.cpp

Index: llvm/lib/Transforms/Utils/MisExpect.cpp
===
--- llvm/lib/Transforms/Utils/MisExpect.cpp
+++ llvm/lib/Transforms/Utils/MisExpect.cpp
@@ -114,7 +114,7 @@
   mdconst::dyn_extract(MD->getOperand(i));
   RealWeights[i - 1] = Value->getZExtValue();
 }
-misexpect::verifyMisExpect(&I, RealWeights, I.getParent()->getParent()->getContext());
+misexpect::verifyMisExpect(&I, RealWeights, I.getContext());
   }
 }
   }
Index: clang/test/Profile/misexpect-switch.c
===
--- clang/test/Profile/misexpect-switch.c
+++ clang/test/Profile/misexpect-switch.c
@@ -1,7 +1,7 @@
 // Test that misexpect detects mis-annotated switch statements
 
 // RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
-// RUN: %clang_cc1 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect
+// RUN: %clang_cc1 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect -debug-info-kind=line-tables-only
 
 int sum(int *buff, int size);
 int random_sample(int *buff, int size);
Index: clang/test/Profile/misexpect-switch-only-default-case.c
===
--- clang/test/Profile/misexpect-switch-only-default-case.c
+++ clang/test/Profile/misexpect-switch-only-default-case.c
@@ -1,7 +1,7 @@
 // Test that misexpect emits no warning when there is only one switch case
 
 // RUN: llvm-profdata merge %S/Inputs/misexpect-switch-default-only.proftext -o %t.profdata
-// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+// RUN: %clang_cc1 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect -debug-info-kind=line-tables-only
 
 // expected-no-diagnostics
 int sum(int *buff, int size);
Index: clang/test/Profile/misexpect-switch-default.c
===
--- clang/test/Profile/misexpect-switch-default.c
+++ clang/test/Profile/misexpect-switch-default.c
@@ -1,7 +1,7 @@
 // Test that misexpect detects mis-annotated switch statements for default case
 
-// RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
-// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch-default.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect -debug-info-kind=line-tables-only
 
 int sum(int *buff, int size);
 int random_sample(int *buff, int size);
@@ -17,26 +17,24 @@
 int main() {
   init_arry();
   int val = 0;
-
-  int j, k;
-  for (j = 0; j < outer_loop; ++j) {
-for (k = 0; k < inner_loop; ++k) {
-  unsigned condition = rand() % 5;
-  switch (__builtin_expect(condition, 6)) { // expected-warning-re {{Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.+}}% of profiled executions.}}
-  case 0:
-val += sum(arry, arry_size);
-break;
-  case 1:
-  case 2:
-  case 3:
-  case 4:
-val += random_sample(arry, arry_size);
-break;
-  default:
-__builtin_unreachable();
-  } // end switch
-}   // end inner_loop
-  } // end outer_loop
+  int j;
+  for (j = 0; j < outer_loop * inner_loop; ++j) {
+unsigned condition = rand() % 5;
+switch (__builtin_expect(condition, 6)) { // expected-warning-re {{Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.+}}% of profiled executions.}}
+case 0:
+  val += sum(arry, arry_size);
+  break;
+case 1:
+case 2:
+case 3:
+  break;
+case 4:
+  val += random_sample(arry, arry_size);
+  break;
+default:
+  __builtin_unreachable();
+} // end switch
+  }   // end outer_loop
 
   return 0;
 }
Index: clang/test/Profile/misexpect-branch.c
=

[PATCH] D64418: [Docs][OpenCL] Documentation of C++ for OpenCL mode

2019-08-19 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh accepted this revision.
svenvh added a comment.

LGTM!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64418/new/

https://reviews.llvm.org/D64418



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


[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

2019-08-19 Thread Princeton Ferro via Phabricator via cfe-commits
Prince781 added a comment.

In D66122#1633990 , @efriedma wrote:

> I think we should send a defect report to the C++ standards committee to 
> clarify the ambiguity here.


I followed the instructions on this page  
and sent it to std-discussion first.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66122/new/

https://reviews.llvm.org/D66122



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


[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

2019-08-19 Thread Johan Vikström via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
jvikstrom marked 2 inline comments as done.
Closed by commit rL369275: [clangd] Added highlighting for tokens that are 
macro arguments. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64741?vs=215907&id=215929#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64741/new/

https://reviews.llvm.org/D64741

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -39,7 +39,27 @@
 llvm::sort(Tokens);
 auto Last = std::unique(Tokens.begin(), Tokens.end());
 Tokens.erase(Last, Tokens.end());
-return Tokens;
+// Macros can give tokens that have the same source range but conflicting
+// kinds. In this case all tokens sharing this source range should be
+// removed.
+std::vector NonConflicting;
+NonConflicting.reserve(Tokens.size());
+for (ArrayRef TokRef = Tokens; !TokRef.empty();) {
+  ArrayRef Conflicting =
+  TokRef.take_while([&](const HighlightingToken &T) {
+// TokRef is guaranteed at least one element here because otherwise
+// this predicate would never fire.
+return T.R == TokRef.front().R;
+  });
+  // If there is exactly one token with this range it's non conflicting and
+  // should be in the highlightings.
+  if (Conflicting.size() == 1)
+NonConflicting.push_back(TokRef.front());
+  // TokRef[Conflicting.size()] is the next token with a different range (or
+  // the end of the Tokens).
+  TokRef = TokRef.drop_front(Conflicting.size());
+}
+return NonConflicting;
   }
 
   bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
@@ -236,13 +256,18 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-if (Loc.isMacroID())
-  // FIXME: skip tokens inside macros for now.
-  return;
+if(Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
 
 // Non top level decls that are included from a header are not filtered by
 // topLevelDecls. (example: method declarations being included from another
 // file for a class from another file)
+// There are also cases with macros where the spelling loc will not be in the
+// main file and the highlighting would be incorrect.
 if (!isInsideMainFile(Loc, SM))
   return;
 
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -16,10 +16,6 @@
 
 namespace clang {
 namespace clangd {
-  void PrintTo(const HighlightingToken &T, ::std::ostream *OS) {
-  *OS << "(" << T.R.start.line << ", " << T.R.start.character << ") -> (" << T.R.end.line << ", " << T.R.end.character << "): " << (int)T.Kind;
-}
-
 namespace {
 
 std::vector
@@ -380,6 +376,56 @@
 $Class[[G]]<$Class[[F]], &$Class[[F]]::$Method[[f]]> $Variable[[GG]];
 $Variable[[GG]].$Method[[foo]](&$Variable[[FF]]);
 $Class[[A]]<$Function[[foo]]> $Variable[[AA]];
+)cpp",
+// Tokens that share a source range but have conflicting Kinds are not
+// highlighted.
+R"cpp(
+  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);
+  DEF_CLASS($Class[[A]])
+  #define MACRO_CONCAT(X, V, T) T foo##X = V
+  #define DEF_VAR(X, V) int X = V
+  #define DEF_VAR_T(T, X, V) T X = V
+  #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
+  #define CPY(X) X
+  #define DEF_VAR_TYPE(X, Y) X Y
+  #define SOME_NAME variable
+  #define SOME_NAME_SET variable2 = 123
+  #define INC_VAR(X) X += 2
+  $Primitive[[void]] $Function[[foo]]() {
+DEF_VAR($Variable[[X]],  123);
+DEF_VAR_REV(908, $Variable[[XY]]);
+$Primitive[[int]] CPY( $Variable[[XX]] );
+DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+$Primitive[[double]] SOME_NAME;
+$Primitive[[int]] SOME_NAME_SET;
+$Variable[[variable]] = 20.1;
+MACRO_CONCAT(var, 2, $Primitive[[float]]);
+DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+  CPY($Class[[A]]()));
+INC_VAR($Variable[[variable]]);
+  }
+  $Primitive[[

[clang-tools-extra] r369275 - [clangd] Added highlighting for tokens that are macro arguments.

2019-08-19 Thread Johan Vikstrom via cfe-commits
Author: jvikstrom
Date: Mon Aug 19 09:27:49 2019
New Revision: 369275

URL: http://llvm.org/viewvc/llvm-project?rev=369275&view=rev
Log:
[clangd] Added highlighting for tokens that are macro arguments.

Summary:
Adds semantic highlighting for tokens that are a macro argument.
Example:
```
D_V(SomeVar);
```
The "SomeVar" inside the macro is highlighted as a variable now.

Tokens that are in a macro body expansion are ignored in this patch for three 
reasons.
* The spelling loc is inside the macro "definition" meaning it would highlight 
inside the macro definition (could probably easily be fixed by using 
getExpansionLoc instead of getSpellingLoc?)
* If wanting to highlight the macro definition this could create duplicate 
tokens. And if the tokens are of different types there would be conflicts 
(tokens in the same range but with different types). Say a macro defines some 
name and both a variable declaration and a function use this, there would be 
two tokens in the macro definition but one with Kind "Variable" and the other 
with Kind "Function".
* Thirdly, macro body expansions could come from a file that is not the main 
file (easily fixed, just check that the Loc is in the main file and not even a 
problem if we wanted to highlight the actual macro "invocation")

Reviewers: hokein, sammccall, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=369275&r1=369274&r2=369275&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Aug 19 09:27:49 
2019
@@ -39,7 +39,27 @@ public:
 llvm::sort(Tokens);
 auto Last = std::unique(Tokens.begin(), Tokens.end());
 Tokens.erase(Last, Tokens.end());
-return Tokens;
+// Macros can give tokens that have the same source range but conflicting
+// kinds. In this case all tokens sharing this source range should be
+// removed.
+std::vector NonConflicting;
+NonConflicting.reserve(Tokens.size());
+for (ArrayRef TokRef = Tokens; !TokRef.empty();) {
+  ArrayRef Conflicting =
+  TokRef.take_while([&](const HighlightingToken &T) {
+// TokRef is guaranteed at least one element here because otherwise
+// this predicate would never fire.
+return T.R == TokRef.front().R;
+  });
+  // If there is exactly one token with this range it's non conflicting and
+  // should be in the highlightings.
+  if (Conflicting.size() == 1)
+NonConflicting.push_back(TokRef.front());
+  // TokRef[Conflicting.size()] is the next token with a different range 
(or
+  // the end of the Tokens).
+  TokRef = TokRef.drop_front(Conflicting.size());
+}
+return NonConflicting;
   }
 
   bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
@@ -236,13 +256,18 @@ private:
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-if (Loc.isMacroID())
-  // FIXME: skip tokens inside macros for now.
-  return;
+if(Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
 
 // Non top level decls that are included from a header are not filtered by
 // topLevelDecls. (example: method declarations being included from another
 // file for a class from another file)
+// There are also cases with macros where the spelling loc will not be in 
the
+// main file and the highlighting would be incorrect.
 if (!isInsideMainFile(Loc, SM))
   return;
 

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=369275&r1=369274&r2=369275&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon 
Aug 19 09:27:49 2019
@@ -16,10 +16,6 @@
 
 namespace clang {
 namespace clangd {
-  void PrintTo(const HighlightingToken &T, ::std::ostream *OS) {
-  *OS << "(" << T.R.start.line << ", " << T.R.start.character << ") -> (" << 
T.R.end.line << ", " << T.R.end.character << "): " << (int)T.Kind;
-}
-
 namespace {
 
 std::vector
@@ -380,6 +376,56 @@ TEST(SemanticHighlighting, GetsCorrectTo
 $Class[[G]]<$Class[[F

[PATCH] D64418: [Docs][OpenCL] Documentation of C++ for OpenCL mode

2019-08-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 215922.
Anastasia added a comment.

Added small corrections in various parts.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64418/new/

https://reviews.llvm.org/D64418

Files:
  docs/LanguageExtensions.rst
  docs/UsersManual.rst

Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -2397,7 +2408,8 @@
 This will produce a generic test.bc file that can be used in vendor toolchains
 to perform machine code generation.
 
-Clang currently supports OpenCL C language standards up to v2.0.
+Clang currently supports OpenCL C language standards up to v2.0. Starting from
+clang 9 a C++ mode is available for OpenCL (see :ref:`C++ for OpenCL `).
 
 OpenCL Specific Options
 ---
@@ -2756,6 +2768,46 @@
   enqueue query functions from `section 6.13.17.5
   `_.
 
+.. _opencl_cpp:
+
+C++ for OpenCL
+--
+
+Starting from clang 9 kernel code can contain C++17 features: classes, templates,
+function overloading, type deduction, etc. Please note that this is not an
+implementation of `OpenCL C++
+`_ and
+there is no plan to support it in clang in any new releases in the near future.
+
+For detailed information about restrictions to allowed C++ features please
+refer to :doc:`LanguageExtensions`.
+
+Since C++ features are to be used on top of OpenCL C functionality, all existing
+restrictions from OpenCL C v2.0 will inherently apply. All OpenCL C builtin types
+and function libraries are supported and can be used in this mode.
+
+To enable the C++ for OpenCL mode, pass one of following command line options when
+compiling ``.cl`` file ``-cl-std=clc++``, ``-cl-std=CLC++``, ``-std=clc++`` or
+``-std=CLC++``.
+
+   .. code-block:: c++
+
+ template T add( T x, T y )
+ {
+   return x + y;
+ }
+
+ __kernel void test( __global float* a, __global float* b)
+ {
+   auto index = get_global_id(0);
+   a[index] = add(b[index], b[index+1]);
+ }
+
+
+   .. code-block:: console
+
+ clang -cl-std=clc++ test.cl
+
 .. _target_features:
 
 Target-Specific Features and Limitations
Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1516,6 +1633,285 @@
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
+
+OpenCL Features
+===
+
+C++ for OpenCL
+--
+
+This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
+regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
+is inherited. This section describes minor differences to OpenCL C and any
+limitations related to C++ support as well as interactions between OpenCL and
+C++ features that are not documented elsewhere.
+
+Restrictions to C++17
+^
+
+The following features are not supported:
+
+- Virtual functions
+- Exceptions
+- ``dynamic_cast`` operator
+- Non-placement ``new``/``delete`` operators
+- Standard C++ libraries. Currently there is no solution for alternative C++
+  libraries provided. Future release will feature library support.
+
+
+Interplay of OpenCL and C++ features
+
+
+Address space behavior
+""
+
+Address spaces are part of the type qualifiers; many rules are just inherited
+from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
+extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
+behavior in C++ is not documented formally, Clang extends the existing concept
+from C and OpenCL. For example conversion rules are extended from qualification
+conversion but the compatibility is determined using notation of sets and
+overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
+s3.1.3). For OpenCL it means that implicit conversions are allowed from
+a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
+v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
+address space does not overlap with any other and therefore no valid conversion
+between ``__constant`` and other address spaces exists. Most of the rules
+follow this logic.
+
+**Casts**
+
+C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
+permit conversion to ``__generic`` implicitly. However converting from
+``__generic`` to named address spaces can only be done using ``addrspace_cast``.
+Note that conversions between ``__constant`` and any other address space
+are disallowed.
+
+.. _opencl_cpp_addrsp_deduction:
+
+**Deduction**
+
+Address spaces are not deduced for:
+
+- non-pointer/non-reference template parameters or any dependent types except

[PATCH] D66294: [Docs][OpenCL] Release 9.0 notes for OpenCL

2019-08-19 Thread Marco Antognini via Phabricator via cfe-commits
mantognini accepted this revision.
mantognini added a comment.
This revision is now accepted and ready to land.

Thanks for addressing my comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66294/new/

https://reviews.llvm.org/D66294



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


[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

2019-08-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM from my side (and a few NIT, but up to you whether to apply them)




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:46
+NonConflicting.reserve(Tokens.size());
+ArrayRef TokRef(Tokens);
+while (!TokRef.empty()) {

NIT: Using a for-loop here could provide a little more structure and limit the 
scope of `TokRef`
```
for (ArrayRef Toks = Tokens; !Toks.empty(); ) {
  // ...
  Toks = Toks.drop_front(Conflicting.size());
}
```



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:60
+  // of the Tokens).
+  TokRef = ArrayRef(Conflicting.end(), TokRef.end());
+}

NIT: the following version seems more readable: `TokRef = 
TokRef.drop_front(Conflicting.size())`;




Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:332
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);

jvikstrom wrote:
> ilya-biryukov wrote:
> > Unrelated to the change: do we plan to highlight macro names?
> > That seems very useful.
> Yes
Awesome, can't wait for it to land!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64741/new/

https://reviews.llvm.org/D64741



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


[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

2019-08-19 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Charusso added a parent revision: D66325: [analyzer] CastValueChecker: Store 
the dynamic types and casts.

-


Repository:
  rC Clang

https://reviews.llvm.org/D66423

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 \
-// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
 // RUN:  -analyzer-output=text -verify %s
 
 namespace llvm {
@@ -18,6 +18,12 @@
 const X *dyn_cast_or_null(Y *Value);
 template 
 const X *dyn_cast_or_null(Y &Value);
+
+template 
+bool isa(Y Value);
+
+template 
+bool isa_and_nonnull(Y Value);
 } // namespace llvm
 
 namespace clang {
@@ -79,10 +85,21 @@
 return;
   }
 
-  (void)(1 / !C);
-  // expected-note@-1 {{'C' is non-null}}
-  // expected-note@-2 {{Division by zero}}
-  // expected-warning@-3 {{Division by zero}}
+  if (isa(C)) {
+// expected-note@-1 {{'C' with type 'Circle' is not the instance of 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' with type 'Circle' is a 'Circle'}}
+// expected-note@-2 {{Taking true branch}}
+
+(void)(1 / !C);
+// expected-note@-1 {{'C' is non-null}}
+// expected-note@-2 {{Division by zero}}
+// expected-warning@-3 {{Division by zero}}
+  }
 }
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
@@ -90,8 +107,14 @@
   // expected-note@-1 {{Checked cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!cast(C)) {
-// expected-note@-1 {{Checked cast from 'Circle' to 'Triangle' succeeds}}
+  if (isa(C)) {
+// expected-note@-1 {{Assuming 'C' with type 'Circle' is not the instance of 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(C)) {
+// expected-note@-1 {{'C' with type 'Circle' is not the instance of 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -10,6 +10,8 @@
 //
 //===--===//
 
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -24,7 +26,7 @@
 
 namespace {
 class CastValueChecker : public Checker {
-  enum class CallKind { Function, Method };
+  enum class CallKind { Function, Method, InstanceOf };
 
   using CastCheck =
   std::functionisFails() : Cast->isSucceeds();
+}
+
 //===--===//
 // Main logic to evaluate a cast.
 //===--===//
@@ -114,13 +135,9 @@
   else
 IsCastSucceeds = IsCheckedCast || IsNonNullReturn || CastFromTy == CastToTy;
 
-  // Check for infeasible casts.
-  if (!IsCheckedCast && Cast) {
-if ((IsCastSucceeds && Cast->isFails()) ||
-(!IsCastSucceeds && Cast->isSucceeds())) {
-  C.generateSink(State, C.getPredecessor());
-  return;
-}
+  if (!IsCheckedCast && isInfeasibleCast(Cast, IsCastSucceeds)) {
+C.generateSink(State, C.getPredecessor());
+return;
   }
 
   // Store the type and the cast information.
@@ -154,6 +171,75 @@
   Tag);
 }
 
+static void addInstanceOfTransition(const CallEvent &Call,
+DefinedOrUnknownSVal DV,
+ProgramStateRef State, CheckerContext &C,
+bool IsInstanceOf) {
+  const auto *CE = cast(Call.getOriginExpr());
+
+  const DeclRefExpr *DRE = nullptr;
+  const Stmt *Body = CE->getCalleeDecl()->getBody();
+  if (Body)
+DRE = dyn_cast(Body);
+
+  if (!Body || !DRE)
+DRE = dyn_cast(CE->getCallee()->IgnoreParenImpCasts());
+
+  QualType CastToTy = DRE->getTemplateArgs()->getArgument().getAsType();
+  QualType CastFromTy = getRecordType(Call.parameters()[0]->getType());
+
+  const DynamicCastInfo *Cast = getDynamicCastInfo(State, CastFromTy, CastToTy);
+
+  bool IsCastSucceeds;
+  if (Cast)
+IsCastSucceeds = IsInstanceOf && Cast->isSucceeds();
+  els

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

2019-08-19 Thread Princeton Ferro via Phabricator via cfe-commits
Prince781 updated this revision to Diff 215912.
Prince781 added a comment.

I think this should order the initializers deterministically according to their 
var declaration order. Let me know if there's something I haven't considered.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66122/new/

https://reviews.llvm.org/D66122

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCXX/cxx11-thread-local.cpp

Index: clang/test/CodeGenCXX/cxx11-thread-local.cpp
===
--- clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -268,6 +268,33 @@
   return this->n;
 }
 
+namespace static_tls_in_lambda {
+  struct X {
+X() {}
+  };
+
+
+  X (*f())() {
+static thread_local X x;
+
+return [] { return x; };
+  }
+
+  auto y = f();
+
+  void g() { y(); }
+
+  void bar(X**, X**, X**);
+  void baz(void());
+  void f2() {
+  thread_local X x;
+  thread_local X* p = &x;
+  thread_local X* q = p;
+  thread_local X* r = q;
+  baz([]{bar(&p, &q, &r);});
+  }
+}
+
 namespace {
 thread_local int anon_i{1};
 }
@@ -303,6 +330,42 @@
 // CHECK: store i64 1, i64* @_ZGVN1XIiE1mE
 // CHECK: br label
 
+// CHECK: define internal void @"_ZZN20static_tls_in_lambda1fEvENK3$_1clEv"
+// CHECK: %[[static_tls_guard_val:.*]] = load i8, i8* @_ZGVZN20static_tls_in_lambda1fEvE1x
+// CHECK: %[[static_tls_guard_init:.*]] = icmp eq i8 %[[static_tls_guard_val]], 0
+// CHECK: br i1 %[[static_tls_guard_init]],
+// CHECK: call void @_ZN20static_tls_in_lambda1XC1Ev(%"struct.static_tls_in_lambda::X"* @_ZZN20static_tls_in_lambda1fEvE1x)
+// CHECK: store i8 1, i8* @_ZGVZN20static_tls_in_lambda1fEvE1x, align 1
+
+// CHECK: define internal void @"_ZZN20static_tls_in_lambda2f2EvENK3$_2clEv"
+// init x
+// CHECK: %[[static_tls_guard_val:.*]] = load i8, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1x
+// CHECK: %[[static_tls_guard_init:.*]] = icmp eq i8 %[[static_tls_guard_val]], 0
+// CHECK: br i1 %[[static_tls_guard_init]],
+// CHECK: call void @_ZN20static_tls_in_lambda1XC1Ev(%"struct.static_tls_in_lambda::X"* @_ZZN20static_tls_in_lambda2f2EvE1x)
+// CHECK: store i8 1, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1x, align 1
+// init p
+// CHECK: %[[static_tls_guard_val:.*]] = load i8, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1p
+// CHECK: %[[static_tls_guard_init:.*]] = icmp eq i8 %[[static_tls_guard_val]], 0
+// CHECK: br i1 %[[static_tls_guard_init]],
+// CHECK: store %"struct.static_tls_in_lambda::X"* @_ZZN20static_tls_in_lambda2f2EvE1x, %"struct.static_tls_in_lambda::X"** @_ZZN20static_tls_in_lambda2f2EvE1p
+// CHECK: store i8 1, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1p, align 1
+// init q
+// CHECK: %[[static_tls_guard_val:.*]] = load i8, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1q
+// CHECK: %[[static_tls_guard_init:.*]] = icmp eq i8 %[[static_tls_guard_val]], 0
+// CHECK: br i1 %[[static_tls_guard_init]],
+// CHECK: %[[static_tls_var_prev:.*]] = load %"struct.static_tls_in_lambda::X"*, %"struct.static_tls_in_lambda::X"** @_ZZN20static_tls_in_lambda2f2EvE1p, align 8
+// CHECK: store %"struct.static_tls_in_lambda::X"* %[[static_tls_var_prev]], %"struct.static_tls_in_lambda::X"** @_ZZN20static_tls_in_lambda2f2EvE1q, align 8
+// CHECK: store i8 1, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1q, align 1
+// init r
+// CHECK: %[[static_tls_guard_val:.*]] = load i8, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1r
+// CHECK: %[[static_tls_guard_init:.*]] = icmp eq i8 %[[static_tls_guard_val]], 0
+// CHECK: br i1 %[[static_tls_guard_init]],
+// CHECK: %[[static_tls_var_prev:.*]] = load %"struct.static_tls_in_lambda::X"*, %"struct.static_tls_in_lambda::X"** @_ZZN20static_tls_in_lambda2f2EvE1q, align 8
+// CHECK: store %"struct.static_tls_in_lambda::X"* %[[static_tls_var_prev]], %"struct.static_tls_in_lambda::X"** @_ZZN20static_tls_in_lambda2f2EvE1r, align 8
+// CHECK: store i8 1, i8* @_ZGVZN20static_tls_in_lambda2f2EvE1r, align 1
+
+
 // CHECK: define {{.*}}@[[GLOBAL_INIT:.*]]()
 // CHECK: call void @[[C_INIT]]()
 // CHECK: call void @[[E_INIT]]()
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -467,6 +467,10 @@
   /// should emit cleanups.
   bool CurFuncIsThunk = false;
 
+  /// static thread-local variables we've referenced that were declared in a
+  /// parent function.
+  llvm::SmallSet ForeignStaticTLSVars;
+
   /// In ARC, whether we should autorelease the return value.
   bool AutoreleaseResult = false;
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -31,6 +31,7 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGe

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

2019-08-19 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:54
+  if (Conflicting.size() > 1) {
+Tokens.erase(Tokens.begin() + I,
+ Tokens.begin() + I + Conflicting.size());

ilya-biryukov wrote:
> This is potentially `O(n^2)`. Could we instead create a new vector and fill 
> it with the new items?
> 
> The memory usage should not matter much - we have the AST stored in the 
> background anyway. I bet we would be looking at it first if we wanted to 
> minimize memory usage.
> 
> If we really wanted to **not** waste any memory, we could do it 
> `std::erase_if`-style, i.e. move the items we want to remove to the end of 
> the vector, call `vector::erase` once at the end.
Completely forgot that erase is linear. Will go for the "allocate another 
vector" approach. 

Rewrote the entire conflicting checking code to hopefully be simpler as well 
(it changed substantially, so you might want to take another look at it).



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:332
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);

ilya-biryukov wrote:
> Unrelated to the change: do we plan to highlight macro names?
> That seems very useful.
Yes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64741/new/

https://reviews.llvm.org/D64741



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


[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

2019-08-19 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 215907.
jvikstrom marked 2 inline comments as done.
jvikstrom added a comment.

Rewrote conflicting token removal code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64741/new/

https://reviews.llvm.org/D64741

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -323,6 +323,57 @@
   $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
   $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
   auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
+)cpp",
+// Tokens that share a source range but have conflicting Kinds are not
+// highlighted.
+R"cpp(
+  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);
+  DEF_CLASS($Class[[A]])
+  #define MACRO_CONCAT(X, V, T) T foo##X = V
+  #define DEF_VAR(X, V) int X = V
+  #define DEF_VAR_T(T, X, V) T X = V
+  #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
+  #define CPY(X) X
+  #define DEF_VAR_TYPE(X, Y) X Y
+  #define SOME_NAME variable
+  #define SOME_NAME_SET variable2 = 123
+  #define INC_VAR(X) X += 2
+  $Primitive[[void]] $Function[[foo]]() {
+DEF_VAR($Variable[[X]],  123);
+DEF_VAR_REV(908, $Variable[[XY]]);
+$Primitive[[int]] CPY( $Variable[[XX]] );
+DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+$Primitive[[double]] SOME_NAME;
+$Primitive[[int]] SOME_NAME_SET;
+$Variable[[variable]] = 20.1;
+MACRO_CONCAT(var, 2, $Primitive[[float]]);
+DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+  CPY($Class[[A]]()));
+INC_VAR($Variable[[variable]]);
+  }
+  $Primitive[[void]] SOME_NAME();
+  DEF_VAR($Variable[[XYZ]], 567);
+  DEF_VAR_REV(756, $Variable[[AB]]);
+
+  #define CALL_FN(F) F();
+  #define DEF_FN(F) void F ()
+  DEF_FN($Function[[g]]) {
+CALL_FN($Function[[foo]]);
+  }
+)cpp",
+R"cpp(
+  #define fail(expr) expr
+  #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  $Primitive[[int]] $Variable[[x]];
+  $Primitive[[int]] $Variable[[y]];
+  $Primitive[[int]] $Function[[f]]();
+  $Primitive[[void]] $Function[[foo]]() {
+assert($Variable[[x]] != $Variable[[y]]);
+assert($Variable[[x]] != $Function[[f]]());
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
@@ -338,6 +389,19 @@
 int someMethod();
 void otherMethod();
   )cpp"}});
+
+  // A separate test for macros in headers.
+  checkHighlightings(R"cpp(
+#include "imp.h"
+DEFINE_Y
+DXYZ_Y(A);
+  )cpp",
+ {{"imp.h", R"cpp(
+#define DXYZ(X) class X {};
+#define DXYZ_Y(Y) DXYZ(x##Y)
+#define DEFINE(X) int X;
+#define DEFINE_Y DEFINE(Y)
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -38,7 +38,28 @@
 llvm::sort(Tokens);
 auto Last = std::unique(Tokens.begin(), Tokens.end());
 Tokens.erase(Last, Tokens.end());
-return Tokens;
+// Macros can give tokens that have the same source range but conflicting
+// kinds. In this case all tokens sharing this source range should be
+// removed.
+std::vector NonConflicting;
+NonConflicting.reserve(Tokens.size());
+ArrayRef TokRef(Tokens);
+while (!TokRef.empty()) {
+  ArrayRef Conflicting =
+  TokRef.take_while([&](const HighlightingToken &T) {
+// TokRef is guaranteed at least one element here because otherwise
+// this predicate would never fire.
+return T.R == TokRef.front().R;
+  });
+  // If there is exactly one token with this range it's non conflicting and
+  // should be in the highlightings.
+  if (Conflicting.size() == 1)
+NonConflicting.push_back(TokRef.front());
+  // Conflicting.end() is the next token with a different range (or the end
+  // of the Tokens).
+  TokRef = ArrayRef(Conflicting.end(), TokRef.end());
+}
+return NonConflicting;
   }
 
   bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
@@ -227,13 +248,18 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-if (Loc.isMacroID())
-  // FIXME: skip tokens inside

[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

2019-08-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:389
 llvm::sort(References, [](const Reference &L, const Reference &R) {
-  return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
- std::tie(R.Loc, R.CanonicalTarget, R.Role);
+  return L.Loc < R.Loc;
 });

hokein wrote:
> ilya-biryukov wrote:
> > hokein wrote:
> > > ilya-biryukov wrote:
> > > > What are the elements `References` for the problematic case?
> > > > 
> > > > If we have duplicate elements, then `sort()` would now give us one of 
> > > > the items. Which exact `Decl` we're gonna end up seeing is not 
> > > > well-defined, i.e. it's non-deterministic.
> > > > What are the elements References for the problematic case?
> > > 
> > > The testcase is using declarations (see the existing test) -- we will get 
> > > 3 refs on `using ::fo^o`, each ref has a different decl.  
> > > 
> > > ```
> > > void [[foo]](int);
> > > void [[foo]](double);
> > > 
> > > namespace ns {
> > > using ::[[fo^o]];
> > > }
> > > ```
> > > 
> > > > If we have duplicate elements, then sort() would now give us one of the 
> > > > items. Which exact Decl we're gonna end up seeing is not well-defined, 
> > > > i.e. it's non-deterministic.
> > > 
> > > I think we could make it deterministic, but only return one refs, WDYT?
> > > 
> > > 
> > To make sure I understand the problem: we used to get 4 references in total:
> > - 2 references to the functions (`foo(int)` and `foo(double)`)
> > - 2 references pointing to the using declaration, e.g. the `using ::[foo]]`
> > 
> > After this patch we would remove the duplicate `using ::[[foo]]` from the 
> > results and get only 3 references as a result.
> > 
> > Is that correct?
> Yes, that is correct.
Interestingly enough, we always ignore the `CanonicalTarget` in the returned 
`Reference`.

Maybe we could remove the `CanonicalTarget` from the `Reference` struct instead?
That keeps the interface more consistent: clients will always get deterministic 
results (as there is no `CanonicalDecl`, which is different now)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66349/new/

https://reviews.llvm.org/D66349



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


[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

2019-08-19 Thread Romain Geissler via Phabricator via cfe-commits
Romain-Geissler-1A added a comment.

Hi,

The only group I know which uses -fsemantic-interposition is the company I work 
for: Amadeus. Since for now the flag doesn't exist on clang, no we don't know 
that the future clang implementation doesn't match the gcc one ;) But then we 
have to wonder if clang wants to reuse the same name if that doesn't do the 
same thing if really the differences are big. What are the differences between 
the gcc and the LLVM implementation (when -fsemantic-interposition will be 
wired up in clang) ?

For now we are not using clang/llvm tools for anything else than static 
analysis and the useful tools from clang-extra-tools. In other words, we don't 
care about code gen of LLVM, yet (that may come eventually). My immediate 
concern right now is that when you use clang-tidy and friends with a 
compilation database generated for gcc, then you we end-up with warnings for 
each of the non-usual flags we use in Amadeus, like for example 
-fsemantic-interposition, that "pollutes" the output of these tools for each 
translation unit and thus it brings a user unfriendly experience.

I am fine with wiring up a new flag -fsemantic-interposition in clang, however 
I have no idea what should be done for that, I have absolutely 0 knowledge in 
the LLVM source code, yet.

Cheers,
Romain


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65616/new/

https://reviews.llvm.org/D65616



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


[PATCH] D66365: [clang-tidy] [readability-convert-member-functions-to-static] ignore functions that hide base class methods

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I'd like some help in understanding the motivation for this change. The bug 
report says `But the intention behind this is clearly that there is a member 
function A::foo which derived classes overwrite.` and I'm not certain that's so 
clear. It's been my experience that hidden member function names are unusual 
and it's hard to ascribe intent to them.

FWIW, I would find this new behavior to be confusing. My mental model for this 
check is "if it doesn't need to be a member function, make it a static function 
instead", and this behavior doesn't fit that model.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66365/new/

https://reviews.llvm.org/D66365



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


[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

2019-08-19 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D65616#1634966 , @serge-sans-paille 
wrote:

> Adding @hfinkel to the thread to confirm it's okay to add this behavior, as 
> he authored an RFC on the subject (see 
> https://lists.llvm.org/pipermail/llvm-dev/2016-November/107625.html)


-fsemantic-interposition was added in GCC 5.1. I don't think lots of projects 
in the wild are using it. The important D20217 
 has been merged, I think we should actually 
implement this option instead of ignoring it (I'd be very happy to help this).

> that may be used by some gcc users

Can you just name the user groups? :) When they write 
`-fsemantic-interposition`, do they know that current clang doesn't match the 
GCC behavior?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65616/new/

https://reviews.llvm.org/D65616



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


[PATCH] D66361: Improve behavior in the case of stack exhaustion.

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:14
 let Component = "Sema" in {
-let CategoryName = "Semantic Issue" in {
+def warn_stack_exhausted : Warning<
+  "stack nearly exhausted; compilation time may suffer, and "

Should this be a Sema warning as opposed to a Basic warning? It seems to me 
that we may want to guard against similar stack exhaustion from the parser as 
well, wouldn't we?



Comment at: include/clang/Basic/Stack.h:43-47
+if (LLVM_UNLIKELY(isStackNearlyExhausted())) {
+  runWithSufficientStackSpaceSlow(Diag, Fn);
+} else {
+  Fn();
+}

Elide braces.



Comment at: lib/Sema/SemaExpr.cpp:15070-15079
+  // Trivial default constructors and destructors are never actually used.
+  // FIXME: What about other special members?
+  if (Func->isTrivial() && !Func->hasAttr() &&
+  OdrUse == OdrUseContext::Used) {
+if (auto *Constructor = dyn_cast(Func))
+  if (Constructor->isDefaultConstructor())
+OdrUse = OdrUseContext::FormallyOdrUsed;

This seems unrelated to the patch?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66361/new/

https://reviews.llvm.org/D66361



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


[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

2019-08-19 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 215893.
Charusso retitled this revision from "[analyzer] CastValueChecker: Store the 
dynamic types in DynamicTypeMap" to "[analyzer] CastValueChecker: Store the 
dynamic types and casts".
Charusso added a comment.

This patch introduces `DynamicCastInfo` similar to `DynamicTypeInfo` which
is stored in `DynamicCastSet` similar to `DynamicTypeMap`. It could be used
to store and check the casts and prevent infeasible paths.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66325/new/

https://reviews.llvm.org/D66325

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicTypeMap.cpp
  clang/test/Analysis/cast-value-logic.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cast-value.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,6 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
+// CHECK-NEXT:   "dynamic_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/dump_egraph.cpp
===
--- clang/test/Analysis/dump_egraph.cpp
+++ clang/test/Analysis/dump_egraph.cpp
@@ -24,4 +24,5 @@
 
 // CHECK: \"cluster\": \"t\", \"pointer\": \"{{0x[0-9a-f]+}}\", \"items\": [\l\{ \"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$2\{int, LC5, no stmt, #1\}\"
 
-// CHECK: \"dynamic_types\": [\l\{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false\}\l
+// CHECK: \"dynamic_types\": [\l  \{ \"region\": \"HeapSymRegion\{conj_$1\{struct S *, LC1, S{{[0-9]+}}, #1\}\}\", \"dyn_type\": \"struct S\", \"sub_classable\": false \}\l
+
Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// RUN:  -analyzer-output=text -verify %s 2>&1 | FileCheck %s
+
+void clang_analyzer_printState();
+
+namespace llvm {
+template 
+const X *dyn_cast_or_null(Y *Value);
+template 
+const X *dyn_cast_or_null(Y &Value);
+} // namespace llvm
+
+namespace clang {
+struct Shape {};
+class Triangle : public Shape {};
+class Circle : public Shape {};
+class Square : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
+
+void evalNonNullParamNonNullReturnReference(const Shape &S) {
+  const auto *C = dyn_cast_or_null(S);
+  // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Dynamic cast from 'Circle' to 'Triangle' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(C)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Circle' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (dyn_cast_or_null(S)) {
+// expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Square' fails}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  clang_analyzer_printState();
+
+  // CHECK:  "dynamic_types": [
+  // CHECK-NEXT:   { "region": "SymRegion{reg_$0}", "dyn_type": "const class clang::Circle", "sub_classable": true }
+  // CHECK-NEXT: ],
+  // CHECK-NEXT: "dynamic_casts": [
+  // CHECK-NEXT:   { "from": "struct clang::Shape", "to": "class clang::Circle", "kind": "success" },
+  // CHECK-NEXT:   { "from": "class clang::Circle", "to": "class clang::Triangle", "kind": "fail" },
+  // CHECK-NEXT:   { "from": "class clang::Circle", "to": "class clang::Square", "kind": "fail" },
+  // CHECK-NEXT:   { "from": "struct clang::Shape", "to": "class clang::Square", "kind": "fail" }
+  // CHECK-NEXT: ],
+
+  (void)(1 / !C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+}
Index: clang/tes

[PATCH] D66294: [Docs][OpenCL] Release 9.0 notes for OpenCL

2019-08-19 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 215894.
Anastasia added a comment.

- Added empty line and ; as requested on the review.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66294/new/

https://reviews.llvm.org/D66294

Files:
  docs/ReleaseNotes.rst

Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -46,6 +46,8 @@
 Major New Features
 --
 
+- Experimental support for :ref:`C++ for OpenCL ` has been
+  added.
 - ...
 
 Improvements to Clang's diagnostics
@@ -133,6 +135,14 @@
 C++ Language Changes in Clang
 -
 
+- Support for the address space attribute in various C++ features was improved,
+  refer to :ref:`C++ for OpenCL ` for more details. The following
+  features deviated from OpenCL:
+
+  (1) Address spaces as method qualifiers are not accepted yet;
+
+  (2) There is no address space deduction.
+
 - ...
 
 C++1z Feature Support
@@ -152,10 +162,87 @@
   // clang used to encode this as "^{NSArray=#}" instead of "@".
   const char *s0 = @encode(MyArray *);
 
-OpenCL C Language Changes in Clang
---
+OpenCL Kernel Language Changes in Clang
+---
 
-...
+OpenCL C
+
+
+- Enabled use of variadic macro as a Clang extension.
+
+- Added initial support for implicitly including OpenCL builtin
+  fuctions using efficient trie lookup generated by TableGen.
+  A corresponding frontend-only flag ``-fdeclare-opencl-builtins``
+  has been added to enable trie during parsing.
+
+- Refactored header file to be used for common parts between
+  regular header added using ``-finclude-default-header`` and trie
+  based declarations added using ``-fdeclare-opencl-builtins``.
+
+- Improved string formatting diagnostics in printf for vector types.
+
+- Simplified the internal representation of blocks, including their
+  generation in IR. Furthermore, indirect calls to block function
+  has been changed to direct function calls.
+
+- Added diagnostics for conversions of nested pointers with
+  different address spaces.
+
+- Added ``cl_arm_integer_dot_product`` extension.
+
+- Fixed global samplers in OpenCL v2.0.
+
+- Improved math builtin functions with parameters of type ``long
+  long`` for x86.
+
+.. _openclcpp:
+
+C++ for OpenCL
+^^
+
+Experimental support for C++17 features in OpenCL has been added
+and backwards compatibility with OpenCL C v2.0 was enabled.
+The documentation has been added for supported language features
+into :doc:`LanguageExtensions` and :doc:`UsersManual`.
+
+Implemented features are:
+
+- Address space behavior is improved in majority of C++ features:
+
+  - Templates parameters and arguments;
+
+  - Reference types;
+
+  - Type deduction;
+
+  - Objects and member functions, including special member
+functions;
+
+  - Builtin operators;
+
+  - Method qualifiers now include address space;
+
+  - Address space deduction has been extended for C++ use cases;
+
+  - Improved overload ranking rules.
+
+  - All standard cast operators now prevent converting address
+spaces (except for conversions allowed implicitly). They
+can still be cast using C-style cast.
+
+- Vector types as in OpenCL C, including compound vector
+  initialization.
+
+- OpenCL-specific types: images, samplers, events, pipes, are
+  accepted. Note that blocks are not supported yet.
+
+- OpenCL standard header in Clang can be compiled in C++ mode.
+
+- Global constructors can be invoked from the host side using
+  a specific, compiler-generated kernel.
+
+- Overloads with generic address space are added to all atomic
+  builtin functions, including the ones prior to OpenCL v2.0.
 
 ABI Changes in Clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66219: [clangd] Added a colorizer to the vscode extension.

2019-08-19 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 215886.
jvikstrom marked 4 inline comments as done.
jvikstrom added a comment.

Renamed colorizer to highlighter and added FIXME about highlightings below eof.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66219/new/

https://reviews.llvm.org/D66219

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
  
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

Index: clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
@@ -1,5 +1,6 @@
 import * as assert from 'assert';
 import * as path from 'path';
+import * as vscode from 'vscode';
 
 import * as SM from '../src/semantic-highlighting';
 
@@ -57,4 +58,72 @@
 assert.deepEqual(tm.getBestThemeRule('variable.other.parameter.cpp').scope,
  'variable.other.parameter');
   });
+  test('Colorizer groups decorations correctly', () => {
+const colorizations: {uri: string, decorations: any[]}[] = [];
+// Mock of a colorizer that saves the parameters in the colorizations array.
+class MockFileColorizer extends SM.FileHighlighter {
+  public highlight(uri: string, decorationRangePairs: any[]) {
+colorizations.push({uri : uri, decorations : decorationRangePairs});
+  }
+  public dispose() {}
+}
+// Helper for creating a vscode Range.
+const createRange = (line: number, startCharacter: number,
+ length: number) =>
+new vscode.Range(new vscode.Position(line, startCharacter),
+ new vscode.Position(line, startCharacter + length));
+const scopeTable = [
+  [ 'variable' ], [ 'entity.type.function' ],
+  [ 'entity.type.function.method' ]
+];
+const rules = [
+  {scope : 'variable', foreground : '1'},
+  {scope : 'entity.type', foreground : '2'},
+];
+const tm = new SM.ThemeRuleMatcher(rules);
+const colorizer = new SM.Highlighter(MockFileColorizer, scopeTable);
+// No colorization if themeRuleMatcher has not been set.
+colorizer.setFileLines('a', []);
+assert.deepEqual(colorizations, []);
+colorizer.updateThemeRuleMatcher(tm);
+assert.deepEqual(colorizations, [ {decorations : [], uri : 'a'} ]);
+// Groups decorations into the scopes used.
+let line = [
+  {character : 1, length : 2, scopeIndex : 1},
+  {character : 5, length : 2, scopeIndex : 1},
+  {character : 10, length : 2, scopeIndex : 2}
+];
+colorizer.setFileLines(
+'a', [ {line : 1, tokens : line}, {line : 2, tokens : line} ]);
+assert.equal(colorizations[1].uri, 'a');
+assert.equal(colorizations[1].decorations.length, 2);
+// Can't test the actual decorations as vscode does not seem to have an api
+// for getting the actual decoration objects.
+assert.deepEqual(colorizations[1].decorations[0].ranges, [
+  createRange(1, 1, 2), createRange(1, 5, 2), createRange(2, 1, 2),
+  createRange(2, 5, 2)
+]);
+assert.deepEqual(colorizations[1].decorations[1].ranges,
+ [ createRange(1, 10, 2), createRange(2, 10, 2) ]);
+// Keeps state separate between files.
+colorizer.setFileLines('b', [
+  {line : 1, tokens : [ {character : 1, length : 1, scopeIndex : 0} ]}
+]);
+assert.equal(colorizations[2].uri, 'b');
+assert.equal(colorizations[2].decorations.length, 1);
+assert.deepEqual(colorizations[2].decorations[0].ranges,
+ [ createRange(1, 1, 1) ]);
+// Does full colorizations.
+colorizer.setFileLines('a', [
+  {line : 1, tokens : [ {character : 2, length : 1, scopeIndex : 0} ]}
+]);
+assert.equal(colorizations[3].uri, 'a');
+assert.equal(colorizations[3].decorations.length, 3);
+assert.deepEqual(colorizations[3].decorations[0].ranges,
+ [ createRange(1, 2, 1) ]);
+assert.deepEqual(colorizations[3].decorations[1].ranges,
+ [ createRange(2, 1, 2), createRange(2, 5, 2) ]);
+assert.deepEqual(colorizations[3].decorations[2].ranges,
+ [ createRange(2, 10, 2) ]);
+  });
 });
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -34,6 +34,13 @@
   // The TextMate scope index to the clangd scope lookup table.
   scopeIndex: number;
 }
+// A line of decoded highlightings from the data clangd sent.
+int

[PATCH] D66206: [CodeGen] Don't keep stale pointers to LoopInfos

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit for you in r369259, thank you for the patch!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66206/new/

https://reviews.llvm.org/D66206



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


r369259 - Don't keep stale pointers to LoopInfos.

2019-08-19 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Mon Aug 19 06:37:41 2019
New Revision: 369259

URL: http://llvm.org/viewvc/llvm-project?rev=369259&view=rev
Log:
Don't keep stale pointers to LoopInfos.

CGLoopInfo was keeping pointers to parent loop LoopInfos, but when the loop 
info vector grew, it reallocated the storage and invalidated all of the parent 
pointers, causing use-after-free. Manage the lifetimes of the LoopInfos 
separately so that the pointers aren't stale.

Patch by Bevin Hansson.

Added:
cfe/trunk/test/CodeGen/loop-info-asan.c
Modified:
cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
cfe/trunk/lib/CodeGen/CGLoopInfo.h

Modified: cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.cpp?rev=369259&r1=369258&r2=369259&view=diff
==
--- cfe/trunk/lib/CodeGen/CGLoopInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGLoopInfo.cpp Mon Aug 19 06:37:41 2019
@@ -563,8 +563,9 @@ void LoopInfo::finish() {
 
 void LoopInfoStack::push(BasicBlock *Header, const llvm::DebugLoc &StartLoc,
  const llvm::DebugLoc &EndLoc) {
-  Active.push_back(LoopInfo(Header, StagedAttrs, StartLoc, EndLoc,
-Active.empty() ? nullptr : &Active.back()));
+  Active.emplace_back(
+  new LoopInfo(Header, StagedAttrs, StartLoc, EndLoc,
+   Active.empty() ? nullptr : Active.back().get()));
   // Clear the attributes so nested loops do not inherit them.
   StagedAttrs.clear();
 }
@@ -756,16 +757,16 @@ void LoopInfoStack::push(BasicBlock *Hea
 
 void LoopInfoStack::pop() {
   assert(!Active.empty() && "No active loops to pop");
-  Active.back().finish();
+  Active.back()->finish();
   Active.pop_back();
 }
 
 void LoopInfoStack::InsertHelper(Instruction *I) const {
   if (I->mayReadOrWriteMemory()) {
 SmallVector AccessGroups;
-for (const LoopInfo &AL : Active) {
+for (const auto &AL : Active) {
   // Here we assume that every loop that has an access group is parallel.
-  if (MDNode *Group = AL.getAccessGroup())
+  if (MDNode *Group = AL->getAccessGroup())
 AccessGroups.push_back(Group);
 }
 MDNode *UnionMD = nullptr;

Modified: cfe/trunk/lib/CodeGen/CGLoopInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.h?rev=369259&r1=369258&r2=369259&view=diff
==
--- cfe/trunk/lib/CodeGen/CGLoopInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGLoopInfo.h Mon Aug 19 06:37:41 2019
@@ -275,11 +275,11 @@ private:
   bool hasInfo() const { return !Active.empty(); }
   /// Return the LoopInfo for the current loop. HasInfo should be called
   /// first to ensure LoopInfo is present.
-  const LoopInfo &getInfo() const { return Active.back(); }
+  const LoopInfo &getInfo() const { return *Active.back(); }
   /// The set of attributes that will be applied to the next pushed loop.
   LoopAttributes StagedAttrs;
   /// Stack of active loops.
-  llvm::SmallVector Active;
+  llvm::SmallVector, 4> Active;
 };
 
 } // end namespace CodeGen

Added: cfe/trunk/test/CodeGen/loop-info-asan.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/loop-info-asan.c?rev=369259&view=auto
==
--- cfe/trunk/test/CodeGen/loop-info-asan.c (added)
+++ cfe/trunk/test/CodeGen/loop-info-asan.c Mon Aug 19 06:37:41 2019
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -o /dev/null
+
+// This test should not exhibit use-after-free in LoopInfo.
+
+int a() {
+  for (;;)
+for (;;)
+  for (;;)
+for (;;)
+  for (;;)
+for (;;)
+  for (;;)
+for (;;)
+  for (;;)
+;
+}


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


[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

2019-08-19 Thread serge via Phabricator via cfe-commits
serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

Sorry for the long delay, this LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56343/new/

https://reviews.llvm.org/D56343



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


[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

2019-08-19 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop accepted this revision.
russell.gallop added a comment.
This revision is now accepted and ready to land.

I don't know a lot about the structure of clang but it LGTM from the point of 
view of the code and traces coming out.

I'm not very keen on having two "Frontend" sections, but I think it works okay. 
It moves Clang CodeGen under "Frontend". "Total Frontend" and "Total Backend" 
between them now cover almost all of the execution time (so is more accurate 
than without this change).

I think there's always going to be a trade off with time-trace between 
presenting something simple that makes sense to an end user and revealing how 
LLVM/Clang is actually structured inside, which is much more complicated (and 
doesn't nicely fit into scopes).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



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


[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

2019-08-19 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added a comment.

I don't believe I have any further comments. What do the front-end guys say?




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:126
+  case LangOptions::FPM_Precise:
+  case LangOptions::FPM_Fast:
+break;

andrew.w.kaylor wrote:
> mibintc wrote:
> > mibintc wrote:
> > > kpn wrote:
> > > > Wait, so "fast" and "precise" are the same thing? That doesn't sound 
> > > > like where the documentation you put in the ticket says "the compiler 
> > > > preserves the source expression ordering and rounding properties of 
> > > > floating-point".
> > > > 
> > > > (Yes, I saw below where "fast" turns on the fast math flags but 
> > > > "precise" doesn't. That doesn't affect my point here.)
> > > "precise" doesn't necessitate the use of Constrained Intrinsics, And 
> > > likewise for "fast".   The words "compiler preserves the source 
> > > expression ordering" were copied from the msdn documentation for 
> > > /fp:precise as you explained it would be useful to have the msdn 
> > > documentation for the option in case it goes offline in, say, 30 years.  
> > > The ICL Intel compiler also provides equivalent floating point options. 
> > > The Intel documentation for precise is phrased differently "Disables 
> > > optimizations that are not value-safe on floating-point data."  
> > > 
> > > fp-model=precise should enable contractions, if that's not true at 
> > > default (I mean, clang -c) then this patch is missing that.
> > > 
> > > fp-model=fast is the same as requesting ffast-math 
> > Well, we haven't heard from Andy yet, but he told me some time ago that 
> > /fp:precise corresponds more or less (there was wiggle room) to clang's 
> > default behavior.  It sounds like you think the description in the msdn of 
> > /fp:precise isn't describing clang's default behavior, @kpn can you say 
> > more about that, and do you think that ConstrainedIntrinsics should be 
> > created to provide the semantics of /fp:precise? 
> "Precise" means that no value unsafe optimizations will be performed. That's 
> what LLVM does by default. As long as no fast math flags are set, we will not 
> perform optimizations that are not value safe.
OK, I stand corrected.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62731/new/

https://reviews.llvm.org/D62731



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


[PATCH] D65456: [OpenCL] Add generic type handling for builtin functions

2019-08-19 Thread Sven van Haastregt via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL369253: [OpenCL] Add generic type handling for builtin 
functions (authored by svenvh, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65456?vs=215086&id=215859#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65456/new/

https://reviews.llvm.org/D65456

Files:
  cfe/trunk/lib/Sema/OpenCLBuiltins.td
  cfe/trunk/lib/Sema/SemaLookup.cpp
  cfe/trunk/test/SemaOpenCL/fdeclare-opencl-builtins.cl
  cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Index: cfe/trunk/lib/Sema/SemaLookup.cpp
===
--- cfe/trunk/lib/Sema/SemaLookup.cpp
+++ cfe/trunk/lib/Sema/SemaLookup.cpp
@@ -673,76 +673,148 @@
 D->dump();
 }
 
-/// When trying to resolve a function name, if the isOpenCLBuiltin function
-/// defined in "OpenCLBuiltins.inc" returns a non-null , then the
-/// identifier is referencing an OpenCL builtin function. Thus, all its
-/// prototypes are added to the LookUpResult.
-///
-/// \param S The Sema instance
-/// \param LR  The LookupResult instance
-/// \param II  The identifier being resolved
-/// \param Index  The list of prototypes starts at Index in OpenCLBuiltins[]
-/// \param Len  The list of prototypes has Len elements
-static void InsertOCLBuiltinDeclarations(Sema &S, LookupResult &LR,
- IdentifierInfo *II, unsigned Index,
- unsigned Len) {
-
-  for (unsigned i = 0; i < Len; ++i) {
-const OpenCLBuiltinDecl &Decl = OpenCLBuiltins[Index - 1 + i];
-ASTContext &Context = S.Context;
+/// Get the QualType instances of the return type and arguments for an OpenCL
+/// builtin function signature.
+/// \param Context (in) The Context instance.
+/// \param OpenCLBuiltin (in) The signature currently handled.
+/// \param GenTypeMaxCnt (out) Maximum number of types contained in a generic
+///type used as return type or as argument.
+///Only meaningful for generic types, otherwise equals 1.
+/// \param RetTypes (out) List of the possible return types.
+/// \param ArgTypes (out) List of the possible argument types.  For each
+///argument, ArgTypes contains QualTypes for the Cartesian product
+///of (vector sizes) x (types) .
+static void GetQualTypesForOpenCLBuiltin(
+ASTContext &Context, const OpenCLBuiltinStruct &OpenCLBuiltin,
+unsigned &GenTypeMaxCnt, std::vector &RetTypes,
+SmallVector, 5> &ArgTypes) {
+  // Get the QualType instances of the return types.
+  unsigned Sig = SignatureTable[OpenCLBuiltin.SigTableIndex];
+  OCL2Qual(Context, TypeTable[Sig], RetTypes);
+  GenTypeMaxCnt = RetTypes.size();
+
+  // Get the QualType instances of the arguments.
+  // First type is the return type, skip it.
+  for (unsigned Index = 1; Index < OpenCLBuiltin.NumTypes; Index++) {
+std::vector Ty;
+OCL2Qual(Context,
+TypeTable[SignatureTable[OpenCLBuiltin.SigTableIndex + Index]], Ty);
+ArgTypes.push_back(Ty);
+GenTypeMaxCnt = (Ty.size() > GenTypeMaxCnt) ? Ty.size() : GenTypeMaxCnt;
+  }
+}
 
-// Ignore this BIF if the version is incorrect.
-if (Context.getLangOpts().OpenCLVersion < Decl.Version)
-  continue;
+/// Create a list of the candidate function overloads for an OpenCL builtin
+/// function.
+/// \param Context (in) The ASTContext instance.
+/// \param GenTypeMaxCnt (in) Maximum number of types contained in a generic
+///type used as return type or as argument.
+///Only meaningful for generic types, otherwise equals 1.
+/// \param FunctionList (out) List of FunctionTypes.
+/// \param RetTypes (in) List of the possible return types.
+/// \param ArgTypes (in) List of the possible types for the arguments.
+static void
+GetOpenCLBuiltinFctOverloads(ASTContext &Context, unsigned GenTypeMaxCnt,
+ std::vector &FunctionList,
+ std::vector &RetTypes,
+ SmallVector, 5> &ArgTypes) {
+  FunctionProtoType::ExtProtoInfo PI;
+  PI.Variadic = false;
+
+  // Create FunctionTypes for each (gen)type.
+  for (unsigned IGenType = 0; IGenType < GenTypeMaxCnt; IGenType++) {
+SmallVector ArgList;
+
+for (unsigned A = 0; A < ArgTypes.size(); A++) {
+  // Builtins such as "max" have an "sgentype" argument that represents
+  // the corresponding scalar type of a gentype.  The number of gentypes
+  // must be a multiple of the number of sgentypes.
+  assert(GenTypeMaxCnt % ArgTypes[A].size() == 0 &&
+ "argument type count not compatible with gentype type count");
+  unsigned Idx = IGenType % ArgTypes[A].size();
+  ArgList.push_back(ArgTypes[A][Idx]);
+}
+
+FunctionList.push_back(Context.getFunctionType(
+RetTypes[(RetTypes.size() != 1) ? IGenType : 

r369253 - [OpenCL] Add generic type handling for builtin functions

2019-08-19 Thread Sven van Haastregt via cfe-commits
Author: svenvh
Date: Mon Aug 19 04:56:03 2019
New Revision: 369253

URL: http://llvm.org/viewvc/llvm-project?rev=369253&view=rev
Log:
[OpenCL] Add generic type handling for builtin functions

Generic types are an abstraction of type sets.  It mimics the way
functions are defined in the OpenCL specification.  For example,
floatN can abstract all the vector sizes of the float type.

This allows to
 * stick more closely to the specification, which uses generic types;
 * factorize definitions of functions with numerous prototypes in the
   tablegen file; and
 * reduce the memory impact of functions with many overloads.

Patch by Pierre Gondois and Sven van Haastregt.

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

Modified:
cfe/trunk/lib/Sema/OpenCLBuiltins.td
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/test/SemaOpenCL/fdeclare-opencl-builtins.cl
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Modified: cfe/trunk/lib/Sema/OpenCLBuiltins.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/OpenCLBuiltins.td?rev=369253&r1=369252&r2=369253&view=diff
==
--- cfe/trunk/lib/Sema/OpenCLBuiltins.td (original)
+++ cfe/trunk/lib/Sema/OpenCLBuiltins.td Mon Aug 19 04:56:03 2019
@@ -40,11 +40,15 @@ def LocalAS  : AddressSpace<"clang::
 def GenericAS: AddressSpace<"clang::LangAS::opencl_generic">;
 
 
-// Qualified Type. Allow to retrieve one ASTContext QualType.
-class QualType {
+// Qualified Type.  These map to ASTContext::QualType.
+class QualType {
   // Name of the field or function in a clang::ASTContext
   // E.g. Name="IntTy" for the int type, and "getIntPtrType()" for an intptr_t
   string Name = _Name;
+  // Some QualTypes in this file represent an abstract type for which there is
+  // no corresponding AST QualType, e.g. a GenType or an `image2d_t` type
+  // without access qualifiers.
+  bit IsAbstract = _IsAbstract;
 }
 
 // Helper class to store type access qualifiers (volatile, const, ...).
@@ -52,25 +56,35 @@ class Qualifier {
   string QualName = _QualName;
 }
 
+// List of integers.
+class IntList _List> {
+  string Name = _Name;
+  list List = _List;
+}
+
 
//===--===//
 //  OpenCL C classes for types
 
//===--===//
-// OpenCL types (int, float, ...)
+// OpenCL C basic data types (int, float, image2d_t, ...).
+// Its Child classes can represent concrete types (e.g.: VectorType) or
+// custom types (e.g.: GenType).
+// Instances of these child classes should be used in Builtin function
+// arguments.  See the definition of the "read_imagef" function as example.
 class Type {
-  // Name of the Type
+  // Name of the Type.
   string Name = _Name;
-  // QualType associated with this type
+  // QualType associated with this type.
   QualType QTName = _QTName;
-  // Size of the vector (if applicable)
-  int VecWidth = 0;
-  // Is pointer
+  // Size of the vector (if applicable).
+  int VecWidth = 1;
+  // Is a pointer.
   bit IsPointer = 0;
   // List of qualifiers associated with the type (volatile, ...)
   list QualList = [];
-  // Address space
-  string AddrSpace = "clang::LangAS::Default";
   // Access qualifier. Must be one of ("RO", "WO", "RW").
   string AccessQualifier = "";
+  // Address space.
+  string AddrSpace = "clang::LangAS::Default";
 }
 
 // OpenCL vector types (e.g. int2, int3, int16, float8, ...)
@@ -78,7 +92,7 @@ class VectorType :
   Type<_Ty.Name, _Ty.QTName> {
   bit IsPointer = 1;
@@ -91,6 +105,50 @@ class ImageType _Type> {
+  string Name = _Name;
+  list List = _Type;
+}
+
+// A GenericType is an abstract type that defines a set of types as a
+// combination of Types and vector sizes.
+//
+// E.g.: If TypeList =  and VectorList = <1, 2, 4>, then it
+//   represents .
+// _Ty  : Name of the GenType.
+// _TypeList: List of basic data Types.
+// _VectorList  : Sizes of the vector for each type of the _TypeList, 1 being a
+//scalar.
+//
+// Some rules apply when using multiple GenericType arguments in a declaration:
+//   1. The number of vector sizes must be equal or 1 for all gentypes in a
+//  declaration.
+//   2. The number of Types must be equal or 1 for all gentypes in a
+//  declaration.
+//   3. Generic types are combined by iterating over all generic types at once.
+//  For example, for the following GenericTypes
+//GenT1 = GenericType and
+//GenT2 = GenericType
+//  A declaration f(GenT1, GenT2) results in the combinations
+//f(half, float), f(half2, float2), f(half, int), f(half2, int2) .
+//   4. "sgentype" from the OpenCL specification is supported by specifying
+//  a single vector size.
+//  For example, for the following GenericTypes
+//GenT = GenericType and
+//

[PATCH] D66137: [OpenCL][PR42385] Improve addr space deduction for pointers/references to arrays

2019-08-19 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL369251: [OpenCL] Fix addr space deduction for 
pointers/references to arrays. (authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66137?vs=214806&id=215858#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66137/new/

https://reviews.llvm.org/D66137

Files:
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl


Index: cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
===
--- cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
+++ cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
@@ -78,3 +78,25 @@
   int foo[10];
   xxx(&foo[0]);
 }
+
+// Addr space for pointer/reference to an array
+//CHECK: FunctionDecl {{.*}} t1 'void (const __generic float (&)[2])'
+void t1(const float (&fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t2 'void (const __generic float (*)[2])'
+void t2(const float (*fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t3 'void (__generic float (((*)))[2])'
+void t3(float(((*fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t4 'void (__generic float (((*__generic *)))[2])'
+void t4(float(((**fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t5 'void (__generic float (*__generic (*))[2])'
+void t5(float (*(*fYZ))[2]);
+
+__kernel void k() {
+  __local float x[2];
+  __local float(*p)[2];
+  t1(x);
+  t2(&x);
+  t3(&x);
+  t4(&p);
+  t5(&p);
+}
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -7385,8 +7385,22 @@
   bool IsPointee =
   ChunkIndex > 0 &&
   (D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Pointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference);
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference ||
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer);
+  // For pointers/references to arrays the next chunk is always an array
+  // followed by any number of parentheses.
+  if (!IsPointee && ChunkIndex > 1) {
+auto AdjustedCI = ChunkIndex - 1;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Array)
+  AdjustedCI--;
+// Skip over all parentheses.
+while (AdjustedCI > 0 &&
+   D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Paren)
+  AdjustedCI--;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Pointer ||
+D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Reference)
+  IsPointee = true;
+  }
   bool IsFuncReturnType =
   ChunkIndex > 0 &&
   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Function;


Index: cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
===
--- cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
+++ cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
@@ -78,3 +78,25 @@
   int foo[10];
   xxx(&foo[0]);
 }
+
+// Addr space for pointer/reference to an array
+//CHECK: FunctionDecl {{.*}} t1 'void (const __generic float (&)[2])'
+void t1(const float (&fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t2 'void (const __generic float (*)[2])'
+void t2(const float (*fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t3 'void (__generic float (((*)))[2])'
+void t3(float(((*fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t4 'void (__generic float (((*__generic *)))[2])'
+void t4(float(((**fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t5 'void (__generic float (*__generic (*))[2])'
+void t5(float (*(*fYZ))[2]);
+
+__kernel void k() {
+  __local float x[2];
+  __local float(*p)[2];
+  t1(x);
+  t2(&x);
+  t3(&x);
+  t4(&p);
+  t5(&p);
+}
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -7385,8 +7385,22 @@
   bool IsPointee =
   ChunkIndex > 0 &&
   (D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Pointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference);
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference ||
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer);
+  // For pointers/references to arrays the next chunk is always an array
+  // followed by any number of parentheses.
+  if (!IsPointee && ChunkIndex > 1) {
+auto AdjustedCI = ChunkIndex - 1;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Array)
+  AdjustedCI--;
+// Skip over all parentheses.
+while (AdjustedCI > 0 &&
+ 

r369251 - [OpenCL] Fix addr space deduction for pointers/references to arrays.

2019-08-19 Thread Anastasia Stulova via cfe-commits
Author: stulova
Date: Mon Aug 19 04:43:16 2019
New Revision: 369251

URL: http://llvm.org/viewvc/llvm-project?rev=369251&view=rev
Log:
[OpenCL] Fix addr space deduction for pointers/references to arrays.

Rewrite the logic for detecting if we are deducing addr space of
a pointee type to take into account special logic for arrays. For
pointers/references to arrays we can have any number of parentheses
expressions as well as nested pointers.

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


Modified:
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=369251&r1=369250&r2=369251&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon Aug 19 04:43:16 2019
@@ -7385,8 +7385,22 @@ static void deduceOpenCLImplicitAddrSpac
   bool IsPointee =
   ChunkIndex > 0 &&
   (D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Pointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference);
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference ||
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer);
+  // For pointers/references to arrays the next chunk is always an array
+  // followed by any number of parentheses.
+  if (!IsPointee && ChunkIndex > 1) {
+auto AdjustedCI = ChunkIndex - 1;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Array)
+  AdjustedCI--;
+// Skip over all parentheses.
+while (AdjustedCI > 0 &&
+   D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Paren)
+  AdjustedCI--;
+if (D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Pointer ||
+D.getTypeObject(AdjustedCI).Kind == DeclaratorChunk::Reference)
+  IsPointee = true;
+  }
   bool IsFuncReturnType =
   ChunkIndex > 0 &&
   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Function;

Modified: cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl?rev=369251&r1=369250&r2=369251&view=diff
==
--- cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl (original)
+++ cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl Mon Aug 19 04:43:16 
2019
@@ -78,3 +78,25 @@ __kernel void test() {
   int foo[10];
   xxx(&foo[0]);
 }
+
+// Addr space for pointer/reference to an array
+//CHECK: FunctionDecl {{.*}} t1 'void (const __generic float (&)[2])'
+void t1(const float (&fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t2 'void (const __generic float (*)[2])'
+void t2(const float (*fYZ)[2]);
+//CHECK: FunctionDecl {{.*}} t3 'void (__generic float (((*)))[2])'
+void t3(float(((*fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t4 'void (__generic float (((*__generic *)))[2])'
+void t4(float(((**fYZ)))[2]);
+//CHECK: FunctionDecl {{.*}} t5 'void (__generic float (*__generic (*))[2])'
+void t5(float (*(*fYZ))[2]);
+
+__kernel void k() {
+  __local float x[2];
+  __local float(*p)[2];
+  t1(x);
+  t2(&x);
+  t3(&x);
+  t4(&p);
+  t5(&p);
+}


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


[PATCH] D65456: [OpenCL] Add generic type handling for builtin functions

2019-08-19 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! Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65456/new/

https://reviews.llvm.org/D65456



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


[PATCH] D66294: [Docs][OpenCL] Release 9.0 notes for OpenCL

2019-08-19 Thread Marco Antognini via Phabricator via cfe-commits
mantognini added inline comments.



Comment at: docs/ReleaseNotes.rst:209
+Implemented features are:
+- Address space behavior is improved in majority of C++ features:
+

I think Sphinx/RST wants an empty line here.

Nitpicking for consistency, could you have a `;` at then end of each bullet 
point (except the last one, obviously)? Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66294/new/

https://reviews.llvm.org/D66294



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


  1   2   >