[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

2019-07-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332
   S->isSemanticForm() ? S->getSyntacticForm() : S, Queue));
   TRY_TO(TraverseSynOrSemInitListExpr(
   S->isSemanticForm() ? S : S->getSemanticForm(), Queue));

gribozavr wrote:
> Instead of adding a whole new if statement, could you wrap the second 
> existing TRY_TO in `if(shouldVisitImplicitCode())` ?
Despite looking very similar, that would **not** be equivalent to the current 
version.

For most init lists (that do not have alternative "form"), the following 
invariants hold:
```
InitList* E = ...;
assert(E->isSemanticForm());
assert(E->isSyntacticForm()); 
assert(E->getSynacticForm() == nullptr);
```

This subtle fact means the current code does not traversed the list twice if 
they do not have an alternative form (either semantic or syntactic).

Now, if we only run the first statement, we will call 
`TraverseSynOrSemInitListExpr(S->getSyntacticForm())` and 
`S->getSyntacticForm()` returns `null`. So we don't traverse anything.

I tried various ways to keep both calls, but they all ended up being too 
complicated, hence the final version. Let me know if you see a better way to 
address this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64762



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


[PATCH] D64487: [clang, test] Fix Clang :: Headers/max_align.c on 64-bit SPARC

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
ro added a reviewer: jyknight.
ro added a comment.

Ping?  It's been a week.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64487



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


[PATCH] D64488: [Driver] Support -fsanitize=function on Solaris/x86

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
ro added a reviewer: krytarowski.
ro added a comment.

Ping?  It's been a week.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64488



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


[PATCH] D64491: [Driver] Enable __cxa_atexit on Solaris

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
ro added reviewers: mehdi_amini, rnk.
ro added a comment.

Ping?  It's been a week and the patch seems completely in line with other ELF 
targets.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64491



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


[PATCH] D64493: [Driver] Don't pass --dynamic-linker to ld on Solaris

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
ro added reviewers: mehdi_amini, rnk.
ro added a comment.

Ping? It's been a ween and unless someone comes up with a good explanation why 
the default interpreter must be specified instead
of letting ld figure it out, this seems pretty obvious.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64493



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


[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

2019-07-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I think this looks reasonable to me, though I am still not certain if the 
relative path in the python script will work with both the svn in-tree 
directory layout as well as the git monorepo layout (which I'm far less 
familiar with).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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


[PATCH] D64482: [Driver] Define _FILE_OFFSET_BITS=64 on Solaris

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
ro added reviewers: mehdi_amini, rnk.
ro added a comment.

Ping?  It's been a week and it would be good to get this patch and its 
companion into LLVM 9: it unbreaks `make check-all` with gcc 9
and restores g++ compatibility.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64482



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


[PATCH] D64415: Consistent types and naming for AArch64 target features (NFC)

2019-07-16 Thread Momchil Velikov via Phabricator via cfe-commits
chill accepted this revision.
chill added a comment.
This revision is now accepted and ready to land.

Going to commit as obvious.


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

https://reviews.llvm.org/D64415



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


[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

2019-07-16 Thread Shaurya Gupta via Phabricator via cfe-commits
SureYeaah updated this revision to Diff 210045.
SureYeaah added a comment.

Fixed wrong function name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64717

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -291,6 +291,7 @@
   const char *Input = "struct ^X { int x; int y; }";
   EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 |   int x"));
 }
+
 TEST(TweakTest, ExtractVariable) {
   llvm::StringLiteral ID = "ExtractVariable";
   checkAvailable(ID, R"cpp(
@@ -302,7 +303,7 @@
   int a = 5 + [[4 ^* ^xyz^()]];
   // multivariable initialization
   if(1)
-int x = ^1, y = ^a + 1, a = ^1, z = a + 1;
+int x = ^1, y = [[a + 1]], a = ^1, z = a + 1;
   // if without else
   if(^1) {}
   // if with else
@@ -315,13 +316,13 @@
 a = ^4;
   else
 a = ^5;
-  // for loop 
+  // for loop
   for(a = ^1; a > ^3^+^4; a++)
 a = ^2;
-  // while 
+  // while
   while(a < ^1)
-^a++;
-  // do while 
+[[a++]];
+  // do while
   do
 a = ^1;
   while(a < ^3);
@@ -343,22 +344,28 @@
 int xyz = ^1;
   };
 }
+void v() { return; }
 // function default argument
 void f(int b = ^1) {
   // void expressions
   auto i = new int, j = new int;
   de^lete i^, del^ete j;
+  [[v]]();
   // if
   if(1)
 int x = 1, y = a + 1, a = 1, z = ^a + 1;
   if(int a = 1)
 if(^a == 4)
   a = ^a ^+ 1;
-  // for loop 
+  // for loop
   for(int a = 1, b = 2, c = 3; ^a > ^b ^+ ^c; ^a++)
 a = ^a ^+ 1;
-  // lambda 
+  // lambda
   auto lamb = [&^a, &^b](int r = ^1) {return 1;}
+  // assigment
+  [[a ^= 5]];
+  // Variable DeclRefExpr
+  a = [[b]];
 }
   )cpp");
   // vector of pairs of input and output strings
@@ -412,6 +419,24 @@
R"cpp(void f(int a) {
 auto dummy = 1; label: [ [gsl::suppress("type")] ] for (;;) a = dummy;
  })cpp"},
+  // MemberExpr
+  {R"cpp(class T {
+   T f() {
+ return [[T().f().f]]();
+   }
+ };)cpp",
+   R"cpp(class T {
+   T f() {
+ auto dummy = T().f().f(); return dummy;
+   }
+ };)cpp"},
+  // Function DeclRefExpr
+  {R"cpp(int f() {
+   return [[f]]();
+ })cpp",
+   R"cpp(int f() {
+   auto dummy = f(); return dummy;
+ })cpp"},
   // FIXME: Doesn't work because bug in selection tree
   /*{R"cpp(#define PLUS(x) x++
  void f(int a) {
@@ -421,8 +446,8 @@
  void f(int a) {
auto dummy = a; PLUS(dummy);
  })cpp"},*/
-  // FIXME: Doesn't work correctly for \[\[clang::uninitialized\]\] int b
-  // = 1; since the attr is inside the DeclStmt and the bounds of
+  // FIXME: Wrong result for \[\[clang::uninitialized\]\] int b = 1;
+  // since the attr is inside the DeclStmt and the bounds of
   // DeclStmt don't cover the attribute
   };
   for (const auto &IO : InputOutputs) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -13,6 +13,7 @@
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
@@ -37,18 +38,16 @@
 const ASTContext &Ctx);
   const clang::Expr *getExpr() const { return Expr; }
   const SelectionTree::Node *getExprNode() const { return ExprNode; }
-  bool isExtractable() const { return Extractable; }
   // Generate Replacement for replacing selected expression with given VarName
   tooling::Replacement replaceWithVar(llvm::StringRef VarName) const;
   // Generate Replacement for declaring the selected Expr as a new variable
   tooling::Replacement insertDeclaration(llvm::StringRef VarName) const;
+  const clang::Stmt *InsertionPoint = nullptr;
 
 private:
-  bool Extractable = false;
   const clang::Expr *Expr;
   const SelectionTree::Node *ExprNode;
   // Stmt before which we will extract
-  const clang::Stmt *InsertionPoint = nullptr;
   const Sourc

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

2019-07-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

based on the offline discussion, now I understand the patch better (thanks).

some comments on the interface.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:60
+  PathRef File,
+  std::vector>> 
Highlightings)
+  override;

how about creating a new structure `LineHighlightingTokens` to represent the 
line-based tokens?



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:82
   bool SemanticHighlighting;
+  HighlightingDiffer Differ;
 };

If we make it as a pointer, the `bool SemanticHighlighting` is not needed (just 
follow the what the other field `FIndex` does).



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:255
+  const FileEntry *CurrentFileEntry = SM.getFileEntryForID(SM.getMainFileID());
+  std::string CurrentPath = CurrentFileEntry->tryGetRealPathName().str() +
+CurrentFileEntry->getName().str();

we could get the canonical path from `onMainAST` callback (the first parameter 
`PathRef Path`).



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:60
+class HighlightingDiffer {
+  std::map> PrevHighlightingsMap;
+  std::mutex PrevMutex;

nit: llvm::StringMap.

I think this map is storing all file highlightings, maybe call it 
`FileToHighlightings`?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:74
+  std::vector>>
+  diffHighlightings(const ParsedAST &AST,
+std::vector Highlightings);

this method does two things, 
1) calculate the diff between old and new
2) replace the old highlighting with new highlightings

but the name just reflects 1).

I think we just narrow it to 2) only:

```
// calculate the new highlightings, and return the old one.
std::vector updateFile(PathRef Path, 
std::vector NewHighlightings);

// to get diff, in `onMainAST`
diffHighlightings(differ.updateFile(Path, NewHighlightings), NewHighlightings);
```




Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:82
+  std::vector>>
+  diffHighlightings(ArrayRef Highlightings,
+ArrayRef PrevHighlightings);

this utility method doesn't use any internal member of the class, we could pull 
it out or make it static.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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


r366194 - [clang-scan-view] Force utf-8 when handling report (python2 only)

2019-07-16 Thread Serge Guelton via cfe-commits
Author: serge_sans_paille
Date: Tue Jul 16 01:56:47 2019
New Revision: 366194

URL: http://llvm.org/viewvc/llvm-project?rev=366194&view=rev
Log:
[clang-scan-view] Force utf-8 when handling report (python2 only)

Original patch by random human 

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

Modified:
cfe/trunk/tools/scan-view/share/ScanView.py

Modified: cfe/trunk/tools/scan-view/share/ScanView.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-view/share/ScanView.py?rev=366194&r1=366193&r2=366194&view=diff
==
--- cfe/trunk/tools/scan-view/share/ScanView.py (original)
+++ cfe/trunk/tools/scan-view/share/ScanView.py Tue Jul 16 01:56:47 2019
@@ -764,11 +764,11 @@ File Bug
 variables['report'] = m.group(2)
 
 try:
-f = open(path,'r')
+f = open(path,'rb')
 except IOError:
 return self.send_404()
 fs = os.fstat(f.fileno())
-data = f.read()
+data = f.read().decode('utf-8')
 for a,b in kReportReplacements:
 data = a.sub(b % variables, data)
 return self.send_string(data, ctype, mtime=fs.st_mtime)


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


[PATCH] D64129: [clang-scan-view] Force utf-8 when handling report (python2 only)

2019-07-16 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366194: [clang-scan-view] Force utf-8 when handling report 
(python2 only) (authored by serge_sans_paille, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64129?vs=207752&id=210046#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64129

Files:
  cfe/trunk/tools/scan-view/share/ScanView.py


Index: cfe/trunk/tools/scan-view/share/ScanView.py
===
--- cfe/trunk/tools/scan-view/share/ScanView.py
+++ cfe/trunk/tools/scan-view/share/ScanView.py
@@ -764,11 +764,11 @@
 variables['report'] = m.group(2)
 
 try:
-f = open(path,'r')
+f = open(path,'rb')
 except IOError:
 return self.send_404()
 fs = os.fstat(f.fileno())
-data = f.read()
+data = f.read().decode('utf-8')
 for a,b in kReportReplacements:
 data = a.sub(b % variables, data)
 return self.send_string(data, ctype, mtime=fs.st_mtime)


Index: cfe/trunk/tools/scan-view/share/ScanView.py
===
--- cfe/trunk/tools/scan-view/share/ScanView.py
+++ cfe/trunk/tools/scan-view/share/ScanView.py
@@ -764,11 +764,11 @@
 variables['report'] = m.group(2)
 
 try:
-f = open(path,'r')
+f = open(path,'rb')
 except IOError:
 return self.send_404()
 fs = os.fstat(f.fileno())
-data = f.read()
+data = f.read().decode('utf-8')
 for a,b in kReportReplacements:
 data = a.sub(b % variables, data)
 return self.send_string(data, ctype, mtime=fs.st_mtime)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

2019-07-16 Thread Shaurya Gupta via Phabricator via cfe-commits
SureYeaah updated this revision to Diff 210047.
SureYeaah added a comment.

Fixed crash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64717

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -291,6 +291,7 @@
   const char *Input = "struct ^X { int x; int y; }";
   EXPECT_THAT(getMessage(ID, Input), ::testing::HasSubstr("0 |   int x"));
 }
+
 TEST(TweakTest, ExtractVariable) {
   llvm::StringLiteral ID = "ExtractVariable";
   checkAvailable(ID, R"cpp(
@@ -302,7 +303,7 @@
   int a = 5 + [[4 ^* ^xyz^()]];
   // multivariable initialization
   if(1)
-int x = ^1, y = ^a + 1, a = ^1, z = a + 1;
+int x = ^1, y = [[a + 1]], a = ^1, z = a + 1;
   // if without else
   if(^1) {}
   // if with else
@@ -315,13 +316,13 @@
 a = ^4;
   else
 a = ^5;
-  // for loop 
+  // for loop
   for(a = ^1; a > ^3^+^4; a++)
 a = ^2;
-  // while 
+  // while
   while(a < ^1)
-^a++;
-  // do while 
+[[a++]];
+  // do while
   do
 a = ^1;
   while(a < ^3);
@@ -343,22 +344,28 @@
 int xyz = ^1;
   };
 }
+void v() { return; }
 // function default argument
 void f(int b = ^1) {
   // void expressions
   auto i = new int, j = new int;
   de^lete i^, del^ete j;
+  [[v]]();
   // if
   if(1)
 int x = 1, y = a + 1, a = 1, z = ^a + 1;
   if(int a = 1)
 if(^a == 4)
   a = ^a ^+ 1;
-  // for loop 
+  // for loop
   for(int a = 1, b = 2, c = 3; ^a > ^b ^+ ^c; ^a++)
 a = ^a ^+ 1;
-  // lambda 
+  // lambda
   auto lamb = [&^a, &^b](int r = ^1) {return 1;}
+  // assigment
+  [[a ^= 5]];
+  // Variable DeclRefExpr
+  a = [[b]];
 }
   )cpp");
   // vector of pairs of input and output strings
@@ -412,6 +419,24 @@
R"cpp(void f(int a) {
 auto dummy = 1; label: [ [gsl::suppress("type")] ] for (;;) a = dummy;
  })cpp"},
+  // MemberExpr
+  {R"cpp(class T {
+   T f() {
+ return [[T().f().f]]();
+   }
+ };)cpp",
+   R"cpp(class T {
+   T f() {
+ auto dummy = T().f().f(); return dummy;
+   }
+ };)cpp"},
+  // Function DeclRefExpr
+  {R"cpp(int f() {
+   return [[f]]();
+ })cpp",
+   R"cpp(int f() {
+   auto dummy = f(); return dummy;
+ })cpp"},
   // FIXME: Doesn't work because bug in selection tree
   /*{R"cpp(#define PLUS(x) x++
  void f(int a) {
@@ -421,8 +446,8 @@
  void f(int a) {
auto dummy = a; PLUS(dummy);
  })cpp"},*/
-  // FIXME: Doesn't work correctly for \[\[clang::uninitialized\]\] int b
-  // = 1; since the attr is inside the DeclStmt and the bounds of
+  // FIXME: Wrong result for \[\[clang::uninitialized\]\] int b = 1;
+  // since the attr is inside the DeclStmt and the bounds of
   // DeclStmt don't cover the attribute
   };
   for (const auto &IO : InputOutputs) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -13,6 +13,7 @@
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
@@ -37,18 +38,16 @@
 const ASTContext &Ctx);
   const clang::Expr *getExpr() const { return Expr; }
   const SelectionTree::Node *getExprNode() const { return ExprNode; }
-  bool isExtractable() const { return Extractable; }
   // Generate Replacement for replacing selected expression with given VarName
   tooling::Replacement replaceWithVar(llvm::StringRef VarName) const;
   // Generate Replacement for declaring the selected Expr as a new variable
   tooling::Replacement insertDeclaration(llvm::StringRef VarName) const;
+  const clang::Stmt *InsertionPoint = nullptr;
 
 private:
-  bool Extractable = false;
   const clang::Expr *Expr;
   const SelectionTree::Node *ExprNode;
   // Stmt before which we will extract
-  const clang::Stmt *InsertionPoint = nullptr;
   const SourceManager &SM;

[PATCH] D64754: [clangd] Added highlighting for the targets in typedefs.

2019-07-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:88
 
+  bool VisitTypedefDecl(TypedefDecl *TD) {
+if(const auto *TSI = TD->getTypeSourceInfo())

Do we plan to support the type alias `using Y = X;` as well? `TypedefNameDecl` 
should cover both cases.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64754



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


[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

2019-07-16 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

On my system clang-format has some complaints, so I think you need to rerun 
clang-format. Probably caused by the rebasing.

I have some minor comments about the TestAST.py (see the inline comments), but 
otherwise this patch LGTM.




Comment at: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py:1
+"""Test that importing modules in C works as expected."""
+

This comment (and the name of the test class) are copy-pasted from TestCModules 
and should be updated. If we can merge the two test cases (see the inline 
comment below), then this would be fixed as a side-effect.



Comment at: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py:57
+# This expr command imports __sFILE with definition
+# (FILE is a typedef to __sFILE.)
+self.expect(

I think this test has a good chance to fail once someone 
renamed/removes/changes this internal struct (especially if it's currently  
abstracted with a typedef). Would it be possible to just make a minimal module 
with open and FILE/__sFILE instead? If it's too much trouble, then I'm also 
fine with merging this as-is (as this regression is easy to fix).



Comment at: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py:67
+# Check that the AST log contains exactly one definition of __sFILE.
+f = open(log_file)
+log_lines = f.readlines()

It seems that this is the only different in the test compared to 
TestCModules.py. Would it be possible to just add this logging/checking to 
TestCModules.py as it's anyway testing something very similar?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333



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


[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

2019-07-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

As discussed offline, it's a little scary that the highlight state for diffing 
lives in a separate map from TUScheduler's workers, but it's hard to fix 
without introducing lots of abstractions or layering violations.

It does seem like a good idea to move the diffing up to ClangdLSPServer, as 
it's fairly bound to protocol details. It needs to be reset in didClose (and 
maybe didOpen too, for safety - I think there's a race when a document is 
closed, then tokens are delivered...).

I'd tend to put the map/mutex directly in ClangdLSPServer rather than 
encapsulating it in the Differ object (again, because it's a protocol/lifetime 
detail rather than diffing logic itself) but up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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


[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-16 Thread Kyrill Tkachov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366197: [AArch64] Implement __jcvt intrinsic from Armv8.3-A 
(authored by ktkachov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64495?vs=209149&id=210051#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64495

Files:
  cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
  cfe/trunk/lib/Basic/Targets/AArch64.cpp
  cfe/trunk/lib/Basic/Targets/AArch64.h
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/arm_acle.h
  cfe/trunk/test/CodeGen/arm_acle.c
  cfe/trunk/test/CodeGen/builtins-arm64.c
  llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
  llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/trunk/test/CodeGen/AArch64/fjcvtzs.ll
  llvm/trunk/utils/git-svn/git-llvm

Index: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td
@@ -717,7 +717,9 @@
 // v8.3a floating point conversion for javascript
 let Predicates = [HasJS, HasFPARMv8] in
 def FJCVTZS  : BaseFPToIntegerUnscaled<0b01, 0b11, 0b110, FPR64, GPR32,
-  "fjcvtzs", []> {
+  "fjcvtzs",
+  [(set GPR32:$Rd,
+ (int_aarch64_fjcvtzs FPR64:$Rn))]> {
   let Inst{31} = 0;
 } // HasJS, HasFPARMv8
 
Index: llvm/trunk/utils/git-svn/git-llvm
===
--- llvm/trunk/utils/git-svn/git-llvm
+++ llvm/trunk/utils/git-svn/git-llvm
@@ -372,7 +372,7 @@
 # Now we're ready to commit.
 commit_msg = git('show', '--pretty=%B', '--quiet', rev)
 if not dry_run:
-commit_args = ['commit', '-m', commit_msg]
+commit_args = ['commit', '-m', commit_msg, '--username', 'ktkachov']
 if '--force-interactive' in svn(svn_repo, 'commit', '--help'):
 commit_args.append('--force-interactive')
 log(svn(svn_repo, *commit_args))
Index: llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
===
--- llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
+++ llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
@@ -31,6 +31,8 @@
 def int_aarch64_udiv : Intrinsic<[llvm_anyint_ty], [LLVMMatchType<0>,
 LLVMMatchType<0>], [IntrNoMem]>;
 
+def int_aarch64_fjcvtzs : Intrinsic<[llvm_i32_ty], [llvm_double_ty], [IntrNoMem]>;
+
 //===--===//
 // HINT
 
Index: llvm/trunk/test/CodeGen/AArch64/fjcvtzs.ll
===
--- llvm/trunk/test/CodeGen/AArch64/fjcvtzs.ll
+++ llvm/trunk/test/CodeGen/AArch64/fjcvtzs.ll
@@ -0,0 +1,10 @@
+; RUN: llc -mtriple=arm64-eabi -mattr=+jsconv -o - %s | FileCheck %s
+
+define i32 @test_jcvt(double %v) {
+; CHECK-LABEL: test_jcvt:
+; CHECK: fjcvtzs w0, d0
+  %val = call i32 @llvm.aarch64.fjcvtzs(double %v)
+  ret i32 %val
+}
+
+declare i32 @llvm.aarch64.fjcvtzs(double)
Index: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
@@ -65,6 +65,8 @@
 BUILTIN(__builtin_arm_dsb, "vUi", "nc")
 BUILTIN(__builtin_arm_isb, "vUi", "nc")
 
+BUILTIN(__builtin_arm_jcvt, "Zid", "nc")
+
 // Prefetch
 BUILTIN(__builtin_arm_prefetch, "vvC*UiUiUiUi", "nc")
 
Index: cfe/trunk/test/CodeGen/arm_acle.c
===
--- cfe/trunk/test/CodeGen/arm_acle.c
+++ cfe/trunk/test/CodeGen/arm_acle.c
@@ -2,6 +2,9 @@
 // RUN: %clang_cc1 -ffreestanding -triple armv8-eabi -target-cpu cortex-a57 -O2  -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch32 -check-prefix=ARM-NEWPM -check-prefix=AArch32-NEWPM
 // RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +neon -target-feature +crc -target-feature +crypto -O2 -fno-experimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch64 -check-prefix=ARM-LEGACY -check-prefix=AArch64-LEGACY
 // RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +neon -target-feature +crc -target-feature +crypto -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=ARM -check-prefix=AArch64 -check-prefix=ARM-NEWPM -check-prefix=AArch64-NEWPM
+// RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 -target-feature +v8.3a -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s -check-prefix=AArch64-v8_3
+// RUN

r366197 - [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-16 Thread Kyrylo Tkachov via cfe-commits
Author: ktkachov
Date: Tue Jul 16 02:27:39 2019
New Revision: 366197

URL: http://llvm.org/viewvc/llvm-project?rev=366197&view=rev
Log:
[AArch64] Implement __jcvt intrinsic from Armv8.3-A

The jcvt intrinsic defined in ACLE [1] is available when ARM_FEATURE_JCVT is 
defined.

This change introduces the AArch64 intrinsic, wires it up to the instruction 
and a new clang builtin function.
The __ARM_FEATURE_JCVT macro is now defined when an Armv8.3-A or higher target 
is used.
I've implemented the target detection logic in Clang so that this feature is 
enabled for architectures from armv8.3-a onwards (so -march=armv8.4-a also 
enables this, for example).

make check-all didn't show any new failures.

[1] https://developer.arm.com/docs/101028/latest/data-processing-intrinsics

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/lib/Basic/Targets/AArch64.cpp
cfe/trunk/lib/Basic/Targets/AArch64.h
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/arm_acle.h
cfe/trunk/test/CodeGen/arm_acle.c
cfe/trunk/test/CodeGen/builtins-arm64.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=366197&r1=366196&r2=366197&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Tue Jul 16 02:27:39 2019
@@ -65,6 +65,8 @@ BUILTIN(__builtin_arm_dmb, "vUi", "nc")
 BUILTIN(__builtin_arm_dsb, "vUi", "nc")
 BUILTIN(__builtin_arm_isb, "vUi", "nc")
 
+BUILTIN(__builtin_arm_jcvt, "Zid", "nc")
+
 // Prefetch
 BUILTIN(__builtin_arm_prefetch, "vvC*UiUiUiUi", "nc")
 

Modified: cfe/trunk/lib/Basic/Targets/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AArch64.cpp?rev=366197&r1=366196&r2=366197&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp Tue Jul 16 02:27:39 2019
@@ -118,6 +118,28 @@ void AArch64TargetInfo::getTargetDefines
   getTargetDefinesARMV81A(Opts, Builder);
 }
 
+void AArch64TargetInfo::getTargetDefinesARMV83A(const LangOptions &Opts,
+MacroBuilder &Builder) const {
+  Builder.defineMacro("__ARM_FEATURE_JCVT", "1");
+  // Also include the Armv8.2 defines
+  getTargetDefinesARMV82A(Opts, Builder);
+}
+
+void AArch64TargetInfo::getTargetDefinesARMV84A(const LangOptions &Opts,
+MacroBuilder &Builder) const {
+  // Also include the Armv8.3 defines
+  // FIXME: Armv8.4 makes some extensions mandatory. Handle them here.
+  getTargetDefinesARMV83A(Opts, Builder);
+}
+
+void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions &Opts,
+MacroBuilder &Builder) const {
+  // Also include the Armv8.4 defines
+  // FIXME: Armv8.5 makes some extensions mandatory. Handle them here.
+  getTargetDefinesARMV84A(Opts, Builder);
+}
+
+
 void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
  MacroBuilder &Builder) const {
   // Target identification.
@@ -209,6 +231,15 @@ void AArch64TargetInfo::getTargetDefines
   case llvm::AArch64::ArchKind::ARMV8_2A:
 getTargetDefinesARMV82A(Opts, Builder);
 break;
+  case llvm::AArch64::ArchKind::ARMV8_3A:
+getTargetDefinesARMV83A(Opts, Builder);
+break;
+  case llvm::AArch64::ArchKind::ARMV8_4A:
+getTargetDefinesARMV84A(Opts, Builder);
+break;
+  case llvm::AArch64::ArchKind::ARMV8_5A:
+getTargetDefinesARMV85A(Opts, Builder);
+break;
   }
 
   // All of the __sync_(bool|val)_compare_and_swap_(1|2|4|8) builtins work.
@@ -256,6 +287,12 @@ bool AArch64TargetInfo::handleTargetFeat
   ArchKind = llvm::AArch64::ArchKind::ARMV8_1A;
 if (Feature == "+v8.2a")
   ArchKind = llvm::AArch64::ArchKind::ARMV8_2A;
+if (Feature == "+v8.3a")
+  ArchKind = llvm::AArch64::ArchKind::ARMV8_3A;
+if (Feature == "+v8.4a")
+  ArchKind = llvm::AArch64::ArchKind::ARMV8_4A;
+if (Feature == "+v8.5a")
+  ArchKind = llvm::AArch64::ArchKind::ARMV8_5A;
 if (Feature == "+fullfp16")
   HasFullFP16 = 1;
 if (Feature == "+dotprod")

Modified: cfe/trunk/lib/Basic/Targets/AArch64.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AArch64.h?rev=366197&r1=366196&r2=366197&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AArch64.h (original)
+++ cfe/trunk/lib/Basic/Targets/AArch64.h Tue Jul 16 02:27:39 2019
@@ -59,6 +59,12 @@ public:
MacroBuilder &Builder) const;
   void getTargetDefinesARMV82A(const LangOptions &Opts,

[PATCH] D61749: [clang-tidy] initial version of readability-convert-member-functions-to-static

2019-07-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a tiny nit.




Comment at: 
clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp:110
+  assert(TSI);
+  auto FTL = TSI->getTypeLoc().IgnoreParens().getAs();
+  assert(FTL);

`const auto *`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61749



___
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-07-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:164
+if(Loc.isMacroID()) {
+  // If the location is not an argument it might be from a macro of the 
form
+  // "#define VAR var". In that case this would highlight "var" in the 
macro

It'd be clearer for the comment to describe which cases we are going to 
highlight (rather than the cases we don't).



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:170
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}

The Loc here maybe not in the main file, considering the case

```
// in .h
#define DEFINE(X) in X;
#define DEFINE_Y DEFINE(Y)

// in .cc

DEFINE_Y
```


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] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

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

In D64454#1587102 , @aaron.ballman 
wrote:

> I think this looks reasonable to me, though I am still not certain if the 
> relative path in the python script will work with both the svn in-tree 
> directory layout as well as the git monorepo layout (which I'm far less 
> familiar with).


I'm fairly certain it wouldn't. If I don't forget it, I'll take a look when I 
get home ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64454



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


[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

2019-07-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a subscriber: klimek.
sammccall added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2427-2428
 return 50;
 
+  if (Left.is(TT_UnaryOperator) && Right.is(TT_ObjCMethodExpr))
+return 60;

This looks a little suspicious only because it's so specific.

It seems like you'd *always* want a harsh penalty for breaking between a unary 
operator and its operand. @klimek does this look right, should we drop the 
`ObcJMethodExpr` requirement or is this case handled elsewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775



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


[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

2019-07-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 210052.
ilya-biryukov added a comment.

- Add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64762

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp


Index: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
@@ -17,14 +17,22 @@
 class InitListExprPreOrderVisitor
 : public ExpectedLocationVisitor {
 public:
+  InitListExprPreOrderVisitor(bool VisitImplicitCode)
+  : VisitImplicitCode(VisitImplicitCode) {}
+
+  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
+
   bool VisitInitListExpr(InitListExpr *ILE) {
 Match(ILE->isSemanticForm() ? "semantic" : "syntactic", 
ILE->getBeginLoc());
 return true;
   }
+
+private:
+  bool VisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, InitListExprIsPreOrderVisitedTwice) {
-  InitListExprPreOrderVisitor Visitor;
+  InitListExprPreOrderVisitor Visitor(/*VisitImplicitCode=*/true);
   Visitor.ExpectMatch("syntactic", 2, 21);
   Visitor.ExpectMatch("semantic", 2, 21);
   EXPECT_TRUE(Visitor.runOver("struct S { int x; };\n"
@@ -32,4 +40,13 @@
   InitListExprPreOrderVisitor::Lang_C));
 }
 
+TEST(RecursiveASTVisitor, InitListExprVisitedOnceWhenNoImplicit) {
+  InitListExprPreOrderVisitor Visitor(/*VisitImplicitCode=*/false);
+  Visitor.ExpectMatch("syntactic", 2, 21);
+  Visitor.DisallowMatch("semantic", 2, 21);
+  EXPECT_TRUE(Visitor.runOver("struct S { int x; };\n"
+  "static struct S s = {.x = 0};\n",
+  InitListExprPreOrderVisitor::Lang_C));
+}
+
 } // end anonymous namespace
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2308,15 +2308,25 @@
   return true;
 }
 
-// This method is called once for each pair of syntactic and semantic
-// InitListExpr, and it traverses the subtrees defined by the two forms. This
-// may cause some of the children to be visited twice, if they appear both in
-// the syntactic and the semantic form.
+// If shouldVisitImplicitCode() returns false, this method traverses only the
+// syntactic form of InitListExpr.
+// If shouldVisitImplicitCode() return true, this method is called once for
+// each pair of syntactic and semantic InitListExpr, and it traverses the
+// subtrees defined by the two forms. This may cause some of the children to be
+// visited twice, if they appear both in the syntactic and the semantic form.
 //
 // There is no guarantee about which form \p S takes when this method is 
called.
 template 
 bool RecursiveASTVisitor::TraverseInitListExpr(
 InitListExpr *S, DataRecursionQueue *Queue) {
+  if (!getDerived().shouldVisitImplicitCode()) {
+// Visit only the syntactic form if the clients are not interested in
+// implicit compiler-generated semantic form.
+TRY_TO(TraverseSynOrSemInitListExpr(
+S->isSyntacticForm() ? S : S->getSyntacticForm(), Queue));
+return true;
+  }
+
   TRY_TO(TraverseSynOrSemInitListExpr(
   S->isSemanticForm() ? S->getSyntacticForm() : S, Queue));
   TRY_TO(TraverseSynOrSemInitListExpr(


Index: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
@@ -17,14 +17,22 @@
 class InitListExprPreOrderVisitor
 : public ExpectedLocationVisitor {
 public:
+  InitListExprPreOrderVisitor(bool VisitImplicitCode)
+  : VisitImplicitCode(VisitImplicitCode) {}
+
+  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
+
   bool VisitInitListExpr(InitListExpr *ILE) {
 Match(ILE->isSemanticForm() ? "semantic" : "syntactic", ILE->getBeginLoc());
 return true;
   }
+
+private:
+  bool VisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, InitListExprIsPreOrderVisitedTwice) {
-  InitListExprPreOrderVisitor Visitor;
+  InitListExprPreOrderVisitor Visitor(/*VisitImplicitCode=*/true);
   Visitor.ExpectMatch("syntactic", 2, 21);
   Visitor.ExpectMatch("semantic", 2, 21);
   EXPECT_TRUE(Visitor.runOver("struct S { int x; };\n"
@@ -32,4 +40,13 @@
   InitListExprPreOrderVisitor::Lang_C));
 }
 
+TEST(RecursiveASTVisitor, InitListExprVisitedOnceWhenNoImplicit) {
+  InitListExprPreOrderVisitor Visitor(/*VisitImplicitCode=*/false);
+  Visitor.ExpectMatch("syntac

[PATCH] D64754: [clangd] Added highlighting for the targets in typedefs.

2019-07-16 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 210053.
jvikstrom added a comment.

Add support for using `A = ...`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64754

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
@@ -90,7 +90,7 @@
 typename T::A* $Field[[D]];
   };
   $Namespace[[abc]]::$Class[[A]] $Variable[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] AAA;
+  typedef $Namespace[[abc]]::$Class[[A]] $Class[[AAA]];
   struct $Class[[B]] {
 $Class[[B]]();
 ~$Class[[B]]();
@@ -166,6 +166,19 @@
 $Variable[[AA]].$Field[[E]].$Field[[C]];
 $Class[[A]]::$Variable[[S]] = 90;
   }
+)cpp",
+R"cpp(
+  namespace $Namespace[[a]] {
+struct $Class[[A]] {};
+  }
+  typedef $Namespace[[a]]::$Class[[A]] $Class[[B]];
+  using $Class[[BB]] = $Namespace[[a]]::$Class[[A]];
+  enum class $Enum[[E]] {};
+  typedef $Enum[[E]] $Enum[[C]];
+  typedef $Enum[[C]] $Enum[[CC]];
+  using $Enum[[CD]] = $Enum[[CC]];
+  $Enum[[CC]] $Function[[f]]($Class[[B]]);
+  $Enum[[CD]] $Function[[f]]($Class[[BB]]);
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -85,6 +85,12 @@
 return true;
   }
 
+  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
+if(const auto *TSI = TD->getTypeSourceInfo())
+  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+return true;
+  }
+
   bool VisitTypeLoc(TypeLoc &TL) {
 // This check is for not getting two entries when there are anonymous
 // structs. It also makes us not highlight certain namespace qualifiers
@@ -93,9 +99,7 @@
 if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
   return true;
 
-if (const Type *TP = TL.getTypePtr())
-  if (const TagDecl *TD = TP->getAsTagDecl())
-addToken(TL.getBeginLoc(), TD);
+addTypeLoc(TL.getBeginLoc(), TL);
 return true;
   }
 
@@ -110,6 +114,12 @@
   }
 
 private:
+  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
+if (const Type *TP = TL.getTypePtr())
+  if (const TagDecl *TD = TP->getAsTagDecl())
+addToken(Loc, TD);
+  }
+
   void addToken(SourceLocation Loc, const NamedDecl *D) {
 if (D->getDeclName().isIdentifier() && D->getName().empty())
   // Don't add symbols that don't have any length.


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -90,7 +90,7 @@
 typename T::A* $Field[[D]];
   };
   $Namespace[[abc]]::$Class[[A]] $Variable[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] AAA;
+  typedef $Namespace[[abc]]::$Class[[A]] $Class[[AAA]];
   struct $Class[[B]] {
 $Class[[B]]();
 ~$Class[[B]]();
@@ -166,6 +166,19 @@
 $Variable[[AA]].$Field[[E]].$Field[[C]];
 $Class[[A]]::$Variable[[S]] = 90;
   }
+)cpp",
+R"cpp(
+  namespace $Namespace[[a]] {
+struct $Class[[A]] {};
+  }
+  typedef $Namespace[[a]]::$Class[[A]] $Class[[B]];
+  using $Class[[BB]] = $Namespace[[a]]::$Class[[A]];
+  enum class $Enum[[E]] {};
+  typedef $Enum[[E]] $Enum[[C]];
+  typedef $Enum[[C]] $Enum[[CC]];
+  using $Enum[[CD]] = $Enum[[CC]];
+  $Enum[[CC]] $Function[[f]]($Class[[B]]);
+  $Enum[[CD]] $Function[[f]]($Class[[BB]]);
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -85,6 +85,12 @@
 return true;
   }
 
+  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
+if(const auto *TSI = TD->getTypeSourceInfo())
+  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+return true;
+  }
+
   bool VisitTypeLoc(TypeLoc &TL) {
 // This check is for not getting two entries when there are anonymous
 // structs. It also makes us not highlight certain namespace qualifiers
@@ -93,9 +99,7 @@
 if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Ela

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

2019-07-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

How should this behave when if the same token is used multiple times inside a 
macro and the uses are different? Could we add this to tests?
E.g.

  #define W(a) class a { void test() { int a = 10; } }
  W(foo); // <-- `foo` is a variable of a class?


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] D62584: [OpenCL][PR42033] Deducing addr space with template parameter types

2019-07-16 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In D62584#1586640 , @rjmccall wrote:

> In D62584#1585091 , @Anastasia wrote:
>
> > In D62584#1583340 , @rjmccall 
> > wrote:
> >
> > > Oh, yes, it definitely can't be done to class types.  I suppose we should 
> > > just forget about it.
> >
> >
> > Ok, regarding address space qualifiers in template instantiation - is it 
> > still ok that we ignore them here in this patch?
>
>
> I think ignoring them in templates rather than diagnosing is the right thing 
> to do, since it means that reasonable things, e.g. template functions 
> returning `T`, won't be erroneous if you instantiate `T = __global int` just 
> like they wouldn't be with `T = const int`.


Ok cool, I have removed the diagnostic btw. :)


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

https://reviews.llvm.org/D62584



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


[PATCH] D64744: Loop #pragma tail_predicate

2019-07-16 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Thanks for taking a look and your suggestions!

I noticed your comment here after I replied to the list. As I wrote there, and 
long story short, I thought I could kill 2 birds with 1 stone, but that doesn't 
seem to be the case. I agree that for the vectorizer an option like this is 
much better:

  #pragma clang loop vectorize(enable) vectorize_remainder(enable)

I will implement that here because that looks a useful addition that people 
like, and I think this will also work for me initially.


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

https://reviews.llvm.org/D64744



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


[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

2019-07-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: include/clang/AST/ASTImporter.h:234
+/// The ASTUnit that this importer belongs to, if any.
+ASTUnit *Unit;
+

`Unit->getASTContext` must be equal to `FromContext`, right?
Then `FromUnit` would be a better naming. 
The importer does handle two ASTContexts, so it would be great to know which 
ASTUnit's ASTContext is which. Or vica-versa, we must know precisely to which 
context does this unit belong. Also a comment would be useful that we must 
store the unit because there is no way to get that from the context.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64554



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


[PATCH] D64789: [clangd] Handle windows line endings in QueryDriver

2019-07-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

fixes second case of https://github.com/clangd/clangd/issues/93


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64789

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test


Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -5,7 +5,7 @@
 # RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh
 # RUN: echo '[ "$0" = "%t.dir/my_driver.sh" ] || exit' >> %t.dir/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh
-# RUN: echo 'echo \#include \<...\> search starts here: >&2' >> 
%t.dir/my_driver.sh
+# RUN: echo 'echo -e "#include <...> search starts here:\r" >&2' >> 
%t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo End of search list. >&2' >> %t.dir/my_driver.sh
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -63,7 +63,9 @@
   llvm::SmallVector Lines;
   Output.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 
-  auto StartIt = std::find(Lines.begin(), Lines.end(), SIS);
+  auto StartIt =
+  std::find_if(Lines.begin(), Lines.end(),
+   [](llvm::StringRef Line) { return Line.trim() == SIS; });
   if (StartIt == Lines.end()) {
 elog("System include extraction: start marker not found: {0}", Output);
 return {};


Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -5,7 +5,7 @@
 # RUN: echo '#!/bin/bash' >> %t.dir/my_driver.sh
 # RUN: echo '[ "$0" = "%t.dir/my_driver.sh" ] || exit' >> %t.dir/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh
-# RUN: echo 'echo \#include \<...\> search starts here: >&2' >> %t.dir/my_driver.sh
+# RUN: echo 'echo -e "#include <...> search starts here:\r" >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo End of search list. >&2' >> %t.dir/my_driver.sh
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -63,7 +63,9 @@
   llvm::SmallVector Lines;
   Output.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 
-  auto StartIt = std::find(Lines.begin(), Lines.end(), SIS);
+  auto StartIt =
+  std::find_if(Lines.begin(), Lines.end(),
+   [](llvm::StringRef Line) { return Line.trim() == SIS; });
   if (StartIt == Lines.end()) {
 elog("System include extraction: start marker not found: {0}", Output);
 return {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64493: [Driver] Don't pass --dynamic-linker to ld on Solaris

2019-07-16 Thread David Chisnall via Phabricator via cfe-commits
theraven added a comment.

I think this was simply mirroring what gcc 4.something did on Solaris.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64493



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


[PATCH] D64753: [CrossTU][NFCI] Refactor loadExternalAST function

2019-07-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

Thank you Endre, this patch is a great initiative.
However, I think we can do better encapsulation then just the reorganization of 
the functions:
We could encapsulate into a nested class `NameASTUnitMap` and the functions 
which operate on this (`getCachedASTUnitForName`,
`loadFromASTFileCached`).
We could do the same with `NameFileMap`, `lazyInitCTUIndex`,  
`getASTFileNameForLookup`.
And we could also encapsulate `NumASTLoaded` and its related function 
(`checkThresholdReached`). In this case perhaps we could use RAII to increase 
the counter.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:167
+  bool checkThresholdReached() const;
+  llvm::Error lazyInitCTUIndex(StringRef CrossTUDir, StringRef IndexName);
+  ASTUnit *getCachedASTUnitForName(StringRef LookupName) const;

Could you please provide comments for these new functions? E.g. it would be 
useful to know what is the `IndexName` param.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:168
+  llvm::Error lazyInitCTUIndex(StringRef CrossTUDir, StringRef IndexName);
+  ASTUnit *getCachedASTUnitForName(StringRef LookupName) const;
+  std::unique_ptr loadFromASTFile(StringRef ASTFileName) const;

`LookupName` is the USR?



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:185
 
   llvm::StringMap> FileASTUnitMap;
   llvm::StringMap NameASTUnitMap;

Could you please add comments for these maps (caches) about what we use them 
for?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:364
+
+  if (auto IndexMapping = parseCrossTUIndex(IndexFile, CrossTUDir)) {
+// Initialize member map.

I don't think we should use `auto` here, because it is not obvious what will be 
the return type. It is important to see that we deal with an `Expected<...>`.
Perhaps we should have a type alias for `llvm::StringMap`.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:448
 
-auto It = NameFileMap.find(LookupName);
-if (It == NameFileMap.end()) {
-  ++NumNotInOtherTU;
-  return 
llvm::make_error(index_error_code::missing_definition);
-}
-StringRef ASTFileName = It->second;
-auto ASTCacheEntry = FileASTUnitMap.find(ASTFileName);
-if (ASTCacheEntry == FileASTUnitMap.end()) {
-  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  TextDiagnosticPrinter *DiagClient =
-  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
-  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
-  IntrusiveRefCntPtr Diags(
-  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
-
-  std::unique_ptr LoadedUnit(ASTUnit::LoadFromASTFile(
-  ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
-  ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
-  Unit = LoadedUnit.get();
-  FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
-  ++NumASTLoaded;
-  if (DisplayCTUProgress) {
-llvm::errs() << "CTU loaded AST file: "
- << ASTFileName << "\n";
-  }
-} else {
-  Unit = ASTCacheEntry->second.get();
+  // Lazily initialize the mapping from function names to AST files.
+  if (llvm::Error InitFailed = lazyInitCTUIndex(CrossTUDir, IndexName))

This comment might be also placed in the .h file as a oneliner above the 
function's prototype.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64753



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


[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

FYI, this change broke git-llvm for everyone with a different username :-)
Fixed in r366198


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64495



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


[clang-tools-extra] r366199 - [clangd] Don't rebuild background index until we indexed one TU per thread.

2019-07-16 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Tue Jul 16 03:17:06 2019
New Revision: 366199

URL: http://llvm.org/viewvc/llvm-project?rev=366199&view=rev
Log:
[clangd] Don't rebuild background index until we indexed one TU per thread.

Summary:
This increases the odds that the boosted file (cpp file matching header)
will be ready. (It always enqueues first, so it'll be present unless
another thread indexes *two* files before the first thread indexes one.)

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, llvm-commits

Tags: #llvm

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

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h
clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=366199&r1=366198&r2=366199&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Jul 16 03:17:06 2019
@@ -127,7 +127,7 @@ BackgroundIndex::BackgroundIndex(
 BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize)
 : SwapIndex(llvm::make_unique()), FSProvider(FSProvider),
   CDB(CDB), BackgroundContext(std::move(BackgroundContext)),
-  Rebuilder(this, &IndexedSymbols),
+  Rebuilder(this, &IndexedSymbols, ThreadPoolSize),
   IndexStorageFactory(std::move(IndexStorageFactory)),
   CommandsChanged(
   CDB.watch([&](const std::vector &ChangedFiles) {

Modified: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h?rev=366199&r1=366198&r2=366199&view=diff
==
--- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h (original)
+++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h Tue Jul 16 
03:17:06 2019
@@ -16,6 +16,7 @@
 
 #include "index/FileIndex.h"
 #include "index/Index.h"
+#include "llvm/Support/Threading.h"
 
 namespace clang {
 namespace clangd {
@@ -45,12 +46,9 @@ namespace clangd {
 // This class is exposed in the header so it can be tested.
 class BackgroundIndexRebuilder {
 public:
-  // Thresholds for rebuilding as TUs get indexed.
-  static constexpr unsigned TUsBeforeFirstBuild = 5;
-  static constexpr unsigned TUsBeforeRebuild = 100;
-
-  BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source)
-  : Target(Target), Source(Source) {}
+  BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source,
+   unsigned Threads)
+  : TUsBeforeFirstBuild(Threads), Target(Target), Source(Source) {}
 
   // Called to indicate a TU has been indexed.
   // May rebuild, if enough TUs have been indexed.
@@ -71,6 +69,10 @@ public:
   // Ensures we won't start any more rebuilds.
   void shutdown();
 
+  // Thresholds for rebuilding as TUs get indexed.
+  const unsigned TUsBeforeFirstBuild; // Typically one per worker thread.
+  const unsigned TUsBeforeRebuild = 100;
+
 private:
   // Run Check under the lock, and rebuild if it returns true.
   void maybeRebuild(const char *Reason, std::function Check);

Modified: clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp?rev=366199&r1=366198&r2=366199&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Tue Jul 
16 03:17:06 2019
@@ -575,7 +575,8 @@ TEST_F(BackgroundIndexTest, CmdLineHash)
 class BackgroundIndexRebuilderTest : public testing::Test {
 protected:
   BackgroundIndexRebuilderTest()
-  : Target(llvm::make_unique()), Rebuilder(&Target, &Source) {
+  : Target(llvm::make_unique()),
+Rebuilder(&Target, &Source, /*Threads=*/10) {
 // Prepare FileSymbols with TestSymbol in it, for checkRebuild.
 TestSymbol.ID = SymbolID("foo");
   }
@@ -610,11 +611,10 @@ protected:
 };
 
 TEST_F(BackgroundIndexRebuilderTest, IndexingTUs) {
-  for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeFirstBuild - 1;
-   ++I)
+  for (unsigned I = 0; I < Rebuilder.TUsBeforeFirstBuild - 1; ++I)
 EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
   EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); }));
-  for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeRebuild - 1; ++I)
+  for (unsigned I = 0; I < Rebuilder.TUsBeforeRebuild - 1; ++I)
 EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
   EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexed

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

2019-07-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: include/clang/AST/ASTImporter.h:317
+std::shared_ptr SharedState = nullptr,
+ASTUnit *Unit = nullptr);
 

What if we provided an additional constructor where we take over the ASTUnits 
instead of the ASTContexts?
Then we would not need to pass the FileManagers neither.
```
ASTImporter(ASTUnit &ToUnit, 
ASTUnit &FromUnit,
bool MinimalImport,
std::shared_ptr SharedState = nullptr,
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D64554



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


[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

2019-07-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332
   S->isSemanticForm() ? S->getSyntacticForm() : S, Queue));
   TRY_TO(TraverseSynOrSemInitListExpr(
   S->isSemanticForm() ? S : S->getSemanticForm(), Queue));

ilya-biryukov wrote:
> gribozavr wrote:
> > Instead of adding a whole new if statement, could you wrap the second 
> > existing TRY_TO in `if(shouldVisitImplicitCode())` ?
> Despite looking very similar, that would **not** be equivalent to the current 
> version.
> 
> For most init lists (that do not have alternative "form"), the following 
> invariants hold:
> ```
> InitList* E = ...;
> assert(E->isSemanticForm());
> assert(E->isSyntacticForm()); 
> assert(E->getSynacticForm() == nullptr);
> ```
> 
> This subtle fact means the current code does not traversed the list twice if 
> they do not have an alternative form (either semantic or syntactic).
> 
> Now, if we only run the first statement, we will call 
> `TraverseSynOrSemInitListExpr(S->getSyntacticForm())` and 
> `S->getSyntacticForm()` returns `null`. So we don't traverse anything.
> 
> I tried various ways to keep both calls, but they all ended up being too 
> complicated, hence the final version. Let me know if you see a better way to 
> address this.
To make the last sentence less confusing:
I tried various ways to keep **only two** calls, but they were too complicated 
and I ended up introducing an extra call to `TraverseSyn...` instead.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64762



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


r366200 - [SemaTemplate] Fix uncorrected typos after pack expansion

2019-07-16 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Tue Jul 16 03:30:21 2019
New Revision: 366200

URL: http://llvm.org/viewvc/llvm-project?rev=366200&view=rev
Log:
[SemaTemplate] Fix uncorrected typos after pack expansion

Summary:
This case is particularly important for clangd, as it is triggered after
inserting the snippet for variadic functions.

Reviewers: kadircet, ilya-biryukov

Subscribers: llvm-commits

Tags: #llvm

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

Added:
cfe/trunk/test/SemaTemplate/typo-variadic.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp?rev=366200&r1=366199&r2=366200&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp Tue Jul 16 03:30:21 2019
@@ -619,6 +619,7 @@ ExprResult Sema::CheckPackExpansion(Expr
   if (!Pattern->containsUnexpandedParameterPack()) {
 Diag(EllipsisLoc, diag::err_pack_expansion_without_parameter_packs)
 << Pattern->getSourceRange();
+CorrectDelayedTyposInExpr(Pattern);
 return ExprError();
   }
 

Added: cfe/trunk/test/SemaTemplate/typo-variadic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/typo-variadic.cpp?rev=366200&view=auto
==
--- cfe/trunk/test/SemaTemplate/typo-variadic.cpp (added)
+++ cfe/trunk/test/SemaTemplate/typo-variadic.cpp Tue Jul 16 03:30:21 2019
@@ -0,0 +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+int x = m(s...); // expected-error{{pack expansion does not}} 
expected-error{{undeclared identifier}}


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


[PATCH] D64677: [SemaTemplate] Fix uncorrected typos after pack expansion

2019-07-16 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366200: [SemaTemplate] Fix uncorrected typos after pack 
expansion (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64677?vs=209631&id=210057#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64677

Files:
  cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
  cfe/trunk/test/SemaTemplate/typo-variadic.cpp


Index: cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
@@ -619,6 +619,7 @@
   if (!Pattern->containsUnexpandedParameterPack()) {
 Diag(EllipsisLoc, diag::err_pack_expansion_without_parameter_packs)
 << Pattern->getSourceRange();
+CorrectDelayedTyposInExpr(Pattern);
 return ExprError();
   }
 
Index: cfe/trunk/test/SemaTemplate/typo-variadic.cpp
===
--- cfe/trunk/test/SemaTemplate/typo-variadic.cpp
+++ cfe/trunk/test/SemaTemplate/typo-variadic.cpp
@@ -0,0 +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+int x = m(s...); // expected-error{{pack expansion does not}} 
expected-error{{undeclared identifier}}


Index: cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
@@ -619,6 +619,7 @@
   if (!Pattern->containsUnexpandedParameterPack()) {
 Diag(EllipsisLoc, diag::err_pack_expansion_without_parameter_packs)
 << Pattern->getSourceRange();
+CorrectDelayedTyposInExpr(Pattern);
 return ExprError();
   }
 
Index: cfe/trunk/test/SemaTemplate/typo-variadic.cpp
===
--- cfe/trunk/test/SemaTemplate/typo-variadic.cpp
+++ cfe/trunk/test/SemaTemplate/typo-variadic.cpp
@@ -0,0 +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+int x = m(s...); // expected-error{{pack expansion does not}} expected-error{{undeclared identifier}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

2019-07-16 Thread Kyrill Tkachov via Phabricator via cfe-commits
ktkachov added a comment.

In D64495#1587266 , @sammccall wrote:

> FYI, this change broke git-llvm for everyone with a different username :-)
>  Fixed in r366198


Ah sorry, I accidentally included it in the commit!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64495



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


[PATCH] D64712: [clangd][NFC] Refactor background-index shard loading

2019-07-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

So I've stared at this refactoring for a while, and I still don't totally get 
it.
It seems like a class that really wants to be a function. It's a complicated 
function though - maybe a separate cpp file does make sense.

We discussed moving ShardVersions and Writes into the loader, splitting 
ShardVersions from IndexSymbols seems pretty odd though.

What's the advantage of making this stateful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64712



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


[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

2019-07-16 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked an inline comment as done.
balazske added inline comments.



Comment at: include/clang/AST/ASTImporter.h:317
+std::shared_ptr SharedState = nullptr,
+ASTUnit *Unit = nullptr);
 

martong wrote:
> What if we provided an additional constructor where we take over the ASTUnits 
> instead of the ASTContexts?
> Then we would not need to pass the FileManagers neither.
> ```
> ASTImporter(ASTUnit &ToUnit, 
> ASTUnit &FromUnit,
> bool MinimalImport,
> std::shared_ptr SharedState = nullptr,
> ```
Is the `SharedState==nullptr` case only for LLDB? If yes then it is possible to 
have a "LLDB" constructor when no shared state and no ASTUnit is needed. And 
another for CTU case when a From and To ASTUnit is specified and a shared state 
(theoretically minimal can be true in any case but probably only true for 
LLDB?).


Repository:
  rC Clang

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

https://reviews.llvm.org/D64554



___
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-07-16 Thread Anton Afanasyev via Phabricator via cfe-commits
anton-afanasyev added a comment.

Ping!


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


r366202 - [Driver] Don't pass --dynamic-linker to ld on Solaris

2019-07-16 Thread Rainer Orth via cfe-commits
Author: ro
Date: Tue Jul 16 04:06:43 2019
New Revision: 366202

URL: http://llvm.org/viewvc/llvm-project?rev=366202&view=rev
Log:
[Driver] Don't pass --dynamic-linker to ld on Solaris

I noticed that clang currently passes --dynamic-linker to ld.  This has been 
the case
since Solaris 11 support was added initially back in 2012 by David Chisnall 
(r150580).
I couldn't find any patch submission, let alone a justification, for this, and 
it seems
completely useless: --dynamic-linker is a gld compatibility form of the option, 
the
native option being -I.  First of all, however, the dynamic linker passed is 
simply the
default, so there's no reason at all to specify it in the first place.

This patch removes passing the option and adjusts the affected testcase 
accordingly.

Tested on x86_64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

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

Removed:
cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/ld.so.1
cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/sparcv9/ld.so.1
cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/amd64/ld.so.1
cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/ld.so.1
Modified:
cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
cfe/trunk/test/Driver/solaris-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Solaris.cpp?rev=366202&r1=366201&r2=366202&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Solaris.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Solaris.cpp Tue Jul 16 04:06:43 2019
@@ -65,10 +65,6 @@ void solaris::Linker::ConstructJob(Compi
 CmdArgs.push_back("-Bdynamic");
 if (Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back("-shared");
-} else {
-  CmdArgs.push_back("--dynamic-linker");
-  CmdArgs.push_back(
-  Args.MakeArgString(getToolChain().GetFilePath("ld.so.1")));
 }
 
 // libpthread has been folded into libc since Solaris 10, no need to do

Removed: cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/ld.so.1
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/ld.so.1?rev=366201&view=auto
==
(empty)

Removed: cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/sparcv9/ld.so.1
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/sparcv9/ld.so.1?rev=366201&view=auto
==
(empty)

Removed: cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/amd64/ld.so.1
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/amd64/ld.so.1?rev=366201&view=auto
==
(empty)

Removed: cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/ld.so.1
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/ld.so.1?rev=366201&view=auto
==
(empty)

Modified: cfe/trunk/test/Driver/solaris-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/solaris-ld.c?rev=366202&r1=366201&r2=366202&view=diff
==
--- cfe/trunk/test/Driver/solaris-ld.c (original)
+++ cfe/trunk/test/Driver/solaris-ld.c Tue Jul 16 04:06:43 2019
@@ -11,7 +11,6 @@
 // CHECK-LD-SPARC32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"sparc-sun-solaris2.11"
 // CHECK-LD-SPARC32-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-SPARC32: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-SPARC32-SAME: "--dynamic-linker" 
"[[SYSROOT]]/usr/lib{{/|}}ld.so.1"
 // CHECK-LD-SPARC32-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|}}crt1.o"
 // CHECK-LD-SPARC32-SAME: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD-SPARC32-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|}}crtbegin.o"
@@ -35,7 +34,6 @@
 // CHECK-LD-SPARC64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"sparcv9-sun-solaris2.11"
 // CHECK-LD-SPARC64-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-SPARC64: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-SPARC64-SAME: "--dynamic-linker" 
"[[SYSROOT]]/usr/lib/sparcv9{{/|}}ld.so.1"
 // CHECK-LD-SPARC64-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9{{/|}}crt1.o"
 // CHECK-LD-SPARC64-SAME: "[[SYSROOT]]/usr/lib/sparcv9{{/|}}crti.o"
 // CHECK-LD-SPARC64-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9{{/|}}crtbegin.o"
@@ -59,7 +57,6 @@
 // CHECK-LD-X32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "i386-pc-solaris2.11"
 // CHECK-LD-X32-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-L

[PATCH] D64493: [Driver] Don't pass --dynamic-linker to ld on Solaris

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366202: [Driver] Don't pass --dynamic-linker to ld on 
Solaris (authored by ro, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64493?vs=208954&id=210059#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64493

Files:
  cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
  cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/ld.so.1
  cfe/trunk/test/Driver/Inputs/solaris_sparc_tree/usr/lib/sparcv9/ld.so.1
  cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/amd64/ld.so.1
  cfe/trunk/test/Driver/Inputs/solaris_x86_tree/usr/lib/ld.so.1
  cfe/trunk/test/Driver/solaris-ld.c


Index: cfe/trunk/test/Driver/solaris-ld.c
===
--- cfe/trunk/test/Driver/solaris-ld.c
+++ cfe/trunk/test/Driver/solaris-ld.c
@@ -11,7 +11,6 @@
 // CHECK-LD-SPARC32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"sparc-sun-solaris2.11"
 // CHECK-LD-SPARC32-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-SPARC32: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-SPARC32-SAME: "--dynamic-linker" 
"[[SYSROOT]]/usr/lib{{/|}}ld.so.1"
 // CHECK-LD-SPARC32-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|}}crt1.o"
 // CHECK-LD-SPARC32-SAME: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD-SPARC32-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|}}crtbegin.o"
@@ -35,7 +34,6 @@
 // CHECK-LD-SPARC64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"sparcv9-sun-solaris2.11"
 // CHECK-LD-SPARC64-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-SPARC64: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-SPARC64-SAME: "--dynamic-linker" 
"[[SYSROOT]]/usr/lib/sparcv9{{/|}}ld.so.1"
 // CHECK-LD-SPARC64-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9{{/|}}crt1.o"
 // CHECK-LD-SPARC64-SAME: "[[SYSROOT]]/usr/lib/sparcv9{{/|}}crti.o"
 // CHECK-LD-SPARC64-SAME: 
"[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9{{/|}}crtbegin.o"
@@ -59,7 +57,6 @@
 // CHECK-LD-X32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "i386-pc-solaris2.11"
 // CHECK-LD-X32-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-X32: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-X32-SAME: "--dynamic-linker" "[[SYSROOT]]/usr/lib{{/|}}ld.so.1"
 // CHECK-LD-X32-SAME: "[[SYSROOT]]/usr/lib{{/|}}crt1.o"
 // CHECK-LD-X32-SAME: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD-X32-SAME: 
"[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4{{/|}}crtbegin.o"
@@ -83,7 +80,6 @@
 // CHECK-LD-X64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"x86_64-pc-solaris2.11"
 // CHECK-LD-X64-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-X64: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-X64-SAME: "--dynamic-linker" 
"[[SYSROOT]]/usr/lib/amd64{{/|}}ld.so.1"
 // CHECK-LD-X64-SAME: "[[SYSROOT]]/usr/lib/amd64{{/|}}crt1.o"
 // CHECK-LD-X64-SAME: "[[SYSROOT]]/usr/lib/amd64{{/|}}crti.o"
 // CHECK-LD-X64-SAME: 
"[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/amd64{{/|}}crtbegin.o"
Index: cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
@@ -65,10 +65,6 @@
 CmdArgs.push_back("-Bdynamic");
 if (Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back("-shared");
-} else {
-  CmdArgs.push_back("--dynamic-linker");
-  CmdArgs.push_back(
-  Args.MakeArgString(getToolChain().GetFilePath("ld.so.1")));
 }
 
 // libpthread has been folded into libc since Solaris 10, no need to do


Index: cfe/trunk/test/Driver/solaris-ld.c
===
--- cfe/trunk/test/Driver/solaris-ld.c
+++ cfe/trunk/test/Driver/solaris-ld.c
@@ -11,7 +11,6 @@
 // CHECK-LD-SPARC32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "sparc-sun-solaris2.11"
 // CHECK-LD-SPARC32-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-SPARC32: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-SPARC32-SAME: "--dynamic-linker" "[[SYSROOT]]/usr/lib{{/|}}ld.so.1"
 // CHECK-LD-SPARC32-SAME: "[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|}}crt1.o"
 // CHECK-LD-SPARC32-SAME: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD-SPARC32-SAME: "[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|}}crtbegin.o"
@@ -35,7 +34,6 @@
 // CHECK-LD-SPARC64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "sparcv9-sun-solaris2.11"
 // CHECK-LD-SPARC64-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD-SPARC64: "{{.*}}ld{{(.exe)?}}"
-// CHECK-LD-SPARC64-SAME: "--dynamic-linker" "[[SYSROOT]]/usr/lib/sparcv9{{/|}}ld.so.1"
 // CHECK-LD-SPARC64-SAME: "[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9{{/|\\

[PATCH] D64791: [OpenCL] Fix sampler initialization for C++ mode

2019-07-16 Thread Neil Hickey via Phabricator via cfe-commits
neil.hickey created this revision.
neil.hickey added a reviewer: Anastasia.
Herald added a subscriber: yaxunl.

Sampler initialization from integer literal was broken in C++ mode for OpenCL. 
Fixing to allow functions that take a sampler to take an integer and for 
samplers to be initialized to integer values.


https://reviews.llvm.org/D64791

Files:
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  test/CodeGenOpenCL/sampler.cl

Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=c++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
 //
 // This test covers 5 cases of sampler initialzation:
 //   1. function argument passing
@@ -29,7 +30,7 @@
 int get_sampler_initializer(void);
 
 void fnc4smp(sampler_t s) {}
-// CHECK: define spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* %
+// CHECK: define spir_func void [[FUNCNAME:@.*fnc4smp.*]](%opencl.sampler_t addrspace(2)* %
 
 kernel void foo(sampler_t smp_par) {
   // CHECK-LABEL: define spir_kernel void @foo(%opencl.sampler_t addrspace(2)* %smp_par)
@@ -45,32 +46,32 @@
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 19)
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1b
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 19)
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1a/2a
   fnc4smp(glb_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1a/2c
   fnc4smp(glb_smp_const);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1c
   fnc4smp(smp_par);
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_par_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 5)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
   fnc4smp(const_smp);
@@ -78,12 +79,12 @@
   // CHECK: store %opencl.sampler_t addrspace(2)* [[CONST_SAMP]], %opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR:%[a-zA-Z0-9]+]]
   fnc4smp(const_smp);
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   constant sampler_t constant_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
   fnc4smp(constant_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // TODO: enable sampler initialization with non-constant integer.
   //const sampler_t const_smp_func_init = get_sampler_initializer();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -1851,6 +1851,10 @@
  (From->EvaluateKnownConstInt(S.getASTContext()) == 0)) {
 SCS.Second = ICK_Zero_Queue_Conversion;
 FromType = ToType;
+  } el

[PATCH] D64613: [clangd] Type hierarchy: don't resolve parents if the client only asked for children

2019-07-16 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked an inline comment as done.
nridge added a comment.

(Changed reviewer to Kadir as he reviewed the previous version.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64613



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


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

2019-07-16 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 210064.
Anastasia marked 5 inline comments as done.
Anastasia added a comment.

Addressed some review comments. Refs are still to be fixed!


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 +2397,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 Clang9
+C++ mode is available for OpenCL (see :ref:``).
 
 OpenCL Specific Options
 ---
@@ -2756,6 +2757,45 @@
   enqueue query functions from `section 6.13.17.5
   `_.
 
+.. _opencl_cpp:
+
+C++ for OpenCL
+--
+
+Starting from Clang9 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.
+
+There are only a few restrictions on allowed C++ features. For detailed information
+please refer to documentation on Extensions (: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 the new mode.
+
+To enable the new mode pass the following command line option when compiling ``.cl``
+file ``-cl-std=c++`` or ``-std=c++``.
+
+   .. 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=c++ test.cl
+
 .. _target_features:
 
 Target-Specific Features and Limitations
Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1516,6 +1516,268 @@
 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. Regular C++
+features can be used in OpenCL kernel code. All 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
+- ``dynamic_cast`` operator
+- Non-placement ``new``/``delete`` operators
+- Standard C++ libraries. Currently there is no solution for alternative
+  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 yet, Clang extends existing concept
+from C and OpenCL. For example conversion rules are extended from qualification
+conversions but the compatibility is determined using sets and overlapping from
+Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
+implicit conversions are allowed from named to ``__generic`` but not vice versa
+(OpenCL C v2.0 s6.5.5) except for constant address space. Most of the rules
+are built on top of this behavior.
+
+**Casts**
+
+C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
+permit implicit conversion to ``__generic``. However converting from named
+address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
+that conversions between ``__constant`` and any other is still disallowed.
+
+.. _opencl_cpp_addrsp_deduction:
+
+**Deduction**
+
+Address spaces are not deduced for:
+
+- non-pointer/non-reference template parameters or any dependent types are except
+  for template specializations.
+- non-pointer/non-reference class members except for static data members that are
+  deduced to __global address space. 
+- non-pointer/non-reference alias declar

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

2019-07-16 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: docs/LanguageExtensions.rst:1673
+
+All builtin operators will be available in the specific address spaces, thus 
no conversion
+to generic is performed.

mantognini wrote:
> "specific" seems to imply to the user will have to explicitly write down the 
> AS. Maybe "requested" or "desired" would fit better?
Ok, specific here is as opposite to generic. 



Comment at: docs/LanguageExtensions.rst:1753-1761
+Global objects are constructed before the first kernel using the global
+objects is executed and destroyed just after the last kernel using the
+program objects is executed. In OpenCL v2.0 drivers there is no specific
+API for invoking global constructors. However, an easy workaround would be
+to enqueue constructor initialization kernel that has a name
+``@_GLOBAL__sub_I_``. This kernel is only present if there
+are any global objects to be initialized in the compiled binary. One way to

mantognini wrote:
> Is it expected of the user to call this special kernel, or is it expected 
> that the driver takes care of that?
In v2.0 drivers this is a manual step. In v2.2 drivers it is expected to be 
done automatically.



Comment at: docs/LanguageExtensions.rst:1758
+to enqueue constructor initialization kernel that has a name
+``@_GLOBAL__sub_I_``. This kernel is only present if there
+are any global objects to be initialized in the compiled binary. One way to

keryell wrote:
> " Is there an issue if you link a program from several files with the same file 
> names?
I think IR linker makes them unique i.e. adds a suffix. Although I think the 
files (including the path) linked are supposed to be unique. Anyway, good point 
to look into.

No idea about obj linker. We currently don't support this feature.



Comment at: docs/LanguageExtensions.rst:1768
+.. code-block:: console
+ clang -cl-std=c++ test.cl
+

mantognini wrote:
> As it stands, I'm not sure why this command is mentioned in this paragraph. 
> Maybe it's a leftover of things that are now in the User's Manual?
This is the example I am explaining just below.



Comment at: docs/UsersManual.rst:2797
+
+ clang -cl-std=c++ test.cl
+

mantognini wrote:
> I would remove this code sample; it is already explained above that 
> `-cl-std=c++` should be used.
Manual is good for examples. Some new developers might find it useful to have 
the full line even though it's quite simple.


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] D64791: [OpenCL] Fix sampler initialization for C++ mode

2019-07-16 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!

I expect the example from the bug is going to compile now: 
https://bugs.llvm.org/show_bug.cgi?id=41009

Can you please close the bug after the commit.




Comment at: lib/Sema/SemaOverload.cpp:1854
 FromType = ToType;
+  } else if (ToType->isSamplerT() &&
+ From->isIntegerConstantExpr(S.getASTContext())) {

My only comment here is that we might be able to remove the same logic where it 
works just for C. But it can be done in a separate patch too.



Comment at: lib/Sema/SemaOverload.cpp:1856
+ From->isIntegerConstantExpr(S.getASTContext())) {
+SCS.Second = ICK_Compatible_Conversion;
+FromType = ToType;

No idea why we are adding separate conversion kind for event and queue types. 
Again, it might be something we can simplify above.


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

https://reviews.llvm.org/D64791



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


[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

2019-07-16 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything(.bind("vardecl"), this);
+}

jpakkane wrote:
> alexfh wrote:
> > I believe, this should skip matches within template instantiations. 
> > Consider this code:
> > ```
> > template
> > void f(T) { T t; }
> > void g() {
> > f(0);
> > f(0.0);
> > }
> > ```
> > 
> > What will the fix  be?
> I tested with the following function:
> 
> 
> ```
> template
> void template_test_function() {
>   T t;
>   int uninitialized;
> }
> ```
> 
> Currently it warns on the "uninitialized" variable regardless of whether the 
> template is instantiated or not. If you call it with an int type, it will 
> warn about variable t being uninitialized. If you call it with a, say, struct 
> type, there is no warnings. Is this a reasonable approach?
And what happens, if there are multiple instantiations of the same template, 
each of them requiring a different fix? Can you try the check with my example 
above (and maybe also add `f("");`inside `g()`). I believe, the check will 
produce multiple warnings with conflicting fixes (and each of them will be 
wrong, btw).



Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:32
+  StringRef VarName = MatchedDecl->getName();
+  if (VarName.empty() || VarName.front() == '_') {
+// Some standard library methods such as "be64toh" are implemented

jpakkane wrote:
> alexfh wrote:
> > Should this just disallow all fixes within macros? Maybe warnings as well.
> I can change that, seems reasonable. Should it still retain this check, 
> though? One would imagine there are other ways of getting variables whose 
> names begin with an underscore.
I don't know, whether the check for leading underscore will still be valuable. 
I don't think there's a valid reason why variables with a leading underscore in 
their names should in general not be initialized.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D64671



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


[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

2019-07-16 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware updated this revision to Diff 210070.
baloghadamsoftware added a comment.

Updated according to the comments.


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

https://reviews.llvm.org/D64736

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tidy/bugprone/InfiniteLoopCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-infinite-loop.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-infinite-loop.cpp

Index: test/clang-tidy/bugprone-infinite-loop.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-infinite-loop.cpp
@@ -0,0 +1,226 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t
+
+void simple_infinite_loop1() {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: None of the condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+j++;
+  } while (i < 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: None of the condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+
+  for (i = 0; i < 10; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: None of the condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+  }
+}
+
+void simple_infinite_loop2() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: None of the condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+j++;
+  } while (i < Limit);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: None of the condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+
+  for (i = 0; i < Limit; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: None of the condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+}
+
+void simple_not_infinite1() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) { // Not an error since 'Limit' is updated
+Limit--;
+  }
+  do {
+Limit--;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; Limit--) {
+  }
+}
+
+void simple_not_infinite2() {
+  for (int i = 10; i-- > 0;) // Not an error, since loop variable is modified in
+;// the condition part.
+}
+
+int any_function();
+
+void function_call() {
+  int i = 0;
+  while (i < any_function()) { // Not an error, since the function may return
+   // different values
+  }
+
+  do { // Not an error, since the function may return
+   // different values
+  } while (i < any_function());
+
+  for (i = 0; i < any_function();) { // Not an error, since the function may
+ // return different values
+  }
+}
+
+void escape_before1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = &i;
+  while (i < Limit) { // Not an error, since *p is alias of i.
+(*p)++;
+  }
+
+  do {
+(*p)++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++(*p)) {
+  }
+}
+
+void escape_before2() {
+  int i = 0;
+  int Limit = 100;
+  int &ii = i;
+  while (i < Limit) { // Not an error, since ii is alias of i.
+ii++;
+  }
+
+  do {
+ii++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++ii) {
+  }
+}
+
+void escape_inside1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = &i;
+  while (i < Limit) { // Not an error, since *p is alias of i.
+int *p = &i;
+(*p)++;
+  }
+
+  do {
+int *p = &i;
+(*p)++;
+  } while (i < Limit);
+}
+
+void escape_inside2() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) { // Not an error, since ii is alias of i.
+int &ii = i;
+ii++;
+  }
+
+  do {
+int &ii = i;
+ii++;
+  } while (i < Limit);
+}
+
+void escape_after1() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: None of the condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+  int *p = &i;
+}
+
+void escape_after2() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: None of the condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+  int &ii = i;
+}
+
+int glob;
+
+void global1(int &x) {
+  int i = 0, Limit = 100;
+  while (x < Limit) { // Not an error since 'x' can be an alias of glob.
+glob++;
+  }
+}
+
+void global2() {
+  int i = 0, Limit = 100;
+  while (glob < Limit) { // Since 'glob' is declared out of the function we do
+ // not warn.
+i++;
+  }
+}
+
+struct X {
+  int m;
+
+  void change_m();
+
+  void member_expr1(int i) {
+while (i < m) { // False negative: No warning, since skipping the case where
+// a member_expr can be foun

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

2019-07-16 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked 5 inline comments as done.
baloghadamsoftware added a comment.

In D64736#1585484 , @lebedev.ri wrote:

> Thanks for working on this!
>  You want to use `clang/Analysis/Analyses/ExprMutationAnalyzer.h`.
>  See also: D51870 


Thank you for your suggestion. I was not aware of this function. Now I use it 
and it seems to work perfectly. Sorry, neither was aware of your pending patch 
for the same functionality.


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

https://reviews.llvm.org/D64736



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


[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

2019-07-16 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked an inline comment as not done.
balazske added inline comments.



Comment at: include/clang/AST/ASTImporter.h:317
+std::shared_ptr SharedState = nullptr,
+ASTUnit *Unit = nullptr);
 

balazske wrote:
> martong wrote:
> > What if we provided an additional constructor where we take over the 
> > ASTUnits instead of the ASTContexts?
> > Then we would not need to pass the FileManagers neither.
> > ```
> > ASTImporter(ASTUnit &ToUnit, 
> > ASTUnit &FromUnit,
> > bool MinimalImport,
> > std::shared_ptr SharedState = 
> > nullptr,
> > ```
> Is the `SharedState==nullptr` case only for LLDB? If yes then it is possible 
> to have a "LLDB" constructor when no shared state and no ASTUnit is needed. 
> And another for CTU case when a From and To ASTUnit is specified and a shared 
> state (theoretically minimal can be true in any case but probably only true 
> for LLDB?).
It is not trivial to make and use this new kind of constructor (with ToUnit), 
there is no ToUnit in the CrossTU context. Is it OK to have the original 
constructor, or one with `FromUnit` (but `ToContext` and `ToFileManager`)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64554



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


[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

2019-07-16 Thread Borsik Gábor via Phabricator via cfe-commits
boga95 updated this revision to Diff 210073.
boga95 marked 11 inline comments as done.
boga95 added a comment.

Report diagnostic error in case of an invalid filename or an ill-formed yaml 
file.


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

https://reviews.llvm.org/D59555

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Checkers/Yaml.h
  test/Analysis/Inputs/taint-generic-config.yaml
  test/Analysis/taint-generic.c

Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
-// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
 char *gets(char *str);
Index: test/Analysis/Inputs/taint-generic-config.yaml
===
--- /dev/null
+++ test/Analysis/Inputs/taint-generic-config.yaml
@@ -0,0 +1,51 @@
+# A list of source/propagation function
+Propagations:
+  # int x = mySource1(); // x is tainted
+  - Name: mySource1
+DstArgs:  [-1] # Index for return value
+
+  # int x;
+  # mySource2(&x); // x is tainted
+  - Name: mySource2
+DstArgs:  [0]
+
+  # int x, y;
+  # myScanf("%d %d", &x, &y); // x and y are tainted
+  - Name:  myScanf
+VariadicType:  Dst
+VariadicIndex: 1
+
+  # int x; // x is tainted
+  # int y;
+  # myPropagator(x, &y); // y is tainted
+  - Name: myPropagator
+SrcArgs:  [0]
+DstArgs:  [1]
+
+  # constexpr unsigned size = 100;
+  # char buf[size];
+  # int x, y;
+  # int n = mySprintf(buf, size, "%d %d", x, y); // If size, x or y is tainted
+  # // the return value and the buf will be tainted
+  - Name:  mySnprintf
+SrcArgs:   [1]
+DstArgs:   [0, -1]
+VariadicType:  Src
+VariadicIndex: 3
+
+# A list of filter functions
+Filters:
+  # int x; // x is tainted
+  # myFilter(&x); // x is not tainted anymore
+  - Name: myFilter
+Args: [0]
+
+# A list of sink functions
+Sinks:
+  # int x, y; // x and y are tainted
+  # mySink(x, 0, 1); // It will warn
+  # mySink(0, 1, y); // It will warn
+  # mySink(0, x, 1); // It won't warn
+  - Name: mySink
+Args: [0, 2]
+
Index: lib/StaticAnalyzer/Checkers/Yaml.h
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/Yaml.h
@@ -0,0 +1,36 @@
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/Support/YAMLTraits.h"
+
+/// Read the given file from the filesystem and parse it as a yaml file. The
+/// template parameter must have a yaml MappingTraits.
+template 
+llvm::Optional getConfiguration(clang::ento::CheckerManager &Mgr,
+   Checker *Chk, llvm::StringRef Option,
+   llvm::StringRef ConfigFile) {
+  if (ConfigFile.trim().empty())
+return {};
+
+  llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get();
+  llvm::ErrorOr> Buffer =
+  FS->getBufferForFile(ConfigFile.str());
+
+  if (std::error_code ec = Buffer.getError()) {
+Mgr.reportInvalidCheckerOptionValue(Chk, Option,
+"a valid filename instead of '" +
+std::string(ConfigFile) + "'");
+return {};
+  }
+
+  llvm::yaml::Input Input(Buffer.get()->getBuffer());
+  T Config;
+  Input >> Config;
+
+  if (std::error_code ec = Input.error()) {
+Mgr.reportInvalidCheckerOptionValue(Chk, Option,
+"a valid yaml file: " + ec.message());
+return {};
+  }
+
+  return Config;
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -15,16 +15,19 @@
 //===--===//
 
 #include "Taint.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
+#includ

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

2019-07-16 Thread Rainer Orth via Phabricator via cfe-commits
ro created this revision.
ro added reviewers: fedor.sergeev, rsmith, mehdi_amini, rnk.
Herald added subscribers: kadircet, jrtc27, ilya-biryukov, mgorny, jyknight.
Herald added a project: clang.

`Builtins-*-sunos :: compiler_rt_logbf_test.c` currently FAILs on Solaris, both 
SPARC and
x86, 32 and 64-bit.

It turned out that this is due to different behaviour of `logb` depending on 
the C
standard compiled for, as documented on `logb(3M)`:

  RETURN VALUES
 Upon successful completion, these functions return the exponent of x.
  
 If x is subnormal:
  
 o  For SUSv3-conforming applications compiled with the c99 com-
piler  driver  (see standards(7)), the exponent of x as if x
were normalized is returned.
  
 o  Otherwise, if compiled with the cc compiler  driver,  -1022,
-126,  and  -16382  are  returned  for  logb(), logbf(), and
logbl(), respectively.

Studio c99 and gcc control this by linking with the appropriate version of 
`values-xpg[46].o`, but clang uses neither of those.

The following patch fixes this by following what gcc does, as corrected some 
time ago
in `Fix use of Solaris values-Xc.o (PR target/40411)`, 
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg02350.html and 
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg02384.html.

It makes use of `LangStandard::getLangStandardForName` to parse the `std=`
values.  However, I found that the function currently doesn't handle the alias 
forms
(like `c90` for `c89`).  Given that it isn't currently used in the clang repo, 
I just added
that handling.

As a consequence, `ClangDriverTests` now also needs to be linked with 
`libclangFrontend`.

Tested on `x86_64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`, and 
`x86_64-pc-linux-gnu`.
Ok for trunk (and a backport to the llvm 9 branch)?


Repository:
  rC Clang

https://reviews.llvm.org/D64793

Files:
  lib/Driver/ToolChains/Solaris.cpp
  lib/Frontend/LangStandards.cpp
  test/Driver/Inputs/solaris_sparc_tree/usr/lib/values-Xa.o
  test/Driver/Inputs/solaris_sparc_tree/usr/lib/values-Xc.o
  test/Driver/Inputs/solaris_sparc_tree/usr/lib/values-xpg4.o
  test/Driver/Inputs/solaris_sparc_tree/usr/lib/values-xpg6.o
  test/Driver/Inputs/solaris_x86_tree/usr/lib/values-Xa.o
  test/Driver/Inputs/solaris_x86_tree/usr/lib/values-Xc.o
  test/Driver/Inputs/solaris_x86_tree/usr/lib/values-xpg4.o
  test/Driver/Inputs/solaris_x86_tree/usr/lib/values-xpg6.o
  test/Driver/solaris-ld-values.c
  test/Driver/solaris-ld-values.cpp
  unittests/Driver/CMakeLists.txt

Index: unittests/Driver/CMakeLists.txt
===
--- unittests/Driver/CMakeLists.txt
+++ unittests/Driver/CMakeLists.txt
@@ -15,4 +15,5 @@
   PRIVATE
   clangDriver
   clangBasic
+  clangFrontend
   )
Index: test/Driver/solaris-ld-values.cpp
===
--- /dev/null
+++ test/Driver/solaris-ld-values.cpp
@@ -0,0 +1,45 @@
+// General tests that the correct versions of values-*.o are used on
+// Solaris targets sane. Note that we use sysroot to make these tests
+// independent of the host system.
+
+// Check sparc-sun-solaris2.11, 32bit
+// RUN: %clang -no-canonical-prefixes -ansi %s -### -o %t.o 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32-ANSI %s
+// CHECK-LD-SPARC32-ANSI: values-Xc.o
+// CHECK-LD-SPARC32-ANSI: values-xpg6.o
+
+// RUN: %clang -no-canonical-prefixes -std=c++98 %s -### -o %t.o 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32-CPP98 %s
+// CHECK-LD-SPARC32-CPP98: values-Xc.o
+// CHECK-LD-SPARC32-CPP98: values-xpg6.o
+
+// RUN: %clang -no-canonical-prefixes -std=c++11 %s -### -o %t.o 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32-CPP11 %s
+// CHECK-LD-SPARC32-CPP11: values-Xc.o
+// CHECK-LD-SPARC32-CPP11: values-xpg6.o
+
+// RUN: %clang -no-canonical-prefixes -std=gnu++98 %s -### -o %t.o 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32-GNUPP98 %s
+// CHECK-LD-SPARC32-GNUPP98: values-Xa.o
+// CHECK-LD-SPARC32-GNUPP98: values-xpg6.o
+
+// Check i386-pc-solaris2.11, 32bit
+// RUN: %clang -no-canonical-prefixes -ANSI %s -### -o %t.o 2>&1 \
+// RUN: --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_x86_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-X32-ANSI %s
+// CHECK-LD-X32-A

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

2019-07-16 Thread Johan Vikström via Phabricator via cfe-commits
jvikstrom updated this revision to Diff 210079.
jvikstrom added a comment.

Moved highlighting state to LSP layer. Removed class holding state. Addressed 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  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
@@ -29,7 +29,9 @@
   return Tokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+std::tuple,
+   std::vector>
+getHighlightingsAnnotated(llvm::StringRef Code) {
   Annotations Test(Code);
   auto AST = TestTU::withCode(Test.code()).build();
   static const std::map KindToString{
@@ -49,9 +51,41 @@
   }
 
   auto ActualTokens = getSemanticHighlightings(AST);
+  return {std::move(AST), ActualTokens, ExpectedTokens};
+}
+
+void checkHighlightings(llvm::StringRef Code) {
+  std::vector ActualTokens;
+  std::vector ExpectedTokens;
+  std::tie(std::ignore, ActualTokens, ExpectedTokens) =
+  getHighlightingsAnnotated(Code);
   EXPECT_THAT(ActualTokens, testing::UnorderedElementsAreArray(ExpectedTokens));
 }
 
+void checkDiffedHighlights(const std::vector &ExpectedTokens,
+   const std::vector &EmptyLines,
+   std::vector &ActualDiffed) {
+  std::map> ExpectedLines;
+  for (const HighlightingToken &Token : ExpectedTokens)
+ExpectedLines[Token.R.start.line].push_back(Token);
+  std::vector ExpectedLinePairHighlighting;
+  for (int Line : EmptyLines)
+ExpectedLinePairHighlighting.push_back({Line, {}});
+  for (auto &LineTokens : ExpectedLines) {
+llvm::sort(LineTokens.second);
+ExpectedLinePairHighlighting.push_back(
+{LineTokens.first, LineTokens.second});
+  }
+
+  // The UnorderedElementsAreArray only checks that the top level vector
+  // is unordered. The vectors in the pair must be in the correct order.
+  for (unsigned I = 0, End = ActualDiffed.size(); I < End; ++I)
+llvm::sort(ActualDiffed[I].Tokens);
+
+  EXPECT_THAT(ActualDiffed,
+  testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting));
+}
+
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
 R"cpp(
@@ -211,21 +245,103 @@
 return Pos;
   };
 
-  std::vector Tokens{
-  {HighlightingKind::Variable,
-Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
-  {HighlightingKind::Function,
-Range{CreatePosition(3, 4), CreatePosition(3, 7)}},
-  {HighlightingKind::Variable,
-Range{CreatePosition(1, 1), CreatePosition(1, 5)}}};
+  std::vector Tokens{
+  {3,
+   {{HighlightingKind::Variable,
+ Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+{HighlightingKind::Function,
+ Range{CreatePosition(3, 4), CreatePosition(3, 7),
+  {1,
+   {{HighlightingKind::Variable,
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
-  {1, "AQAEAAA="},
-  {3, "CAAEAAAEAAMAAQ=="}};
+  {3, "CAAEAAAEAAMAAQ=="}, {1, "AQAEAAA="}};
   EXPECT_EQ(ActualResults, ExpectedResults);
 }
 
+TEST(SemanticHighlighting, HighlightingDiffer) {
+  std::vector<
+  std::pair, std::pair>>
+  TestCases{{{},
+ {
+ R"cpp(
+int $Variable[[A]]
+double $Variable[[B]];
+struct $Class[[C]] {};
+  )cpp",
+ R"cpp(
+int A;
+double B;
+struct C {};
+  )cpp"}},
+{{5},
+ {
+ R"cpp(
+  struct $Class[[Alpha]] {
+double SomeVariable = 9483.301;
+  };
+  struct $Class[[Beta]] {};
+  int $Variable[[A]] = 121;
+  $Class[[Alpha]] $Variable[[Var]];
+  )cpp",
+ R"cpp(
+  struct Alpha {
+double SomeVariable = 9483.301;
+  };
+  struct Beta   {}; // Some comment
+  intA = 121;
+  $Class[[Beta]] $Variable[[Var]];
+  )cpp"}},
+{{},
+ {
+ R"cpp(
+  int $Variable[[A]] = 121; int $Variable[[B]];
+)cpp",
+ R"cpp(
+  intA = 121; int $Variable[[B]];
+)cpp"}},
+{{},
+ {
+

[PATCH] D64569: [OpenCL] Improve destructor support in C++ for OpenCL

2019-07-16 Thread Marco Antognini via Phabricator via cfe-commits
mantognini updated this revision to Diff 210080.
mantognini added a comment.

- Refactored common bits from CheckConstructorDeclarator and 
CheckDestructorDeclarator.
- Added as many "ThisTy" parameter I could.
- Addressed issue with identifier format in tests (%N to %var.ascast).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64569

Files:
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl
  clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl

Index: clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s --check-prefix=CHECK-DEFINITIONS
+
+// This test ensures the proper address spaces and address space cast are used
+// for constructors, member functions and destructors.
+// See also atexit.cl and global_init.cl for other specific tests.
+
+// CHECK: %struct.MyType = type { i32 }
+struct MyType {
+  MyType(int i) : i(i) {}
+  MyType(int i) __constant : i(i) {}
+  ~MyType() {}
+  ~MyType() __constant {}
+  int bar() { return i + 2; }
+  int bar() __constant { return i + 1; }
+  int i;
+};
+
+// CHECK: @const1 = addrspace(2) global %struct.MyType zeroinitializer
+__constant MyType const1 = 1;
+// CHECK: @const2 = addrspace(2) global %struct.MyType zeroinitializer
+__constant MyType const2(2);
+// CHECK: @glob = addrspace(1) global %struct.MyType zeroinitializer
+MyType glob(1);
+
+// CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const1, i32 1)
+// CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const2, i32 2)
+// CHECK: call void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* addrspacecast (%struct.MyType addrspace(1)* @glob to %struct.MyType addrspace(4)*), i32 1)
+
+// CHECK-LABEL: define spir_kernel void @fooGlobal()
+kernel void fooGlobal() {
+  // CHECK: call i32 @_ZNU3AS46MyType3barEv(%struct.MyType addrspace(4)* addrspacecast (%struct.MyType addrspace(1)* @glob to %struct.MyType addrspace(4)*))
+  glob.bar();
+  // CHECK: call i32 @_ZNU3AS26MyType3barEv(%struct.MyType addrspace(2)* @const1)
+  const1.bar();
+  // CHECK: call void @_ZNU3AS26MyTypeD1Ev(%struct.MyType addrspace(2)* @const1)
+  const1.~MyType();
+}
+
+// CHECK-LABEL: define spir_kernel void @fooLocal()
+kernel void fooLocal() {
+  // CHECK: [[VAR:%.*]] = alloca %struct.MyType
+  // CHECK: [[REG:%.*]] = addrspacecast %struct.MyType* [[VAR]] to %struct.MyType addrspace(4)*
+  // CHECK: call void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* [[REG]], i32 3)
+  MyType myLocal(3);
+  // CHECK: [[REG:%.*]] = addrspacecast %struct.MyType* [[VAR]] to %struct.MyType addrspace(4)*
+  // CHECK: call i32 @_ZNU3AS46MyType3barEv(%struct.MyType addrspace(4)* [[REG]])
+  myLocal.bar();
+  // CHECK: [[REG:%.*]] = addrspacecast %struct.MyType* [[VAR]] to %struct.MyType addrspace(4)*
+  // CHECK: call void @_ZNU3AS46MyTypeD1Ev(%struct.MyType addrspace(4)* [[REG]])
+}
+
+// Ensure all members are defined for all the required address spaces.
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* %this, i32 %i)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* %this, i32 %i)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS26MyTypeD1Ev(%struct.MyType addrspace(2)* %this)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr void @_ZNU3AS46MyTypeD1Ev(%struct.MyType addrspace(4)* %this)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr i32 @_ZNU3AS26MyType3barEv(%struct.MyType addrspace(2)* %this)
+// CHECK-DEFINITIONS-DAG: define linkonce_odr i32 @_ZNU3AS46MyType3barEv(%struct.MyType addrspace(4)* %this)
Index: clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl
===
--- clang/test/CodeGenOpenCLCXX/addrspace-ctor.cl
+++ /dev/null
@@ -1,14 +0,0 @@
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
-
-struct MyType {
-  MyType(int i) : i(i) {}
-  MyType(int i) __constant : i(i) {}
-  int i;
-};
-
-//CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const1, i32 1)
-__constant MyType const1 = 1;
-//CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const2, i32 2)
-__constant MyType const2(2);
-//CHECK: call void @_ZNU3A

[clang-tools-extra] r366207 - [clangd] Added highlighting for the targets in typedefs and using.

2019-07-16 Thread Johan Vikstrom via cfe-commits
Author: jvikstrom
Date: Tue Jul 16 06:23:12 2019
New Revision: 366207

URL: http://llvm.org/viewvc/llvm-project?rev=366207&view=rev
Log:
[clangd] Added highlighting for the targets in typedefs and using.

Summary:
In `typedef int A` the `A` was not highlighted previously.

This patch gives `A` the same kind of highlighting that the underlying type has 
(class/enum) (which in this example is no special highlighting because builtins 
are not handled yet)
Will add highlightings for built ins in another patch.

Reviewers: hokein, sammccall, ilya-biryukov

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

Tags: #clang

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

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=366207&r1=366206&r2=366207&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Tue Jul 16 06:23:12 
2019
@@ -93,6 +93,12 @@ public:
 return true;
   }
 
+  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
+if(const auto *TSI = TD->getTypeSourceInfo())
+  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+return true;
+  }
+
   bool VisitTypeLoc(TypeLoc &TL) {
 // This check is for not getting two entries when there are anonymous
 // structs. It also makes us not highlight certain namespace qualifiers
@@ -101,9 +107,7 @@ public:
 if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
   return true;
 
-if (const Type *TP = TL.getTypePtr())
-  if (const TagDecl *TD = TP->getAsTagDecl())
-addToken(TL.getBeginLoc(), TD);
+addTypeLoc(TL.getBeginLoc(), TL);
 return true;
   }
 
@@ -118,6 +122,12 @@ public:
   }
 
 private:
+  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
+if (const Type *TP = TL.getTypePtr())
+  if (const TagDecl *TD = TP->getAsTagDecl())
+addToken(Loc, TD);
+  }
+
   void addToken(SourceLocation Loc, const NamedDecl *D) {
 if (D->getDeclName().isIdentifier() && D->getName().empty())
   // Don't add symbols that don't have any length.

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=366207&r1=366206&r2=366207&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Tue 
Jul 16 06:23:12 2019
@@ -90,7 +90,7 @@ TEST(SemanticHighlighting, GetsCorrectTo
 typename T::A* $Field[[D]];
   };
   $Namespace[[abc]]::$Class[[A]] $Variable[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] AAA;
+  typedef $Namespace[[abc]]::$Class[[A]] $Class[[AAA]];
   struct $Class[[B]] {
 $Class[[B]]();
 ~$Class[[B]]();
@@ -173,6 +173,19 @@ TEST(SemanticHighlighting, GetsCorrectTo
   }
   int $Variable[[B]];
   $Class[[AA]] $Variable[[A]]{$Variable[[B]]};
+)cpp",
+R"cpp(
+  namespace $Namespace[[a]] {
+struct $Class[[A]] {};
+  }
+  typedef $Namespace[[a]]::$Class[[A]] $Class[[B]];
+  using $Class[[BB]] = $Namespace[[a]]::$Class[[A]];
+  enum class $Enum[[E]] {};
+  typedef $Enum[[E]] $Enum[[C]];
+  typedef $Enum[[C]] $Enum[[CC]];
+  using $Enum[[CD]] = $Enum[[CC]];
+  $Enum[[CC]] $Function[[f]]($Class[[B]]);
+  $Enum[[CD]] $Function[[f]]($Class[[BB]]);
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);


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


[PATCH] D64754: [clangd] Added highlighting for the targets in typedefs.

2019-07-16 Thread Johan Vikström via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366207: [clangd] Added highlighting for the targets in 
typedefs and using. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64754?vs=210053&id=210084#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64754

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
@@ -93,6 +93,12 @@
 return true;
   }
 
+  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
+if(const auto *TSI = TD->getTypeSourceInfo())
+  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+return true;
+  }
+
   bool VisitTypeLoc(TypeLoc &TL) {
 // This check is for not getting two entries when there are anonymous
 // structs. It also makes us not highlight certain namespace qualifiers
@@ -101,9 +107,7 @@
 if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
   return true;
 
-if (const Type *TP = TL.getTypePtr())
-  if (const TagDecl *TD = TP->getAsTagDecl())
-addToken(TL.getBeginLoc(), TD);
+addTypeLoc(TL.getBeginLoc(), TL);
 return true;
   }
 
@@ -118,6 +122,12 @@
   }
 
 private:
+  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
+if (const Type *TP = TL.getTypePtr())
+  if (const TagDecl *TD = TP->getAsTagDecl())
+addToken(Loc, TD);
+  }
+
   void addToken(SourceLocation Loc, const NamedDecl *D) {
 if (D->getDeclName().isIdentifier() && D->getName().empty())
   // Don't add symbols that don't have any length.
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
@@ -90,7 +90,7 @@
 typename T::A* $Field[[D]];
   };
   $Namespace[[abc]]::$Class[[A]] $Variable[[AA]];
-  typedef $Namespace[[abc]]::$Class[[A]] AAA;
+  typedef $Namespace[[abc]]::$Class[[A]] $Class[[AAA]];
   struct $Class[[B]] {
 $Class[[B]]();
 ~$Class[[B]]();
@@ -173,6 +173,19 @@
   }
   int $Variable[[B]];
   $Class[[AA]] $Variable[[A]]{$Variable[[B]]};
+)cpp",
+R"cpp(
+  namespace $Namespace[[a]] {
+struct $Class[[A]] {};
+  }
+  typedef $Namespace[[a]]::$Class[[A]] $Class[[B]];
+  using $Class[[BB]] = $Namespace[[a]]::$Class[[A]];
+  enum class $Enum[[E]] {};
+  typedef $Enum[[E]] $Enum[[C]];
+  typedef $Enum[[C]] $Enum[[CC]];
+  using $Enum[[CD]] = $Enum[[CC]];
+  $Enum[[CC]] $Function[[f]]($Class[[B]]);
+  $Enum[[CD]] $Function[[f]]($Class[[BB]]);
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);


Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -93,6 +93,12 @@
 return true;
   }
 
+  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
+if(const auto *TSI = TD->getTypeSourceInfo())
+  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+return true;
+  }
+
   bool VisitTypeLoc(TypeLoc &TL) {
 // This check is for not getting two entries when there are anonymous
 // structs. It also makes us not highlight certain namespace qualifiers
@@ -101,9 +107,7 @@
 if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
   return true;
 
-if (const Type *TP = TL.getTypePtr())
-  if (const TagDecl *TD = TP->getAsTagDecl())
-addToken(TL.getBeginLoc(), TD);
+addTypeLoc(TL.getBeginLoc(), TL);
 return true;
   }
 
@@ -118,6 +122,12 @@
   }
 
 private:
+  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
+if (const Type *TP = TL.getTypePtr())
+  if (const TagDecl *TD = TP->getAsTagDecl())
+addToken(Loc, TD);
+  }
+
   void addToken(SourceLocation Loc, const NamedDecl *D) {
 if (D->getDeclName().isIdentifier() && D->getName().empty())
   // Don't add symbols that don't have any length.
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
@@ -90,7 +90,7 @@
 typename 

[PATCH] D64569: [OpenCL] Improve destructor support in C++ for OpenCL

2019-07-16 Thread Marco Antognini via Phabricator via cfe-commits
mantognini marked 10 inline comments as done.
mantognini added a comment.

Mind the fact that I've rebased the changes onto a more recent master version. 
If you look at the diff of v1 against v2 you might see some unrelated changes.

Let me know if there's anything else that I need to change.




Comment at: clang/lib/CodeGen/CGClass.cpp:2016
   CGF.EmitCXXDestructorCall(dtor, Dtor_Complete, /*for vbase*/ false,
-/*Delegating=*/false, addr);
+/*Delegating=*/false, addr, type);
 }

rjmccall wrote:
> mantognini wrote:
> > This is the only place where the new parameter has an interesting value. In 
> > the ~10 other calls to `EmitCXXDestructorCall` overloads, the default value 
> > of this new parameter is used instead.
> Arguments that are potentially required for correctness — as opposed to just 
> enabling some optimization — should generally not have defaults.  I think you 
> should remove these defaults and require a sensible type to be passed down in 
> all cases.
I've addressed that. I had to add some extra parameter/attributes here and 
there. Please let me know if anything is can be improved.



Comment at: clang/lib/CodeGen/CGClass.cpp:1447
+if (HaveInsertPoint()) {
+  // FIXME: Determine the type of the object being destroyed.
+  QualType ThisTy;

I'm not familiar enough with CXXCtorType and such, and would welcome some help 
for these two FIXMEs.



Comment at: clang/lib/CodeGen/CGExprCXX.cpp:103
+  // we ensure a cast is added where necessary.
+  if (ThisTy.isNull()) {
+#ifndef NDEBUG

Despite no longer having a default parameter, not all call site can provide a 
meaningful value ATM. That is why this check is still required.



Comment at: clang/lib/CodeGen/CGExprCXX.cpp:96
+llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE,
+QualType ThisTy) {
+  const CXXMethodDecl *DtorDecl = cast(Dtor.getDecl());

mantognini wrote:
> This new parameter is required in order to call `performAddrSpaceCast`.
Now that it's no longer a default parameter. I've group ThisTy with This.



Comment at: clang/lib/CodeGen/CGExprCXX.cpp:117-118
+  llvm::Type *NewType = CGM.getTypes().ConvertType(DstTy);
+  This = getTargetHooks().performAddrSpaceCast(*this, This, SrcAS, DstAS,
+   NewType);
+}

rjmccall wrote:
> Anastasia wrote:
> > mantognini wrote:
> > > This is effectively the fix for the third point mentioned in the 
> > > description.
> > > 
> > > Alternatively, `This = Builder.CreatePointerBitCastOrAddrSpaceCast(This, 
> > > NewType);` seems to work equally well and does not require the extra new 
> > > parameter.
> > Yes, I agree just using `CreatePointerBitCastOrAddrSpaceCast` would be 
> > easier.
> > 
> > As far as I remember (@rjmccall can correct me if I am wrong) 
> > `performAddrSpaceCast` was added to workaround the fact that `nullptr` 
> > constant might not be value 0 for some address spaces. However, considering 
> > that it's not the first place where some big change has to be done in 
> > CodeGen to be able to use the new API I am wondering if it's worth looking 
> > at moving this logic to LLVM lowering phases that can map `nullptr` 
> > constant into some arbitrary value. I think the challenge is to find where 
> > LLVM assumes that `nullptr` is always 0. That might not be an easy task.
> > 
> > PS, I am not suggesting to do it for this patch but just an idea to 
> > investigate in the future. 
> I continue to think that it makes sense to set Clang up to be able to handle 
> language-level address spaces that don't exist at the level of LLVM IR.

Alright, I've kept it.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8428-8439
+bool DiagOccured = false;
 FTI.MethodQualifiers->forEachQualifier(
 [&](DeclSpec::TQ TypeQual, StringRef QualName, SourceLocation SL) {
+  // This diagnostic should be emitted on any qualifier except an addr
+  // space qualifier. However, forEachQualifier currently doesn't visit
+  // addr space qualifiers, so there's no way to write this condition
+  // right now; we just diagnose on everything.

rjmccall wrote:
> mantognini wrote:
> > This is the fix for the first point in the description.
> Can we unify this with the similar check we do elsewhere?
Done.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8197
+  const DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
+  if (FTI.hasMethodTypeQualifiers() && !D.isInvalidType()) {
+bool DiagOccured = false;

I've refactored that bit out of the two mentioned functions. Mind the fact that 
it now always check `!D.isInvalidType()`. I think it makes sense, but let me 
know if that's not the case

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

2019-07-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: include/clang/AST/ASTImporter.h:317
+std::shared_ptr SharedState = nullptr,
+ASTUnit *Unit = nullptr);
 

balazske wrote:
> balazske wrote:
> > martong wrote:
> > > What if we provided an additional constructor where we take over the 
> > > ASTUnits instead of the ASTContexts?
> > > Then we would not need to pass the FileManagers neither.
> > > ```
> > > ASTImporter(ASTUnit &ToUnit, 
> > > ASTUnit &FromUnit,
> > > bool MinimalImport,
> > > std::shared_ptr SharedState = 
> > > nullptr,
> > > ```
> > Is the `SharedState==nullptr` case only for LLDB? If yes then it is 
> > possible to have a "LLDB" constructor when no shared state and no ASTUnit 
> > is needed. And another for CTU case when a From and To ASTUnit is specified 
> > and a shared state (theoretically minimal can be true in any case but 
> > probably only true for LLDB?).
> It is not trivial to make and use this new kind of constructor (with ToUnit), 
> there is no ToUnit in the CrossTU context. Is it OK to have the original 
> constructor, or one with `FromUnit` (but `ToContext` and `ToFileManager`)?
> Is the SharedState==nullptr case only for LLDB?
Yes.

> Is it OK to have the original constructor, or one with FromUnit (but 
> ToContext and ToFileManager)?

I think it is OK to have a new ctor with FromUnit (but ToContext and 
ToFileManager). Because ASTUnit is just a utility class for loading an 
ASTContext from an AST file. So, one of the "to" or the "from" context could be 
initialized by an ASTUnit.

In the future the API of the ASTImporter could be extended with such a ctor 
which takes two ASTUnits, also we could add the counterpart ctor where we have 
ToUnit, FromContext and FromFileManager as params.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64554



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


[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

2019-07-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: include/clang/AST/ASTImporter.h:317
+std::shared_ptr SharedState = nullptr,
+ASTUnit *Unit = nullptr);
 

martong wrote:
> balazske wrote:
> > balazske wrote:
> > > martong wrote:
> > > > What if we provided an additional constructor where we take over the 
> > > > ASTUnits instead of the ASTContexts?
> > > > Then we would not need to pass the FileManagers neither.
> > > > ```
> > > > ASTImporter(ASTUnit &ToUnit, 
> > > > ASTUnit &FromUnit,
> > > > bool MinimalImport,
> > > > std::shared_ptr SharedState = 
> > > > nullptr,
> > > > ```
> > > Is the `SharedState==nullptr` case only for LLDB? If yes then it is 
> > > possible to have a "LLDB" constructor when no shared state and no ASTUnit 
> > > is needed. And another for CTU case when a From and To ASTUnit is 
> > > specified and a shared state (theoretically minimal can be true in any 
> > > case but probably only true for LLDB?).
> > It is not trivial to make and use this new kind of constructor (with 
> > ToUnit), there is no ToUnit in the CrossTU context. Is it OK to have the 
> > original constructor, or one with `FromUnit` (but `ToContext` and 
> > `ToFileManager`)?
> > Is the SharedState==nullptr case only for LLDB?
> Yes.
> 
> > Is it OK to have the original constructor, or one with FromUnit (but 
> > ToContext and ToFileManager)?
> 
> I think it is OK to have a new ctor with FromUnit (but ToContext and 
> ToFileManager). Because ASTUnit is just a utility class for loading an 
> ASTContext from an AST file. So, one of the "to" or the "from" context could 
> be initialized by an ASTUnit.
> 
> In the future the API of the ASTImporter could be extended with such a ctor 
> which takes two ASTUnits, also we could add the counterpart ctor where we 
> have ToUnit, FromContext and FromFileManager as params.
Note that if we have a ctor which takes FromUnit as a param then we won't need 
the assertions which check whether the Unit provides the same context or not.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64554



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


[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

2019-07-16 Thread Alexander Potapenko via Phabricator via cfe-commits
glider added inline comments.



Comment at: llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:27
+static const char GlobalArgumentMemAccess[] = {
+// grep  -P -o "(?<=FUNCTION_INFO: ).*"  | sort -u >
+// ../llvm-project/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpData.h

vitalybuka wrote:
> glider wrote:
> > Ditto.
> These files should be empty. Raw diff 
> https://reviews.llvm.org/file/data/o6sk6gw2gqs4u4pmodrn/PHID-FILE-s6c6nsofxnqekkcvzdzs/D61879.diff
>  already contains them. 
> It's ThinLTO replacement experiments. They don't improve results enough, so I 
> didn't bother to create real ThinLTO stuff. Anyway it is not needed for full 
> LTO.
> 
```
$ ninja -j64 check-clang
...
/usr/local/google/src/llvm-git-monorepo/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:20:32:
 error: zero-size array ‘llvm::GUIDS_LOOKUP’
 static const GlobalValue::GUID GUIDS_LOOKUP[] = {
^~~~
/usr/local/google/src/llvm-git-monorepo/llvm/lib/Transforms/Scalar/DeadStoreEliminationExpGlobal.cpp:26:19:
 error: zero-size array ‘llvm::GlobalArgumentMemAccess’
 static const char GlobalArgumentMemAccess[] = {
   ^~~
```
Am I doing something wrong? Looks like empty files aren't enough.
I've fixed the problem by putting "0" into each file, but it's strange it works 
differently for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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


[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

2019-07-16 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment.

Ping.


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

https://reviews.llvm.org/D61466



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


[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Starting to look real good!




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:807
+  "",
+  Released>,
+  ]>,

We mark options that are not yet ready for used with `InAlpha`, rather then 
`Released`. I think it would be more fitting in this case!

Mind that if you'd like to list this option after that, you'd have to use
```lang=bash
clang -cc1 -analyzer-checker-option-help-alpha
```



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:58
+
+  /// The ``TaintConfiguration`` is used to parse configuration file.
+  struct TaintConfiguration {

Just simply `Used to parse the configuration file`.
https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:302-303
+  Result.push_back(InvalidArgIndex);
+  llvm::errs() << "Invalid arg number for propagation rules: " << Arg
+   << '\n';
+} else

Do we emit an error for this case?



Comment at: lib/StaticAnalyzer/Checkers/Yaml.h:1
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/ADT/APInt.h"

Header blurb (licence stuff etc)!



Comment at: test/Analysis/taint-generic.c:1-2
-// RUN: %clang_analyze_cc1  
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-Wno-format-security -verify %s
-// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT 
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-analyzer-config 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT 
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-analyzer-config 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 -Wno-format-security -verify %s
 

Could you please use line breaks here? I know it isn't your making, but if 
we're touching it anyways :^)

```
// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
// RUN:   -DFILE_IS_STRUCT \
// RUN:   -analyzer-checker=alpha.security.taint \
// RUN:   -analyzer-checker=core \
// RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
// RUN:   -analyzer-config \
// RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
```

And also a testcase for an incorrect checker option:
```

// RUN: not %clang_analyze_cc1 -verify %s \
// RUN:   -analyzer-checker=alpha.security.taint \
// RUN:   -analyzer-config \
// RUN: alpha.security.taint.TaintPropagation:Config=justguessit \
// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FILE

// CHECK-INVALID-FILE: (frontend): invalid input for checker option
// CHECK-INVALID-FILE-SAME:
'alpha.security.taint.TaintPropagation:Config',
// CHECK-INVALID-FILE-SAME:that expects a valid yaml file
```
something like that.


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

https://reviews.llvm.org/D59555



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


[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

2019-07-16 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton updated this revision to Diff 210095.
benhamilton marked an inline comment as done.
benhamilton added a comment.

- Rebase.
- Change to just avoid breaking when the left-hand side is a unary operator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2425,6 +2425,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2425,6 +2425,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

2019-07-16 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton marked an inline comment as done.
benhamilton added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2427-2428
 return 50;
 
+  if (Left.is(TT_UnaryOperator) && Right.is(TT_ObjCMethodExpr))
+return 60;

sammccall wrote:
> This looks a little suspicious only because it's so specific.
> 
> It seems like you'd *always* want a harsh penalty for breaking between a 
> unary operator and its operand. @klimek does this look right, should we drop 
> the `ObcJMethodExpr` requirement or is this case handled elsewhere?
I tried changing it to just `if (Left.is(TT_UnaryOperator))` and all the tests 
passed. I'll go for the gusto and assume this is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775



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


r366211 - [OPENMP]Add support for analysis of if clauses.

2019-07-16 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Tue Jul 16 07:51:46 2019
New Revision: 366211

URL: http://llvm.org/viewvc/llvm-project?rev=366211&view=rev
Log:
[OPENMP]Add support for analysis of if clauses.

Summary:
Added support for analysis of if clauses in the OpenMP directives to be
able to check for the use of uninitialized variables.

Reviewers: NoQ

Subscribers: guansong, jfb, jdoerfert, caomhin, kkwli0, cfe-commits

Tags: clang

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

Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/test/Analysis/cfg-openmp.cpp
cfe/trunk/test/OpenMP/cancel_if_messages.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_if_messages.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_if_messages.cpp
cfe/trunk/test/OpenMP/parallel_for_if_messages.cpp
cfe/trunk/test/OpenMP/parallel_for_simd_if_messages.cpp
cfe/trunk/test/OpenMP/parallel_if_messages.cpp
cfe/trunk/test/OpenMP/parallel_sections_if_messages.cpp
cfe/trunk/test/OpenMP/target_data_if_messages.cpp
cfe/trunk/test/OpenMP/target_enter_data_if_messages.cpp
cfe/trunk/test/OpenMP/target_exit_data_if_messages.cpp
cfe/trunk/test/OpenMP/target_if_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_if_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_if_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_if_messages.cpp
cfe/trunk/test/OpenMP/target_simd_if_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_if_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_if_messages.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_if_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_if_messages.cpp
cfe/trunk/test/OpenMP/target_teams_if_messages.cpp
cfe/trunk/test/OpenMP/target_update_if_messages.cpp
cfe/trunk/test/OpenMP/task_if_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_if_messages.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_if_messages.cpp

Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=366211&r1=366210&r2=366211&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Tue Jul 16 07:51:46 2019
@@ -501,11 +501,10 @@ public:
 return const_child_range(&Condition, &Condition + 1);
   }
 
-  child_range used_children() {
-return child_range(child_iterator(), child_iterator());
-  }
+  child_range used_children();
   const_child_range used_children() const {
-return const_child_range(const_child_iterator(), const_child_iterator());
+auto Children = const_cast(this)->used_children();
+return const_child_range(Children.begin(), Children.end());
   }
 
   static bool classof(const OMPClause *T) {

Modified: cfe/trunk/lib/AST/OpenMPClause.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/OpenMPClause.cpp?rev=366211&r1=366210&r2=366211&view=diff
==
--- cfe/trunk/lib/AST/OpenMPClause.cpp (original)
+++ cfe/trunk/lib/AST/OpenMPClause.cpp Tue Jul 16 07:51:46 2019
@@ -209,6 +209,25 @@ const OMPClauseWithPostUpdate *OMPClause
   return nullptr;
 }
 
+/// Gets the address of the original, non-captured, expression used in the
+/// clause as the preinitializer.
+static Stmt **getAddrOfExprAsWritten(Stmt *S) {
+  if (!S)
+return nullptr;
+  if (auto *DS = dyn_cast(S)) {
+assert(DS->isSingleDecl() && "Only single expression must be captured.");
+if (auto *OED = dyn_cast(DS->getSingleDecl()))
+  return OED->getInitAddress();
+  }
+  return nullptr;
+}
+
+OMPClause::child_range OMPIfClause::used_children() {
+  if (Stmt **C = getAddrOfExprAsWritten(getPreInitStmt()))
+return child_range(C, C + 1);
+  return child_range(&Condition, &Condition + 1);
+}
+
 OMPOrderedClause *OMPOrderedClause::Create(const ASTContext &C, Expr *Num,
unsigned NumLoops,
SourceLocation StartLoc,

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=366211&r1=366210&r2=366211&view=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Tue Jul 16 07:51:46 2019
@@ -4746,8 +4746,9 @@ CFGBlock *CFGBuilder::VisitOMPExecutable
 
   // Reverse the elements to process them in natural order. Iterators are not
   // bidirectional, so we need to create temp vector.
-  for (Stmt *S : llvm::reverse(llvm::to_vector<8>(
-   OMPExecutableDirective::used_clauses_children(D->clauses() {
+  Smal

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

2019-07-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

the code looks clearer now!




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:609
 FixItsMap.erase(File);
+std::lock_guard HLock(HighlightingsMutex);
+FileToPrevHighlightings.erase(File);

nit: I would create a new `{}` block merely for the hightlightings.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1110
 PathRef File, std::vector Highlightings) {
+  llvm::ArrayRef Prev;
+  {

this seems unsafe, we get a reference of the map value, we might access it 
without the mutex being guarded. 

```
std::vector Old;
{
std::lock_guard Lock(HighlightingsMutex);
Old = std::move(FileToHighlightings[File]);
}
```



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1124
+std::lock_guard Lock(HighlightingsMutex);
+FileToPrevHighlightings[File] = Highlightings;
+  }

`FileToPrevHighlightings[File] = std::move(Highlightings);`



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:140
+  std::mutex HighlightingsMutex;
+  llvm::StringMap> FileToPrevHighlightings;
 

nit: I'd drop `Prev`.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:229
+// Get the highlights for \c Line where the first entry for \c Line is
+// immediately preceded by \c OldLine
+ArrayRef takeLine(ArrayRef AllTokens,

nit: I'm not sure I understand the comment.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:233
+ int Line) {
+  return ArrayRef(OldLine.end(), AllTokens.end())
+  .take_while([Line](const HighlightingToken &Token) {

nit: this implies that OldLine and AllTokens must ref to the same container, 
could you refine the `OldLine` as a start `Iterator`? 



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:255
+  // ArrayRefs to the current line in the highlights.
+  ArrayRef NewLine(Highlightings.data(),
+  Highlightings.data()),

IIUC, we are initializing an empty ArrayRef, if so, how about using 
`NewLine(Highlightings.data(), /*length*/0)`? `NewLine(Highlightings.data(), 
Highlightings.data())` is a bit weird, it took me a while to understand the 
purpose.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:257
+  Highlightings.data()),
+  OldLine = {PrevHighlightings.data(), PrevHighlightings.data()};
+  // ArrayRefs to the new and old highlighting vectors.

nit: split it into a new statement.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:259
+  // ArrayRefs to the new and old highlighting vectors.
+  ArrayRef Current = {Highlightings},
+  Prev = {PrevHighlightings};

we don't change Current and Prev below, how about re-using `Highlightings` and 
`PrevHighlightings`?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:71
+/// in \c Highlightings an empty line is added. Returns the resulting
+/// HighlightingTokens grouped by their line number. Assumes the highlightings
+/// are sorted by the tokens' ranges.

I believe `Assumes the highlightings are sorted by the tokens' ranges.` is a 
requirement for this function?

If so, mark it more explicitly in the comment, like 

```
///
/// REQUIRED: OldHighlightings and NewHighlightings are sorted.
```



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:74
+std::vector
+diffHighlightings(ArrayRef Highlightings,
+  ArrayRef PrevHighlightings);

nit: I'd use name the parameters symmetrically, `NewHighlightings` and 
`OldHighlightings`.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:32
 
-void checkHighlightings(llvm::StringRef Code) {
+std::tuple,
+   std::vector>

we don't need `ParsedAST`  now I think.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:265
+TEST(SemanticHighlighting, HighlightingDiffer) {
+  std::vector<
+  std::pair, std::pair>>

this is a complicated structure, could you add some comments describing the 
test strategy here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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


r366212 - [OpenCL] Fixing sampler initialisations for C++ mode.

2019-07-16 Thread Neil Hickey via cfe-commits
Author: neil.hickey
Date: Tue Jul 16 07:57:32 2019
New Revision: 366212

URL: http://llvm.org/viewvc/llvm-project?rev=366212&view=rev
Log:
[OpenCL] Fixing sampler initialisations for C++ mode.

Allow conversions between integer and sampler type.

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

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CodeGenOpenCL/sampler.cl

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=366212&r1=366211&r2=366212&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Jul 16 07:57:32 2019
@@ -5640,6 +5640,9 @@ void InitializationSequence::InitializeF
   bool allowObjCWritebackConversion = S.getLangOpts().ObjCAutoRefCount &&
  Entity.isParameterKind();
 
+  if (TryOCLSamplerInitialization(S, *this, DestType, Initializer))
+return;
+
   // We're at the end of the line for C: it's either a write-back conversion
   // or it's a C assignment. There's no need to check anything else.
   if (!S.getLangOpts().CPlusPlus) {
@@ -5649,9 +5652,6 @@ void InitializationSequence::InitializeF
   return;
 }
 
-if (TryOCLSamplerInitialization(S, *this, DestType, Initializer))
-  return;
-
 if (TryOCLZeroOpaqueTypeInitialization(S, *this, DestType, Initializer))
   return;
 

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=366212&r1=366211&r2=366212&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue Jul 16 07:57:32 2019
@@ -1851,6 +1851,10 @@ static bool IsStandardConversion(Sema &S
  (From->EvaluateKnownConstInt(S.getASTContext()) == 0)) {
 SCS.Second = ICK_Zero_Queue_Conversion;
 FromType = ToType;
+  } else if (ToType->isSamplerT() &&
+ From->isIntegerConstantExpr(S.getASTContext())) {
+SCS.Second = ICK_Compatible_Conversion;
+FromType = ToType;
   } else {
 // No second conversion required.
 SCS.Second = ICK_Identity;

Modified: cfe/trunk/test/CodeGenOpenCL/sampler.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/sampler.cl?rev=366212&r1=366211&r2=366212&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/sampler.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/sampler.cl Tue Jul 16 07:57:32 2019
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | 
FileCheck %s
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o 
- -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=c++ -emit-llvm -triple spir-unknown-unknown -o - 
-O0 | FileCheck %s
 //
 // This test covers 5 cases of sampler initialzation:
 //   1. function argument passing
@@ -29,7 +30,7 @@ const sampler_t glb_smp_const = CLK_ADDR
 int get_sampler_initializer(void);
 
 void fnc4smp(sampler_t s) {}
-// CHECK: define spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* %
+// CHECK: define spir_func void [[FUNCNAME:@.*fnc4smp.*]](%opencl.sampler_t 
addrspace(2)* %
 
 kernel void foo(sampler_t smp_par) {
   // CHECK-LABEL: define spir_kernel void @foo(%opencl.sampler_t addrspace(2)* 
%smp_par)
@@ -45,32 +46,32 @@ kernel void foo(sampler_t smp_par) {
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 19)
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, 
%opencl.sampler_t addrspace(2)** [[smp_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 
   // Case 1b
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 19)
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, 
%opencl.sampler_t addrspace(2)** [[smp_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 
   // Case 1a/2a
   fnc4smp(glb_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 
   // Case 1a/2c
   fnc4smp(glb_smp_const);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+  // CHECK: call spir_func void [[F

[PATCH] D64791: [OpenCL] Fix sampler initialization for C++ mode

2019-07-16 Thread Neil Hickey via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366212: [OpenCL] Fixing sampler initialisations for C++ 
mode. (authored by neil.hickey, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64791?vs=210061&id=210096#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64791

Files:
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/CodeGenOpenCL/sampler.cl

Index: cfe/trunk/test/CodeGenOpenCL/sampler.cl
===
--- cfe/trunk/test/CodeGenOpenCL/sampler.cl
+++ cfe/trunk/test/CodeGenOpenCL/sampler.cl
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=c++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
 //
 // This test covers 5 cases of sampler initialzation:
 //   1. function argument passing
@@ -29,7 +30,7 @@
 int get_sampler_initializer(void);
 
 void fnc4smp(sampler_t s) {}
-// CHECK: define spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* %
+// CHECK: define spir_func void [[FUNCNAME:@.*fnc4smp.*]](%opencl.sampler_t addrspace(2)* %
 
 kernel void foo(sampler_t smp_par) {
   // CHECK-LABEL: define spir_kernel void @foo(%opencl.sampler_t addrspace(2)* %smp_par)
@@ -45,32 +46,32 @@
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 19)
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1b
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 19)
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1a/2a
   fnc4smp(glb_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1a/2c
   fnc4smp(glb_smp_const);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // Case 1c
   fnc4smp(smp_par);
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[smp_par_ptr]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 5)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   const sampler_t const_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
   fnc4smp(const_smp);
@@ -78,12 +79,12 @@
   // CHECK: store %opencl.sampler_t addrspace(2)* [[CONST_SAMP]], %opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR:%[a-zA-Z0-9]+]]
   fnc4smp(const_smp);
   // CHECK: [[SAMP:%[0-9]+]] = load %opencl.sampler_t addrspace(2)*, %opencl.sampler_t addrspace(2)** [[CONST_SMP_PTR]]
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   constant sampler_t constant_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
   fnc4smp(constant_smp);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 35)
-  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+  // CHECK: call spir_func void [[FUNCNAME]](%opencl.sampler_t addrspace(2)* [[SAMP]])
 
   // TODO: enable sampler initialization with non-constant integer.
   //const sampler_t const_smp_func_init = get_sampler_initializer();
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOver

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

2019-07-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added reviewers: rnk, sammccall.
Herald added a subscriber: kadircet.
Herald added a project: clang.

Instead of asserting all typos are corrected in the sema destructor.

The sema destructor is not run in the common case of running the compiler
with the -disable-free cc1 flag (which is the default in the driver).

Having this assertion led to crashes in libclang and clangd, which are not
reproducible when running the compiler.

Asserting at the end of the TU could be an option, but finding all
missing typo correction cases is hard and having worse diagnostics instead
of a failing assertion is a better trade-off.

For more discussion on this, see:
https://lists.llvm.org/pipermail/cfe-dev/2019-July/062872.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that point, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that point, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
___
cfe-commits mailing list
cfe-commits@li

[PATCH] D64765: [OPENMP]Add support for analysis of firstprivate variables.

2019-07-16 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev updated this revision to Diff 210100.
ABataev added a comment.

Rebase


Repository:
  rC Clang

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

https://reviews.llvm.org/D64765

Files:
  include/clang/AST/OpenMPClause.h
  test/Analysis/cfg-openmp.cpp
  test/OpenMP/distribute_parallel_for_firstprivate_messages.cpp
  test/OpenMP/distribute_parallel_for_simd_firstprivate_messages.cpp
  test/OpenMP/parallel_firstprivate_messages.cpp
  test/OpenMP/parallel_for_firstprivate_messages.cpp
  test/OpenMP/parallel_for_simd_firstprivate_messages.cpp
  test/OpenMP/parallel_sections_firstprivate_messages.cpp
  test/OpenMP/target_firstprivate_messages.cpp
  test/OpenMP/target_parallel_firstprivate_messages.cpp
  test/OpenMP/target_parallel_for_firstprivate_messages.cpp
  test/OpenMP/target_parallel_for_simd_firstprivate_messages.cpp
  test/OpenMP/target_simd_firstprivate_messages.cpp
  test/OpenMP/target_teams_distribute_firstprivate_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_firstprivate_messages.cpp
  
test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_messages.cpp
  test/OpenMP/target_teams_distribute_simd_firstprivate_messages.cpp
  test/OpenMP/target_teams_firstprivate_messages.cpp
  test/OpenMP/task_firstprivate_messages.cpp
  test/OpenMP/taskloop_firstprivate_messages.cpp
  test/OpenMP/taskloop_simd_firstprivate_messages.cpp
  test/OpenMP/teams_distribute_firstprivate_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
  test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
  test/OpenMP/teams_firstprivate_messages.cpp

Index: test/OpenMP/teams_firstprivate_messages.cpp
===
--- test/OpenMP/teams_firstprivate_messages.cpp
+++ test/OpenMP/teams_firstprivate_messages.cpp
@@ -10,6 +10,14 @@
   return argc;
 }
 
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp target
+#pragma omp teams firstprivate(fp) // expected-warning {{variable 'fp' is uninitialized when used here}}
+  for (int i = 0; i < 10; ++i)
+;
+}
+
 struct S1; // expected-note {{declared here}} expected-note{{forward declaration of 'S1'}}
 extern S1 a;
 class S2 {
Index: test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
===
--- test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
+++ test/OpenMP/teams_distribute_simd_firstprivate_messages.cpp
@@ -10,6 +10,14 @@
   return argc;
 }
 
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp target
+#pragma omp teams distribute simd firstprivate(fp) // expected-warning {{variable 'fp' is uninitialized when used here}}
+  for (int i = 0; i < 10; ++i)
+;
+}
+
 struct S1; // expected-note {{declared here}} expected-note{{forward declaration of 'S1'}}
 extern S1 a;
 class S2 {
Index: test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_messages.cpp
@@ -10,6 +10,14 @@
   return argc;
 }
 
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp target
+#pragma omp teams distribute parallel for simd firstprivate(fp) // expected-warning {{variable 'fp' is uninitialized when used here}}
+  for (int i = 0; i < 10; ++i)
+;
+}
+
 struct S1; // expected-note {{declared here}} expected-note{{forward declaration of 'S1'}}
 extern S1 a;
 class S2 {
Index: test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_firstprivate_messages.cpp
@@ -10,6 +10,14 @@
   return argc;
 }
 
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp target
+#pragma omp teams distribute parallel for firstprivate(fp) // expected-warning {{variable 'fp' is uninitialized when used here}}
+  for (int i = 0; i < 10; ++i)
+;
+}
+
 struct S1; // expected-note {{declared here}} expected-note{{forward declaration of 'S1'}}
 extern S1 a;
 class S2 {
Index: test/OpenMP/teams_distribute_firstprivate_messages.cpp
===
--- test/OpenMP/teams_distribute_firstprivate_messages.cpp
+++ test/OpenMP/teams_distribute_firstprivate_messages.cpp
@@ -10,6 +10,14 @@
   return argc;
 }
 
+void xxx(int argc) {
+  int fp; // expected-note {{initialize the variable 'fp' to silence this warning}}
+#pragma omp ta

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

2019-07-16 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 210101.

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 +2397,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 Clang9
+C++ mode is available for OpenCL (see :ref:`C++ for OpenCL `).
 
 OpenCL Specific Options
 ---
@@ -2756,6 +2757,45 @@
   enqueue query functions from `section 6.13.17.5
   `_.
 
+.. _opencl_cpp:
+
+C++ for OpenCL
+--
+
+Starting from Clang9 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.
+
+There are only a few restrictions on allowed C++ features. For detailed information
+please refer to documentation on Extensions (: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 the new mode.
+
+To enable the new mode pass the following command line option when compiling ``.cl``
+file ``-cl-std=c++`` or ``-std=c++``.
+
+   .. 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=c++ test.cl
+
 .. _target_features:
 
 Target-Specific Features and Limitations
Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -1516,6 +1516,269 @@
 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. Regular C++
+features can be used in OpenCL kernel code. All 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
+- ``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 yet, Clang extends existing concept
+from C and OpenCL. For example conversion rules are extended from qualification
+conversion but the compatibility is determined using sets and overlapping from
+Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
+implicit conversions are allowed from named to ``__generic`` but not vice versa
+(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
+rules are built on top of this behavior.
+
+**Casts**
+
+C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
+permit implicit conversion to ``__generic``. However converting from named
+address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
+that conversions between ``__constant`` and any other is still disallowed.
+
+.. _opencl_cpp_addrsp_deduction:
+
+**Deduction**
+
+Address spaces are not deduced for:
+
+- non-pointer/non-reference template parameters or any dependent types except
+  for template specializations.
+- non-pointer/non-reference class members except for static data members that are
+  deduced to ``__global`` address space.
+- non-pointer/non-reference alias declarations.
+- ``decltype`` expression.
+
+.. code-block:: c++
+
+  template 
+  void foo() {
+T m; // address

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

2019-07-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 210105.
ilya-biryukov added a comment.

Fix a typo (xD)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/SemaObjC/typo-correction-subscript.m


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 
'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the


Index: clang/test/SemaObjC/typo-correction-subscript.m
===
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
 - (void)rdar47403222:(Dictionary *)opts {
   [self undeclaredMethod:undeclaredArg];
   // expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+  // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
   opts[(__bridge id)undeclaredKey] = 0;
   // expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
   // Detach from the PP callback handler which outlives Sema since it's owned
   // by the preprocessor.
   SemaPPCallbackHandler->reset();
-
-  assert(DelayedTypos.empty() && "Uncorrected typos!");
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
   assert(LateParsedInstantiations.empty() &&
  "end of TU template instantiation should not create more "
  "late-parsed templates");
+
+  // Report diagnostics for uncorrected delayed typos. Ideally all of them
+  // should have been corrected by that time, but it is very hard to cover all
+  // cases in practice.
+  for (const auto &Typo : DelayedTypos) {
+// We pass an empty TypoCorrection to indicate no correction was performed.
+Typo.second.DiagHandler(TypoCorrection());
+  }
+  DelayedTypos.clear();
 }
 
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64801: [analyzer] Add CTU user docs

2019-07-16 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: dkrupp, a_sidorin, Szelethus, NoQ.
Herald added subscribers: cfe-commits, Charusso, gamesh411, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, 
whisperity.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64801

Files:
  clang/docs/analyzer/user-docs.rst
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst

Index: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
===
--- /dev/null
+++ clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -0,0 +1,147 @@
+=
+Cross Translation Unit (CTU) Analysis
+=
+
+Normally, static analysis works in the boundary of one translation unit (TU).
+However, with additional steps and configuration we can enable the analysis to inline the definition of a function from another TU.
+
+.. contents::
+   :local:
+
+Manual CTU Analysis
+---
+
+Let's consider these source files in our minimal example:
+
+.. code-block:: cpp
+
+  // main.cpp
+  int foo();
+
+  int main() {
+return 3 / foo();
+  }
+
+.. code-block:: cpp
+
+  // foo.cpp
+  int foo() {
+return 0;
+  }
+
+And a compilation database:
+
+.. code-block:: bash
+
+  [
+{
+  "directory": "/path/to/your/project",
+  "command": "clang++ -c foo.cpp -o foo.o",
+  "file": "foo.cpp"
+},
+{
+  "directory": "/path/to/your/project",
+  "command": "clang++ -c main.cpp -o main.o",
+  "file": "main.cpp"
+}
+  ]
+
+We'd like to analyze `main.cpp` and discover the division by zero bug.
+In order to be able to inline the definition of `foo` from `foo.cpp` first we have to generate the `AST` (or `PCH`) file of `foo.cpp`:
+
+.. code-block:: bash
+
+  $ pwd $ /path/to/your/project
+  $ clang++ -emit-ast -o foo.cpp.ast foo.cpp
+  $ # Check that the .ast file is generated:
+  $ ls
+  compile_commands.json  foo.cpp.ast  foo.cpp  main.cpp
+  $
+
+The next step is to create a CTU index file which holds the `USR` name and location of external definitions in the source files:
+
+.. code-block:: bash
+
+  $ clang-extdef-mapping -p . foo.cpp
+  c:@F@foo# /path/to/your/project/foo.cpp
+  $ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
+
+We have to modify `externalDefMap.txt` to contain the name of the `.ast` files instead of the source files:
+
+.. code-block:: bash
+
+  $ sed -i -e "s/.cpp/.cpp.ast/g" externalDefMap.txt
+
+We still have to further modify the `externalDefMap.txt` file to contain relative paths:
+
+.. code-block:: bash
+
+  $ sed -i -e "s|$(pwd)/||g" externalDefMap.txt
+
+Now everything is available for the CTU analysis.
+We have to feed Clang with CTU specific extra arguments:
+
+.. code-block:: bash
+
+  $ pwd
+  /path/to/your/project
+  $ clang++ --analyze -Xclang -analyzer-config -Xclang experimental-enable-naive-ctu-analysis=true -Xclang -analyzer-config -Xclang ctu-dir=. -Xclang -analyzer-output=plist-multi-file main.cpp
+  main.cpp:5:12: warning: Division by zero
+return 3 / foo();
+   ~~^~~
+  1 warning generated.
+  $ # The plist file with the result is generated.
+  $ ls
+  compile_commands.json  externalDefMap.txt  foo.ast  foo.cpp  foo.cpp.ast  main.cpp  main.plist
+  $
+
+This manual procedure is boring and error-prone, so sooner or later we'd like to have a script which automates this for us.
+
+Automated CTU Analysis with CodeChecker
+---
+The `CodeChecker `_ project fully supports automated CTU analysis with Clang.
+Once we have set up the `PATH` environment variable and we activated the python `venv` then it is all it takes:
+
+.. code-block:: bash
+
+  $ CodeChecker analyze --ctu compile_commands.json -o reports
+  [INFO 2019-07-16 17:21] - Pre-analysis started.
+  [INFO 2019-07-16 17:21] - Collecting data for ctu analysis.
+  [INFO 2019-07-16 17:21] - [1/2] foo.cpp
+  [INFO 2019-07-16 17:21] - [2/2] main.cpp
+  [INFO 2019-07-16 17:21] - Pre-analysis finished.
+  [INFO 2019-07-16 17:21] - Starting static analysis ...
+  [INFO 2019-07-16 17:21] - [1/2] clangsa analyzed foo.cpp successfully.
+  [INFO 2019-07-16 17:21] - [2/2] clangsa analyzed main.cpp successfully.
+  [INFO 2019-07-16 17:21] -  Summary 
+  [INFO 2019-07-16 17:21] - Successfully analyzed
+  [INFO 2019-07-16 17:21] -   clangsa: 2
+  [INFO 2019-07-16 17:21] - Total analyzed compilation commands: 2
+  [INFO 2019-07-16 17:21] - =
+  [INFO 2019-07-16 17:21] - Analysis finished.
+  [INFO 2019-07-16 17:21] - To view results in the terminal use the "CodeChecker parse" command.
+  [INFO 2019-07-16 17:21] - To store results use the "CodeChecker store" command.
+  [INFO 2019-07-16 17:21] - See --help and the user guide for further options about parsing and storing the reports.
+

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

2019-07-16 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

In D61634#1586047 , @tejohnson wrote:

> Checking in to see where we are on this issue. I haven't had any time to work 
> on 4 but hopefully can start on that soon. But it needs the first part done 
> to be effective.


Thx for the heads up @tejohnson.
The patch is missing a bit of documentation but it shouldn't be too far from 
complete:

- it adds a clang function attribute, e.g. 
`__attribute__((no_builtin("memcpy")))`
- instructs clang to compose individual IR attributes from the clang attribute 
above, e.g. `no-builtin-memcpy`, `no-builtin-memset`, `no-builtin-sqrt`...
- adds a specific builtin to clang for the memcpy case `__builtin_memcpy_inline`
- adds an LLVM IR intrinsic `int_memcpy_inline`
- adds an LLVM Instruction `MemCpyInlineInst`
- instructs LLVM to forward the `no-builtin-memcpy` IR attribute from the 
function declaration to the actual memcpy calls inside the function's body 
(same for `memset` and `memmove`)
- adds code to turn the `MemCpyInlineInst` into code, using `DAG.getMemcpy` 
with `always_inline` set.

Left to do:

- finish implementing memset / memmove
- check attributes and reject the one that are not implemented
- add documentation

There's too many things going on in this patch and it may worth splitting it.


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] D64804: [OpenCL][Sema] Minor refactoring and constraint checking

2019-07-16 Thread Marco Antognini via Phabricator via cfe-commits
mantognini created this revision.
Herald added subscribers: cfe-commits, Anastasia, yaxunl.
Herald added a project: clang.

Simplify code a bit and add assertion to address post-landing comments
from D64083 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64804

Files:
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4216,17 +4216,12 @@
 break;
 
   case ICK_Block_Pointer_Conversion: {
-QualType LHSType = Context.getCanonicalType(ToType).getUnqualifiedType();
-QualType RHSType = Context.getCanonicalType(FromType).getUnqualifiedType();
-
-// Assumptions based on Sema::IsBlockPointerConversion.
-assert(isa(LHSType) && "BlockPointerType expected");
-assert(isa(RHSType) && "BlockPointerType expected");
-
 LangAS AddrSpaceL =
-LHSType->getAs()->getPointeeType().getAddressSpace();
+ToType->castAs()->getPointeeType().getAddressSpace();
 LangAS AddrSpaceR =
-RHSType->getAs()->getPointeeType().getAddressSpace();
+
FromType->castAs()->getPointeeType().getAddressSpace();
+assert(Qualifiers::isAddressSpaceSupersetOf(AddrSpaceL, AddrSpaceR) &&
+   "Invalid cast");
 CastKind Kind =
 AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
 From = ImpCastExprToType(From, ToType.getUnqualifiedType(), Kind,


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4216,17 +4216,12 @@
 break;
 
   case ICK_Block_Pointer_Conversion: {
-QualType LHSType = Context.getCanonicalType(ToType).getUnqualifiedType();
-QualType RHSType = Context.getCanonicalType(FromType).getUnqualifiedType();
-
-// Assumptions based on Sema::IsBlockPointerConversion.
-assert(isa(LHSType) && "BlockPointerType expected");
-assert(isa(RHSType) && "BlockPointerType expected");
-
 LangAS AddrSpaceL =
-LHSType->getAs()->getPointeeType().getAddressSpace();
+ToType->castAs()->getPointeeType().getAddressSpace();
 LangAS AddrSpaceR =
-RHSType->getAs()->getPointeeType().getAddressSpace();
+FromType->castAs()->getPointeeType().getAddressSpace();
+assert(Qualifiers::isAddressSpaceSupersetOf(AddrSpaceL, AddrSpaceR) &&
+   "Invalid cast");
 CastKind Kind =
 AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
 From = ImpCastExprToType(From, ToType.getUnqualifiedType(), Kind,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64804: [OpenCL][Sema] Minor refactoring and constraint checking

2019-07-16 Thread Marco Antognini via Phabricator via cfe-commits
mantognini added reviewers: Anastasia, rjmccall.
mantognini added a comment.

This should address the minor refactoring requesting in D64083 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64804



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


[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

2019-07-16 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Ping!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63648



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


[PATCH] D64644: Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

2019-07-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante updated this revision to Diff 210128.
Mordante marked an inline comment as done.
Mordante retitled this revision from "Fixes a clang frontend assertion failure 
(bug 35682)" to "Fixes an assertion failure while instantiation a template with 
an incomplete typo corrected type".
Mordante edited the summary of this revision.
Mordante added a comment.

Fixes @lebedev.ri's remark and adds the requested test.
While working on the test I discovered the initial assumption in bug 35682 was 
incorrect. Updated the title and summary accordingly.


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

https://reviews.llvm.org/D64644

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp


Index: 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,47 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | 
FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+#include 
+#include 
+
+template 
+class allocator;
+
+template 
+struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template 
+struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template 
+static constexpr bool enable_implicit() {
+  return std::is_constructible::value;
+}
+  };
+
+  template (),
+bool>::type = false>
+  single(U1 &&u1) : first(std::forward(u1)) {}
+};
+
+using SetKeyType = String;
+single v;
+
+// CHECK: error: unknown type name 'String'; did you mean 'string'?
+// CHECK: fatal error: too many errors emitted, stopping now [-ferror-limit=]
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,14 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *TemplateArgs) {
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+return ExprError();
+
   return DependentScopeDeclRefExpr::Create(
-  Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
-  TemplateArgs);
+  Context, std::move(QualifierLoc), TemplateKWLoc, NameInfo, TemplateArgs);
 }
 
 


Index: clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,47 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+#include 
+#include 
+
+template 
+class allocator;
+
+template 
+struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template 
+struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template 
+static constexpr bool enable_implicit() {
+  return std::is_constructible::value;
+}
+  };
+
+  template (),
+bool>::type = false>
+  single(U1 &&u1) : first(std::forward(u1)) {}
+};
+
+using SetKeyType = String;
+single v;
+
+// CHECK: error: unknown type name 'String'; did you mean 'string'?
+// CHECK: fatal error: too many errors emitted, stopping now [-ferror-limit=]
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,14 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 

[PATCH] D64644: Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

2019-07-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp:9-10
+
+#include 
+#include 
+

Please don't pull in any external headers - just mock what you need.


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

https://reviews.llvm.org/D64644



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


r366231 - fix unnamed fiefield issue and add tests for __builtin_preserve_access_index intrinsic

2019-07-16 Thread Yonghong Song via cfe-commits
Author: yhs
Date: Tue Jul 16 10:24:33 2019
New Revision: 366231

URL: http://llvm.org/viewvc/llvm-project?rev=366231&view=rev
Log:
fix unnamed fiefield issue and add tests for __builtin_preserve_access_index 
intrinsic

The original commit is r366076. It is temporarily reverted (r366155)
due to test failure. This resubmit makes test more robust by accepting
regex instead of hardcoded names/references in several places.

This is a followup patch for https://reviews.llvm.org/D61809.
Handle unnamed bitfield properly and add more test cases.

Fixed the unnamed bitfield issue. The unnamed bitfield is ignored
by debug info, so we need to ignore such a struct/union member
when we try to get the member index in the debug info.

D61809 contains two test cases but not enough as it does
not checking generated IRs in the fine grain level, and also
it does not have semantics checking tests.
This patch added unit tests for both code gen and semantics checking for
the new intrinsic.

Signed-off-by: Yonghong Song 

Added:
cfe/trunk/test/CodeGen/builtin-preserve-access-index.c
cfe/trunk/test/Sema/builtin-preserve-access-index.c
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=366231&r1=366230&r2=366231&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Jul 16 10:24:33 2019
@@ -3892,6 +3892,23 @@ LValue CodeGenFunction::EmitLValueForLam
   return EmitLValueForField(LambdaLV, Field);
 }
 
+/// Get the field index in the debug info. The debug info structure/union
+/// will ignore the unnamed bitfields.
+unsigned CodeGenFunction::getDebugInfoFIndex(const RecordDecl *Rec,
+ unsigned FieldIndex) {
+  unsigned I = 0, Skipped = 0;
+
+  for (auto F : Rec->getDefinition()->fields()) {
+if (I == FieldIndex)
+  break;
+if (F->isUnnamedBitfield())
+  Skipped++;
+I++;
+  }
+
+  return FieldIndex - Skipped;
+}
+
 /// Get the address of a zero-sized field within a record. The resulting
 /// address doesn't necessarily have the right type.
 static Address emitAddrOfZeroSizeField(CodeGenFunction &CGF, Address Base,
@@ -3931,7 +3948,7 @@ static Address emitPreserveStructAccess(
   CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
 
   return CGF.Builder.CreatePreserveStructAccessIndex(
-  base, idx, field->getFieldIndex(), DbgInfo);
+  base, idx, CGF.getDebugInfoFIndex(rec, field->getFieldIndex()), DbgInfo);
 }
 
 static bool hasAnyVptr(const QualType Type, const ASTContext &Context) {
@@ -4048,7 +4065,7 @@ LValue CodeGenFunction::EmitLValueForFie
   getContext().getRecordType(rec), rec->getLocation());
   addr = Address(
   Builder.CreatePreserveUnionAccessIndex(
-  addr.getPointer(), field->getFieldIndex(), DbgInfo),
+  addr.getPointer(), getDebugInfoFIndex(rec, 
field->getFieldIndex()), DbgInfo),
   addr.getAlignment());
 }
   } else {

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=366231&r1=366230&r2=366231&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Jul 16 10:24:33 2019
@@ -2652,6 +2652,9 @@ public:
   /// Converts Location to a DebugLoc, if debug information is enabled.
   llvm::DebugLoc SourceLocToDebugLoc(SourceLocation Location);
 
+  /// Get the record field index as represented in debug info.
+  unsigned getDebugInfoFIndex(const RecordDecl *Rec, unsigned FieldIndex);
+
 
   
//======//
   //Declaration Emission

Added: cfe/trunk/test/CodeGen/builtin-preserve-access-index.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-preserve-access-index.c?rev=366231&view=auto
==
--- cfe/trunk/test/CodeGen/builtin-preserve-access-index.c (added)
+++ cfe/trunk/test/CodeGen/builtin-preserve-access-index.c Tue Jul 16 10:24:33 
2019
@@ -0,0 +1,177 @@
+// RUN: %clang -target x86_64 -emit-llvm -S -g %s -o - | FileCheck %s
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+const void *unit1(const void *arg) {
+  return _(arg);
+}
+// CHECK: define dso_local i8* @unit1
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+
+const void *unit2(void) {
+  return _((const void *)0xULL);
+}
+// CHECK: define dso_local i8* @unit2
+// CHECK-NOT: llvm.preserve.array.access.index
+// 

[PATCH] D62584: [OpenCL][PR42033] Deducing addr space with template parameter types

2019-07-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Minor comment then LGTM




Comment at: lib/Sema/SemaType.cpp:7418
+  // Expect for pointer or reference types because the addr space in
+  // template argument can only belong to a pointee.
+  (T->isDependentType() && !T->isPointerType() && !T->isReferenceType()) ||

"Except"


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

https://reviews.llvm.org/D62584



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


Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies

2019-07-16 Thread Chris Bieneman via cfe-commits
I get that CMake does something different for PRIVATE, but the way we use CMake 
in LLVM we really can't make a private distinction reasonable. Because we don't 
sub-divide our headers per library, we don't support per-dependency include 
paths in LLVM or Clang's libraries.

This behavior also assumes you are using `target_include_directories` to assign 
PRIVATE | PUBLIC | INTERFACE to include directories, which we don't generally 
do in LLVM (a quick grep showed one place in LLVM code excluding our google 
benchmark drop). We do generally use `include_directories` which always treats 
things as PRIVATE, so they don't cascade to dependencies.

-Chris

> On Jul 12, 2019, at 7:16 PM, Shoaib Meenai  wrote:
> 
> I struggled for a while thinking why PRIVATE might be useful in a 
> target_link_libraries call for a shared library, and then I found 
> https://cmake.org/pipermail/cmake/2016-May/063400.html 
> . The relevant bit is:
>  
> If you were paying careful attention, you would have noticed that when A
> links in B as PRIVATE, the include directories of B never propagate to
> something linking to A, but if A is a static library, then the *linking* of
> B behaves as though the relationship was PUBLIC. This
> PRIVATE-becomes-PUBLIC behaviour for static libraries only applies to the
> *linking*, not to the other dependencies (compiler options/flags and
> include search paths).
>  
> So PRIVATE/INTERFACE/PUBLIC doesn’t make any difference as far as the actual 
> linking goes, but it does affect propagation of other options, and I think 
> it’s valid to want to have a PRIVATE dependency for a static library.
>  
> From:  on behalf of Chris Bieneman 
> Date: Friday, July 12, 2019 at 9:08 AM
> To: Shoaib Meenai 
> Cc: Alex Bradbury , cfe-commits 
> Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies
>  
> Ah! I see what is going on here. This is kinda a silliness of CMake. 
> `PRIVATE` linkage for a static archive is silly, and shouldn't be possible. 
> All link dependencies for static archives are really `INTERFACE` 
> dependencies, and it looks like CMake is doing something kinda silly instead 
> of producing a reasonable error or warning like it probably should do.
>  
> For context, `PRIVATE` linkage in the context of that change would mean 
> DirectoryWalker links something, but things that link DirectoryWalker don't. 
> That just isn't possible with static archives, so they become interface 
> dependencies (and CMake seems to insert a generator expression to make that 
> work).
>  
> In `llvm_add_library` we always use `PRIVATE` linkage for shared libraries, 
> and `INTERFACE` for static, which does the right thing.
>  
> Unless there is a better reason than a new patch coming in, I think the right 
> fix is to revert this back and expect the new patch to correct its linkage 
> behavior.
>  
> -Chris
> 
> 
> On Jul 12, 2019, at 8:53 AM, Shoaib Meenai  > wrote:
>  
> See https://reviews.llvm.org/D58418#1577670 
> .
>  More generally it would appear for any static library with a PRIVATE 
> dependency though.
>  
> I guess an alternative would be to use the LINK_LIBRARIES property (which 
> should be free of generator expressions, I believe) to propagate dependencies 
> instead of INTERFACE_LINK_LIBRARIES, and just assume that no one is gonna set 
> an INTERFACE dependency on a static library. (Supporting PRIVATE dependencies 
> on a static library definitely seems more valuable than supporting INTERFACE 
> dependencies.)
>  
> From: mailto:cbiene...@apple.com>> on behalf of Chris 
> Bieneman mailto:be...@apple.com>>
> Date: Friday, July 12, 2019 at 8:49 AM
> To: Shoaib Meenai mailto:smee...@fb.com>>
> Cc: Alex Bradbury mailto:a...@lowrisc.org>>, cfe-commits 
> mailto:cfe-commits@lists.llvm.org>>
> Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies
>  
> One of the benefits of the object library approach for generating the clang 
> dylib is that it was compatible with BUILD_SHARED_LIBS. We had lots of issues 
> with libLLVM where people using BUILD_SHARED_LIBS would make changes that 
> broke it, so I was trying to make the clang dylib in a way that it could 
> always be enabled.
>  
> Do we know where the nested generator expression was coming from?
>  
> -Chris
> 
> 
> 
> On Jul 12, 2019, at 8:32 AM, Shoaib Meenai  > wrote:
>  
> Oops, sorry about the breakage.
>  
> Chris, aren't BUILD_SHARED_LIBS and the combined Clang dylib incompatible? 
> Should we disable building the latter if the former is set?
>  
> From: Alex Bradbury mailto:a...@lowrisc.org>>
> Date: Friday, July 12, 2019 at 2:02 AM
> To: Shoaib Meenai mailto:

Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies

2019-07-16 Thread Shoaib Meenai via cfe-commits
Makes sense, thanks. Seems like INTERFACE is the way to go then.

From:  on behalf of Chris Bieneman 
Date: Tuesday, July 16, 2019 at 10:32 AM
To: Shoaib Meenai 
Cc: Alex Bradbury , cfe-commits 
Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies

I get that CMake does something different for PRIVATE, but the way we use CMake 
in LLVM we really can't make a private distinction reasonable. Because we don't 
sub-divide our headers per library, we don't support per-dependency include 
paths in LLVM or Clang's libraries.

This behavior also assumes you are using `target_include_directories` to assign 
PRIVATE | PUBLIC | INTERFACE to include directories, which we don't generally 
do in LLVM (a quick grep showed one place in LLVM code excluding our google 
benchmark drop). We do generally use `include_directories` which always treats 
things as PRIVATE, so they don't cascade to dependencies.

-Chris


On Jul 12, 2019, at 7:16 PM, Shoaib Meenai 
mailto:smee...@fb.com>> wrote:

I struggled for a while thinking why PRIVATE might be useful in a 
target_link_libraries call for a shared library, and then I found 
https://cmake.org/pipermail/cmake/2016-May/063400.html.
 The relevant bit is:

If you were paying careful attention, you would have noticed that when A
links in B as PRIVATE, the include directories of B never propagate to
something linking to A, but if A is a static library, then the *linking* of
B behaves as though the relationship was PUBLIC. This
PRIVATE-becomes-PUBLIC behaviour for static libraries only applies to the
*linking*, not to the other dependencies (compiler options/flags and
include search paths).

So PRIVATE/INTERFACE/PUBLIC doesn’t make any difference as far as the actual 
linking goes, but it does affect propagation of other options, and I think it’s 
valid to want to have a PRIVATE dependency for a static library.

From: mailto:cbiene...@apple.com>> on behalf of Chris 
Bieneman mailto:be...@apple.com>>
Date: Friday, July 12, 2019 at 9:08 AM
To: Shoaib Meenai mailto:smee...@fb.com>>
Cc: Alex Bradbury mailto:a...@lowrisc.org>>, cfe-commits 
mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies

Ah! I see what is going on here. This is kinda a silliness of CMake. `PRIVATE` 
linkage for a static archive is silly, and shouldn't be possible. All link 
dependencies for static archives are really `INTERFACE` dependencies, and it 
looks like CMake is doing something kinda silly instead of producing a 
reasonable error or warning like it probably should do.

For context, `PRIVATE` linkage in the context of that change would mean 
DirectoryWalker links something, but things that link DirectoryWalker don't. 
That just isn't possible with static archives, so they become interface 
dependencies (and CMake seems to insert a generator expression to make that 
work).

In `llvm_add_library` we always use `PRIVATE` linkage for shared libraries, and 
`INTERFACE` for static, which does the right thing.

Unless there is a better reason than a new patch coming in, I think the right 
fix is to revert this back and expect the new patch to correct its linkage 
behavior.

-Chris



On Jul 12, 2019, at 8:53 AM, Shoaib Meenai 
mailto:smee...@fb.com>> wrote:

See 
https://reviews.llvm.org/D58418#1577670.
 More generally it would appear for any static library with a PRIVATE 
dependency though.

I guess an alternative would be to use the LINK_LIBRARIES property (which 
should be free of generator expressions, I believe) to propagate dependencies 
instead of INTERFACE_LINK_LIBRARIES, and just assume that no one is gonna set 
an INTERFACE dependency on a static library. (Supporting PRIVATE dependencies 
on a static library definitely seems more valuable than supporting INTERFACE 
dependencies.)

From: mailto:cbiene...@apple.com>> on behalf of Chris 
Bieneman mailto:be...@apple.com>>
Date: Friday, July 12, 2019 at 8:49 AM
To: Shoaib Meenai mailto:smee...@fb.com>>
Cc: Alex Bradbury mailto:a...@lowrisc.org>>, cfe-commits 
mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies

One of the benefits of the object library approach for generating the clang 
dylib is that it was compatible with BUILD_SHARED_LIBS. We had lots of issues 
with libLLVM where people using BUILD_SHARED_LIBS would make changes that broke 
it, so I was trying to make the clang dylib in a way that it could always be 
enabled.

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

2019-07-16 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1307
+  HasStdlibxxIsystem ? TC.AddClangCXXStdlibIsystemArgs(Args, CmdArgs)
+ : TC.AddClangCXXStdlibIncludeArgs(Args, CmdArgs);
+});

So, before, we would populate `-stdlib=XXX` here unconditionally for `cc1`.

After the patch, we do _not_ populate `-stdlib=XXX` and instead we pass 
`-internal-isystem` to `cc1` with the contents of `-stdlib++-isystem` whenever 
that option is provided, otherwise we fall back to the previous behaviour.

Did I get that right? If so, could we investigate getting rid of 
`AddClangCXXStdlibIncludeArgs` altogether? I don't think it is needed anymore 
since my refactor of the driver for Darwin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64089



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


[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

2019-07-16 Thread Ziang Wan via Phabricator via cfe-commits
ziangwan marked an inline comment as done.
ziangwan added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:11429
+  S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)));
+TargetFloatValue.convertFromAPInt(SourceInt,
+  SourceBT->isSignedInteger(), llvm::APFloat::rmNearestTiesToEven);

scanon wrote:
> Why don't we just check that the result of the first conversion is opOK? I 
> don't think doing a round-trip check is required here.
I have changed the code to check the status of the first conversion only. 
Please review it again.


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

https://reviews.llvm.org/D64666



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


[PATCH] D64569: [OpenCL] Improve destructor support in C++ for OpenCL

2019-07-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGClass.cpp:2016
   CGF.EmitCXXDestructorCall(dtor, Dtor_Complete, /*for vbase*/ false,
-/*Delegating=*/false, addr);
+/*Delegating=*/false, addr, type);
 }

mantognini wrote:
> rjmccall wrote:
> > mantognini wrote:
> > > This is the only place where the new parameter has an interesting value. 
> > > In the ~10 other calls to `EmitCXXDestructorCall` overloads, the default 
> > > value of this new parameter is used instead.
> > Arguments that are potentially required for correctness — as opposed to 
> > just enabling some optimization — should generally not have defaults.  I 
> > think you should remove these defaults and require a sensible type to be 
> > passed down in all cases.
> I've addressed that. I had to add some extra parameter/attributes here and 
> there. Please let me know if anything is can be improved.
Thanks, other than the FIXMEs and the object/pointer mismatches this all looks 
right; very nicely done.



Comment at: clang/lib/CodeGen/CGClass.cpp:496
+  // destroyed should have the expected type.
+  QualType ThisTy = D->getThisType();
   Address Addr =

I think the rule we want is that the type passed here is the (qualified) object 
type, but `getThisType()` will return a pointer type.  Consider adding a 
`getThisObjectType()` method to `CXXMethodDecl` which does that computation 
(and then make `getThisType()` just wrap that in a `PointerType`).



Comment at: clang/lib/CodeGen/CGClass.cpp:1447
+if (HaveInsertPoint()) {
+  // FIXME: Determine the type of the object being destroyed.
+  QualType ThisTy;

mantognini wrote:
> I'm not familiar enough with CXXCtorType and such, and would welcome some 
> help for these two FIXMEs.
`Dtor->getThisObjectType()` should be right in both of these cases.



Comment at: clang/lib/CodeGen/CGClass.cpp:2376
+  // Therefore, "this" should have the expected type.
+  QualType ThisTy = Dtor->getThisType();
   CGF.EmitCXXDestructorCall(Dtor, Type, /*ForVirtualBase=*/false,

Same thing about `getThisObjectType()`.



Comment at: clang/lib/CodeGen/CGExprCXX.cpp:103
+  // we ensure a cast is added where necessary.
+  if (ThisTy.isNull()) {
+#ifndef NDEBUG

mantognini wrote:
> Despite no longer having a default parameter, not all call site can provide a 
> meaningful value ATM. That is why this check is still required.
Is that resolved with fixing the FIXME?

Please assert that `ThisTy->getAsCXXRecordDecl() == Dtor->getParent()` to guard 
against that pointer/object mixup.



Comment at: clang/lib/CodeGen/CGExprCXX.cpp:374
   EmitCXXDestructorCall(GD, Callee, This.getPointer(),
+/*ThisTy=*/QualType(),
 /*ImplicitParam=*/nullptr,

`ThisTy` here can be either `Base->getType()` or 
`Base->getType()->getPointeeType()` depending on whether this is an arrow 
access.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1758
+  // This is suboptimal for correctness purposes. Instead, CE should probably
+  // always be defined.
+  QualType ThisTy;

It looks like the only places that pass down a null CE are the two 
implementations in `emitVirtualObjectDelete`, which both have a 
`CXXDeleteExpr`.  You could just have this method take an 
`llvm::PointerUnion` or something, and then 
someone else could take advantage of that for better source locations on the 
virtual call eventually.



Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:1928
+  if (CE)
+ThisTy = CE->getImplicitObjectArgument()->getType();
+

Same point as in the ItaniumCXXABI implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64569



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


[PATCH] D48680: Add missing visibility annotation for __base

2019-07-16 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

@pcc can you please submit this patch if there are no objections?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48680



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


Re: r366123 - ARM MTE stack sanitizer.

2019-07-16 Thread Evgenii Stepanov via cfe-commits
Hi,

thanks for letting me know! Is this reproducible on Linux? It is
possible to extract a reproducer from the bot?

On Mon, Jul 15, 2019 at 9:30 PM Amara Emerson  wrote:
>
> Hi Evgeniy,
>
> This commit looks like it broke the lldb bot: 
> http://green.lab.llvm.org/green/job/lldb-cmake/31011/
>
> Can you take a look?
>
> Amara
>
> On Jul 15, 2019, at 1:02 PM, Evgeniy Stepanov via cfe-commits 
>  wrote:
>
> Author: eugenis
> Date: Mon Jul 15 13:02:23 2019
> New Revision: 366123
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366123&view=rev
> Log:
> ARM MTE stack sanitizer.
>
> Add "memtag" sanitizer that detects and mitigates stack memory issues
> using armv8.5 Memory Tagging Extension.
>
> It is similar in principle to HWASan, which is a software implementation
> of the same idea, but there are enough differencies to warrant a new
> sanitizer type IMHO. It is also expected to have very different
> performance properties.
>
> The new sanitizer does not have a runtime library (it may grow one
> later, along with a "debugging" mode). Similar to SafeStack and
> StackProtector, the instrumentation pass (in a follow up change) will be
> inserted in all cases, but will only affect functions marked with the
> new sanitize_memtag attribute.
>
> Reviewers: pcc, hctim, vitalybuka, ostannard
>
> Subscribers: srhines, mehdi_amini, javed.absar, kristof.beyls, hiraditya, 
> cryptoad, steven_wu, dexonsmith, cfe-commits, llvm-commits
>
> Tags: #clang, #llvm
>
> Differential Revision: https://reviews.llvm.org/D64169
>
> Added:
>cfe/trunk/test/CodeGen/memtag-attr.cpp
>cfe/trunk/test/Lexer/has_feature_memtag_sanitizer.cpp
> Modified:
>cfe/trunk/include/clang/Basic/Features.def
>cfe/trunk/include/clang/Basic/Sanitizers.def
>cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>cfe/trunk/lib/CodeGen/SanitizerMetadata.cpp
>cfe/trunk/lib/Driver/SanitizerArgs.cpp
>cfe/trunk/lib/Driver/ToolChains/Linux.cpp
>cfe/trunk/test/Driver/fsanitize.c
>cfe/trunk/test/SemaCXX/attr-no-sanitize.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Features.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=366123&r1=366122&r2=366123&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Features.def (original)
> +++ cfe/trunk/include/clang/Basic/Features.def Mon Jul 15 13:02:23 2019
> @@ -42,6 +42,7 @@ FEATURE(address_sanitizer,
> FEATURE(hwaddress_sanitizer,
> LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
>SanitizerKind::KernelHWAddress))
> +FEATURE(memtag_sanitizer, LangOpts.Sanitize.has(SanitizerKind::MemTag))
> FEATURE(xray_instrument, LangOpts.XRayInstrument)
> FEATURE(undefined_behavior_sanitizer,
> LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
>
> Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=366123&r1=366122&r2=366123&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
> +++ cfe/trunk/include/clang/Basic/Sanitizers.def Mon Jul 15 13:02:23 2019
> @@ -55,6 +55,9 @@ SANITIZER("hwaddress", HWAddress)
> // Kernel Hardware-assisted AddressSanitizer (KHWASan)
> SANITIZER("kernel-hwaddress", KernelHWAddress)
>
> +// A variant of AddressSanitizer using AArch64 MTE extension.
> +SANITIZER("memtag", MemTag)
> +
> // MemorySanitizer
> SANITIZER("memory", Memory)
>
>
> Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=366123&r1=366122&r2=366123&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Mon Jul 15 13:02:23 2019
> @@ -369,6 +369,10 @@ llvm::Function *CodeGenModule::CreateGlo
>   !isInSanitizerBlacklist(SanitizerKind::KernelHWAddress, Fn, Loc))
> Fn->addFnAttr(llvm::Attribute::SanitizeHWAddress);
>
> +  if (getLangOpts().Sanitize.has(SanitizerKind::MemTag) &&
> +  !isInSanitizerBlacklist(SanitizerKind::MemTag, Fn, Loc))
> +Fn->addFnAttr(llvm::Attribute::SanitizeMemTag);
> +
>   if (getLangOpts().Sanitize.has(SanitizerKind::Thread) &&
>   !isInSanitizerBlacklist(SanitizerKind::Thread, Fn, Loc))
> Fn->addFnAttr(llvm::Attribute::SanitizeThread);
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=366123&r1=366122&r2=366123&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Mon Ju

[PATCH] D48680: Add missing visibility annotation for __base

2019-07-16 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

I think it would be up to the libc++ maintainers to approve the patch.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48680



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


[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

2019-07-16 Thread Steve Canon via Phabricator via cfe-commits
scanon accepted this revision.
scanon added a comment.
This revision is now accepted and ready to land.

LGTM. Please get at least one additional reviewer's approval before merging, as 
this is a corner of clang that I don't work on often.


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

https://reviews.llvm.org/D64666



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


[PATCH] D48680: Add missing visibility annotation for __base

2019-07-16 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: dexonsmith.

@pcc In your reproducer, what is `~/l3/ra-cmake/bin/clang++`? Are you able to 
reproduce without `-fuse-ld=lld`? I'm trying to reproduce locally but I can't.

I know this is a small change, but I'd like to understand why it's needed since 
I suspect it may uncover other places where visibility is wrong -- if the 
problem is really on libc++'s side.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48680



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


[PATCH] D64811: Warn when NumParams overflows

2019-07-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision.
Mordante added reviewers: rsmith, rjmccall.
Mordante added a project: clang.

Before when the overflow occurred an assertion was triggered. Now check whether 
the maximum has been reached and warn properly.

This patch fixes bug 33162 which is marked as 'duplicate' of bug 19607. The 
original part of bug 19607 is fixed by D63975 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64811

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/Parser/function_parameter_overflow.cpp
  clang/test/Parser/lambda_function_parameter_overflow.cpp
  clang/test/Parser/parameter_overflow.h

Index: clang/test/Parser/parameter_overflow.h
===
--- /dev/null
+++ clang/test/Parser/parameter_overflow.h
@@ -0,0 +1,9 @@
+#pragma once
+
+#define I10 int, int, int, int, int, int, int, int, int, int
+#define I50 I10, I10, I10, I10, I10
+#define I500 I50, I50, I50, I50, I50, I50, I50, I50, I50, I50
+#define I5000 I500, I500, I500, I500, I500, I500, I500, I500, I500, I500
+#define I6 I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000
+
+#define  I65535 I6, I5000, I500, I10, I10, I10, int, int, int, int, int
Index: clang/test/Parser/lambda_function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/lambda_function_parameter_overflow.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#include "parameter_overflow.h"
+
+auto foo = [](I65535
+#ifdef FAIL
+, int
+#endif
+){};
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/test/Parser/function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/function_parameter_overflow.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#include "parameter_overflow.h"
+
+void foo(I65535
+#ifdef FAIL
+, int
+#endif
+);
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -1265,6 +1265,14 @@
 
   ParseParameterDeclarationClause(D, Attr, ParamInfo, EllipsisLoc);
 
+  // If the parameters could not be parsed stop processing. Proceeding is
+  // not an issue when the maximum scope depth is exceeded. But when the
+  // maximum number of parameters is exceeded the processing will still hit
+  // the assertion in FunctionProtoType's constructor.
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (Tok.is(tok::eof))
+return ExprError();
+
   // For a generic lambda, each 'auto' within the parameter declaration
   // clause creates a template type parameter, so increment the depth.
   // If we've parsed any explicit template parameters, then the depth will
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6668,6 +6668,15 @@
 
 // If the next token is a comma, consume it and keep reading arguments.
   } while (TryConsumeToken(tok::comma));
+
+  // Avoid exceeding the maximum function parameters
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (ParamInfo.size() > Type::getMaxNumParams()) {
+Diag(ParamInfo[Type::getMaxNumParams() - 1].IdentLoc,
+ diag::err_number_of_function_parameters_exceeded)
+<< Type::getMaxNumParams();
+cutOffParsing();
+  }
 }
 
 /// [C90]   direct-declarator '[' constant-expression[opt] ']'
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -326,6 +326,8 @@
 def err_argument_required_after_attribute : Error<
   "argument required after attribute">;
 def err_missing_param : Error<"expected parameter declarator">;
+def err_number_of_function_parameters_exceeded : Error<
+  "number of function parameters exceeded maximum of %0">, DefaultFatal;
 def err_missing_comma_before_ellipsis : Error<
   "C requires a comma prior to the ellipsis in a variadic function type">;
 def err_unexpected_typedef_ident : Error<
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1520,6 +1520,8 @@
 unsigned Kind : 8;
   };
 
+  enum { Num

Re: r366076 - fix unnamed fiefield issue and add tests for __builtin_preserve_access_index intrinsic

2019-07-16 Thread Nick Desaulniers via cfe-commits
On Mon, Jul 15, 2019 at 5:13 PM Eric Christopher  wrote:
>
> I'm going to cheat and make Nick do it :)

I suspect that Eric still compiles LLVM by specifying all object files
in order on the command line and doesn't want to talk about it on
cfe-commits. :) Though, even I'm behind the times as I think the cool
kids are using "gn" these days?  I'm so out of touch.

> On Mon, Jul 15, 2019 at 5:12 PM Yonghong Song  wrote:
> > I just tried the following cmake (removing -DLLVM_ENABLE_ASSERTIONS=ON 
> > which is used in my previous build)
> >
> > cmake -G "Unix Makefiles" -DLLVM_TARGETS_TO_BUILD="BPF;X86" \
> > -DCMAKE_C_COMPILER=/llvm8/bin/clang \
> > -DCMAKE_CXX_COMPILER=/llvm8/bin/clang++ \
> > -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON
> > -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$PWD/install ..
> >
> > and cannot reproduce the issue. If you could send me the cmake
> > command line which caused failure in your environment. That will
> > be great!

I did:
$ cmake .. -DCMAKE_BUILD_TYPE=Release -G Ninja
-DCMAKE_C_COMPILER=
-DCMAKE_CXX_COMPILER=
-DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_PROJECTS="clang;lld"
-DLLVM_TARGETS_TO_BUILD="AArch64;ARM;X86"

I sometimes add:
-DLLVM_ENABLE_ASSERTIONS=ON
which hopefully gets cleared when rerunning cmake, but I wouldn't bet
my life on that. (Looks like it's OFF in my CMakeCache.txt, so I guess
I could've taken that bet).

-- 
Thanks,
~Nick Desaulniers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r366076 - fix unnamed fiefield issue and add tests for __builtin_preserve_access_index intrinsic

2019-07-16 Thread Nick Desaulniers via cfe-commits
On Tue, Jul 16, 2019 at 11:02 AM Nick Desaulniers
 wrote:
>
> On Mon, Jul 15, 2019 at 5:13 PM Eric Christopher  wrote:
> >
> > I'm going to cheat and make Nick do it :)
>
> I suspect that Eric still compiles LLVM by specifying all object files
> in order on the command line and doesn't want to talk about it on
> cfe-commits. :) Though, even I'm behind the times as I think the cool
> kids are using "gn" these days?  I'm so out of touch.
>
> > On Mon, Jul 15, 2019 at 5:12 PM Yonghong Song  wrote:
> > > I just tried the following cmake (removing -DLLVM_ENABLE_ASSERTIONS=ON 
> > > which is used in my previous build)
> > >
> > > cmake -G "Unix Makefiles" -DLLVM_TARGETS_TO_BUILD="BPF;X86" \
> > > -DCMAKE_C_COMPILER=/llvm8/bin/clang \
> > > -DCMAKE_CXX_COMPILER=/llvm8/bin/clang++ \
> > > -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON
> > > -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$PWD/install ..
> > >
> > > and cannot reproduce the issue. If you could send me the cmake
> > > command line which caused failure in your environment. That will
> > > be great!
>
> I did:
> $ cmake .. -DCMAKE_BUILD_TYPE=Release -G Ninja
> -DCMAKE_C_COMPILER=
> -DCMAKE_CXX_COMPILER=
> -DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_PROJECTS="clang;lld"
> -DLLVM_TARGETS_TO_BUILD="AArch64;ARM;X86"
>
> I sometimes add:
> -DLLVM_ENABLE_ASSERTIONS=ON
> which hopefully gets cleared when rerunning cmake, but I wouldn't bet
> my life on that. (Looks like it's OFF in my CMakeCache.txt, so I guess
> I could've taken that bet).

Forgot to mention that jdoerfert also mentioned on IRC yesterday that
-DLLVM_ENABLE_ASSERTIONS=ON can influence Clang's codegen of LLVM IR.
-- 
Thanks,
~Nick Desaulniers
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D48680: Add missing visibility annotation for __base

2019-07-16 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment.

In D48680#1587967 , @ldionne wrote:

> @pcc In your reproducer, what is `~/l3/ra-cmake/bin/clang++`?


That's just clang built from trunk at the time that I posted my comment.

> Are you able to reproduce without `-fuse-ld=lld`? I'm trying to reproduce 
> locally but I can't.

On which platform? On Linux lld (or gold) is needed in order to use LTO which 
is a requirement for CFI.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48680



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


Re: r366076 - fix unnamed fiefield issue and add tests for __builtin_preserve_access_index intrinsic

2019-07-16 Thread Yonghong Song via cfe-commits


On 7/16/19 11:03 AM, Nick Desaulniers wrote:
> On Tue, Jul 16, 2019 at 11:02 AM Nick Desaulniers
>  wrote:
>>
>> On Mon, Jul 15, 2019 at 5:13 PM Eric Christopher  wrote:
>>>
>>> I'm going to cheat and make Nick do it :)
>>
>> I suspect that Eric still compiles LLVM by specifying all object files
>> in order on the command line and doesn't want to talk about it on
>> cfe-commits. :) Though, even I'm behind the times as I think the cool
>> kids are using "gn" these days?  I'm so out of touch.
>>
>>> On Mon, Jul 15, 2019 at 5:12 PM Yonghong Song  wrote:
 I just tried the following cmake (removing -DLLVM_ENABLE_ASSERTIONS=ON 
 which is used in my previous build)

 cmake -G "Unix Makefiles" -DLLVM_TARGETS_TO_BUILD="BPF;X86" \
  -DCMAKE_C_COMPILER=/llvm8/bin/clang \
  -DCMAKE_CXX_COMPILER=/llvm8/bin/clang++ \
  -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON
  -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$PWD/install ..

 and cannot reproduce the issue. If you could send me the cmake
 command line which caused failure in your environment. That will
 be great!
>>
>> I did:
>> $ cmake .. -DCMAKE_BUILD_TYPE=Release -G Ninja
>> -DCMAKE_C_COMPILER=
>> -DCMAKE_CXX_COMPILER=
>> -DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_PROJECTS="clang;lld"
>> -DLLVM_TARGETS_TO_BUILD="AArch64;ARM;X86"
>>
>> I sometimes add:
>> -DLLVM_ENABLE_ASSERTIONS=ON
>> which hopefully gets cleared when rerunning cmake, but I wouldn't bet
>> my life on that. (Looks like it's OFF in my CMakeCache.txt, so I guess
>> I could've taken that bet).
> 
> Forgot to mention that jdoerfert also mentioned on IRC yesterday that
> -DLLVM_ENABLE_ASSERTIONS=ON can influence Clang's codegen of LLVM IR.

Thanks, Nick. Will give your cmake a try and keep in mind that
testing both LLVM_ENABLE_ASSERTIONS ON/OFF for clang changes.

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


[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

2019-07-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added reviewers: JonasToth, gribozavr.
lebedev.ri added a comment.

Thanks.
Are there any tests missing for `volatile`, atomics?
I'm not really current on clang-tidy state of affairs, so i'm gonna leave most 
of the review for others..




Comment at: clang-tidy/bugprone/InfiniteLoopCheck.cpp:21
+static internal::Matcher loopEndingStmt() {
+  return stmt(anyOf(breakStmt(), returnStmt(), gotoStmt(), cxxThrowExpr()));
+}

What about function calls marked `noreturn`?


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

https://reviews.llvm.org/D64736



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


[PATCH] D64644: Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

2019-07-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante updated this revision to Diff 210146.
Mordante added a comment.

Remove the includes from the test.
Changed the `std::is_constructible` to `is_same` since the latter is easier to 
mock.


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

https://reviews.llvm.org/D64644

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp


Index: 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,60 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | 
FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+template  class allocator;
+
+template  struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template  struct enable_if {};
+template  struct enable_if { typedef Tp type; };
+
+template  struct integral_constant {
+  static constexpr const Tp value = v;
+  typedef Tp value_type;
+  typedef integral_constant type;
+
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
+};
+
+template  constexpr const Tp integral_constant::value;
+
+using true_type = integral_constant;
+using false_type = integral_constant;
+
+template  struct is_same : public false_type {};
+template  struct is_same : public true_type {};
+
+template  struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template  static constexpr bool enable_implicit() {
+  return is_same::value;
+}
+  };
+
+  template (),
+   bool>::type = false>
+  single(U1 &&u1);
+};
+
+using SetKeyType = String;
+single v;
+
+// CHECK: error: unknown type name 'String'; did you mean 'string'?
+// CHECK: fatal error: too many errors emitted, stopping now [-ferror-limit=]
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,14 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *TemplateArgs) {
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+return ExprError();
+
   return DependentScopeDeclRefExpr::Create(
-  Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
-  TemplateArgs);
+  Context, std::move(QualifierLoc), TemplateKWLoc, NameInfo, TemplateArgs);
 }
 
 


Index: clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,60 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+template  class allocator;
+
+template  struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template  struct enable_if {};
+template  struct enable_if { typedef Tp type; };
+
+template  struct integral_constant {
+  static constexpr const Tp value = v;
+  typedef Tp value_type;
+  typedef integral_constant type;
+
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
+};
+
+template  constexpr const Tp integral_constant::value;
+
+using true_type = integral_constant;
+using false_type = integral_constant;
+
+template  struct is_same : public false_type {};
+template  struct is_same : public true_type {};
+
+template  struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template  static constexpr bool enable_implicit() {
+  

  1   2   >