[PATCH] D36209: [Bash-autocompletion] Add comment to test so that it is easier to fix

2017-08-02 Thread Yuka Takahashi via Phabricator via cfe-commits
yamaguchi updated this revision to Diff 109283.
yamaguchi added a comment.

Fixed typo.


https://reviews.llvm.org/D36209

Files:
  clang/test/Driver/autocomplete.c


Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -1,3 +1,7 @@
+// Test for the --autocompletion flag, which is an API used for shell
+// autocompletion. You may have to update tests in this file when you
+// add/modify flags, change HelpTexts or the values of some flags.
+
 // RUN: %clang --autocomplete=-fsyn | FileCheck %s -check-prefix=FSYN
 // FSYN: -fsyntax-only
 // RUN: %clang --autocomplete=-std= | FileCheck %s -check-prefix=STD


Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -1,3 +1,7 @@
+// Test for the --autocompletion flag, which is an API used for shell
+// autocompletion. You may have to update tests in this file when you
+// add/modify flags, change HelpTexts or the values of some flags.
+
 // RUN: %clang --autocomplete=-fsyn | FileCheck %s -check-prefix=FSYN
 // FSYN: -fsyntax-only
 // RUN: %clang --autocomplete=-std= | FileCheck %s -check-prefix=STD
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36209: [Bash-autocompletion] Add comment to test so that it is easier to fix

2017-08-02 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu accepted this revision.
ruiu added a comment.

LGTM


https://reviews.llvm.org/D36209



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


r309794 - [Bash-autocompletion] Add comment to test so that it is easier to fix

2017-08-02 Thread Yuka Takahashi via cfe-commits
Author: yamaguchi
Date: Wed Aug  2 00:20:27 2017
New Revision: 309794

URL: http://llvm.org/viewvc/llvm-project?rev=309794&view=rev
Log:
[Bash-autocompletion] Add comment to test so that it is easier to fix

Summary:
clang/test/Driver/autocomplete.c is a test for --autocomplete, and this
test might break if people add/modify flags or HelpText. So I've add
comment for future developers so that they can fix this file according
to the change they had made.

Reviewers: v.g.vassilev, teemperor, ruiu

Subscribers: cfe-commits

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

Modified:
cfe/trunk/test/Driver/autocomplete.c

Modified: cfe/trunk/test/Driver/autocomplete.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/autocomplete.c?rev=309794&r1=309793&r2=309794&view=diff
==
--- cfe/trunk/test/Driver/autocomplete.c (original)
+++ cfe/trunk/test/Driver/autocomplete.c Wed Aug  2 00:20:27 2017
@@ -1,3 +1,7 @@
+// Test for the --autocompletion flag, which is an API used for shell
+// autocompletion. You may have to update tests in this file when you
+// add/modify flags, change HelpTexts or the values of some flags.
+
 // RUN: %clang --autocomplete=-fsyn | FileCheck %s -check-prefix=FSYN
 // FSYN: -fsyntax-only
 // RUN: %clang --autocomplete=-std= | FileCheck %s -check-prefix=STD


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


[PATCH] D36209: [Bash-autocompletion] Add comment to test so that it is easier to fix

2017-08-02 Thread Yuka Takahashi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309794: [Bash-autocompletion] Add comment to test so that it 
is easier to fix (authored by yamaguchi).

Changed prior to commit:
  https://reviews.llvm.org/D36209?vs=109283&id=109285#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36209

Files:
  cfe/trunk/test/Driver/autocomplete.c


Index: cfe/trunk/test/Driver/autocomplete.c
===
--- cfe/trunk/test/Driver/autocomplete.c
+++ cfe/trunk/test/Driver/autocomplete.c
@@ -1,3 +1,7 @@
+// Test for the --autocompletion flag, which is an API used for shell
+// autocompletion. You may have to update tests in this file when you
+// add/modify flags, change HelpTexts or the values of some flags.
+
 // RUN: %clang --autocomplete=-fsyn | FileCheck %s -check-prefix=FSYN
 // FSYN: -fsyntax-only
 // RUN: %clang --autocomplete=-std= | FileCheck %s -check-prefix=STD


Index: cfe/trunk/test/Driver/autocomplete.c
===
--- cfe/trunk/test/Driver/autocomplete.c
+++ cfe/trunk/test/Driver/autocomplete.c
@@ -1,3 +1,7 @@
+// Test for the --autocompletion flag, which is an API used for shell
+// autocompletion. You may have to update tests in this file when you
+// add/modify flags, change HelpTexts or the values of some flags.
+
 // RUN: %clang --autocomplete=-fsyn | FileCheck %s -check-prefix=FSYN
 // FSYN: -fsyntax-only
 // RUN: %clang --autocomplete=-std= | FileCheck %s -check-prefix=STD
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36155: Use VFS operations in FileManager::makeAbsolutePath.

2017-08-02 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309795: Use VFS operations in FileManager::makeAbsolutePath. 
(authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D36155

Files:
  cfe/trunk/lib/Basic/FileManager.cpp
  cfe/trunk/unittests/Basic/FileManagerTest.cpp


Index: cfe/trunk/lib/Basic/FileManager.cpp
===
--- cfe/trunk/lib/Basic/FileManager.cpp
+++ cfe/trunk/lib/Basic/FileManager.cpp
@@ -408,7 +408,7 @@
   bool Changed = FixupRelativePath(Path);
 
   if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size( {
-llvm::sys::fs::make_absolute(Path);
+FS->makeAbsolute(Path);
 Changed = true;
   }
 
Index: cfe/trunk/unittests/Basic/FileManagerTest.cpp
===
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/FileSystemStatCache.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Path.h"
@@ -296,4 +297,30 @@
 
 #endif  // !LLVM_ON_WIN32
 
+TEST_F(FileManagerTest, makeAbsoluteUsesVFS) {
+  SmallString<64> CustomWorkingDir;
+#ifdef LLVM_ON_WIN32
+  CustomWorkingDir = "C:";
+#else
+  CustomWorkingDir = "/";
+#endif
+  llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path");
+
+  auto FS =
+  IntrusiveRefCntPtr(new vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  SmallString<64> Path("a/foo.cpp");
+
+  SmallString<64> ExpectedResult(CustomWorkingDir);
+  llvm::sys::path::append(ExpectedResult, Path);
+
+  ASSERT_TRUE(Manager.makeAbsolutePath(Path));
+  EXPECT_EQ(Path, ExpectedResult);
+}
+
 } // anonymous namespace


Index: cfe/trunk/lib/Basic/FileManager.cpp
===
--- cfe/trunk/lib/Basic/FileManager.cpp
+++ cfe/trunk/lib/Basic/FileManager.cpp
@@ -408,7 +408,7 @@
   bool Changed = FixupRelativePath(Path);
 
   if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size( {
-llvm::sys::fs::make_absolute(Path);
+FS->makeAbsolute(Path);
 Changed = true;
   }
 
Index: cfe/trunk/unittests/Basic/FileManagerTest.cpp
===
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/FileSystemStatCache.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Path.h"
@@ -296,4 +297,30 @@
 
 #endif  // !LLVM_ON_WIN32
 
+TEST_F(FileManagerTest, makeAbsoluteUsesVFS) {
+  SmallString<64> CustomWorkingDir;
+#ifdef LLVM_ON_WIN32
+  CustomWorkingDir = "C:";
+#else
+  CustomWorkingDir = "/";
+#endif
+  llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path");
+
+  auto FS =
+  IntrusiveRefCntPtr(new vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  SmallString<64> Path("a/foo.cpp");
+
+  SmallString<64> ExpectedResult(CustomWorkingDir);
+  llvm::sys::path::append(ExpectedResult, Path);
+
+  ASSERT_TRUE(Manager.makeAbsolutePath(Path));
+  EXPECT_EQ(Path, ExpectedResult);
+}
+
 } // anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r309795 - Use VFS operations in FileManager::makeAbsolutePath.

2017-08-02 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Aug  2 00:25:24 2017
New Revision: 309795

URL: http://llvm.org/viewvc/llvm-project?rev=309795&view=rev
Log:
Use VFS operations in FileManager::makeAbsolutePath.

Summary: It used to call into llvm::sys::fs::make_absolute.

Reviewers: akyrtzi, erikjv, bkramer, krasimir, klimek

Reviewed By: klimek

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=309795&r1=309794&r2=309795&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Wed Aug  2 00:25:24 2017
@@ -408,7 +408,7 @@ bool FileManager::makeAbsolutePath(Small
   bool Changed = FixupRelativePath(Path);
 
   if (!llvm::sys::path::is_absolute(StringRef(Path.data(), Path.size( {
-llvm::sys::fs::make_absolute(Path);
+FS->makeAbsolute(Path);
 Changed = true;
   }
 

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=309795&r1=309794&r2=309795&view=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Wed Aug  2 00:25:24 2017
@@ -10,6 +10,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/FileSystemStatCache.h"
+#include "clang/Basic/VirtualFileSystem.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Path.h"
@@ -296,4 +297,30 @@ TEST_F(FileManagerTest, getVirtualFileWi
 
 #endif  // !LLVM_ON_WIN32
 
+TEST_F(FileManagerTest, makeAbsoluteUsesVFS) {
+  SmallString<64> CustomWorkingDir;
+#ifdef LLVM_ON_WIN32
+  CustomWorkingDir = "C:";
+#else
+  CustomWorkingDir = "/";
+#endif
+  llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path");
+
+  auto FS =
+  IntrusiveRefCntPtr(new vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  SmallString<64> Path("a/foo.cpp");
+
+  SmallString<64> ExpectedResult(CustomWorkingDir);
+  llvm::sys::path::append(ExpectedResult, Path);
+
+  ASSERT_TRUE(Manager.makeAbsolutePath(Path));
+  EXPECT_EQ(Path, ExpectedResult);
+}
+
 } // anonymous namespace


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


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

2017-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.

I just wanted to take a step back and discuss what we want to get from code 
hover in the first place.

I would expect a typical code hover to be a bit more involved and include:

- "kind" of a symbol under hover (i.e. a MACRO, global function, typedef, 
class/struct),
- type of the symbol, if applicable (i.e. types for local vars, typedefs, etc.),
- name of the symbol,
- containing namespace/class information (i.e. "member of namespace std", 
"member of class std::vector"),
- doc comment, if any.

We could start with a subset of those.

This could be implemented by reusing the code of `findDefinitions` that gets 
all `Decl` and `MacroDefinition` instances, referenced in the current location.
When you have a `Decl` or `MacroDefinition`, you could pretty-print the hover 
text.


https://reviews.llvm.org/D35894



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


[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

2017-08-02 Thread Manuel Klimek via Phabricator via cfe-commits
klimek updated this revision to Diff 109287.
klimek marked an inline comment as done.
klimek added a comment.

Address review comment.


https://reviews.llvm.org/D36154

Files:
  clang-tidy/google/StringReferenceMemberCheck.cpp
  clang-tidy/misc/DanglingHandleCheck.cpp
  clang-tidy/misc/InaccurateEraseCheck.cpp
  clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tidy/modernize/MakeSharedCheck.cpp
  clang-tidy/modernize/MakeUniqueCheck.cpp
  clang-tidy/performance/FasterStringFindCheck.cpp
  clang-tidy/performance/InefficientStringConcatenationCheck.cpp
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tidy/readability/RedundantStringCStrCheck.cpp
  clang-tidy/readability/RedundantStringInitCheck.cpp
  include-fixer/find-all-symbols/FindAllSymbols.cpp

Index: include-fixer/find-all-symbols/FindAllSymbols.cpp
===
--- include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -230,7 +230,8 @@
   MatchFinder->addMatcher(
   typeLoc(isExpansionInMainFile(),
   loc(templateSpecializationType(hasDeclaration(
-  classTemplateDecl(has(CXXRecords.bind("use"))),
+  classTemplateSpecializationDecl(hasSpecializedTemplate(
+  classTemplateDecl(has(CXXRecords.bind("use"),
   this);
 }
 
Index: clang-tidy/readability/RedundantStringInitCheck.cpp
===
--- clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -47,7 +47,8 @@
   // string bar("");
   Finder->addMatcher(
   namedDecl(
-  varDecl(hasType(cxxRecordDecl(hasName("basic_string"))),
+  varDecl(hasType(hasUnqualifiedDesugaredType(recordType(
+  hasDeclaration(cxxRecordDecl(hasName("basic_string")),
   hasInitializer(expr(ignoringImplicit(anyOf(
   EmptyStringCtorExpr,
   EmptyStringCtorExprWithTemporaries)))
Index: clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -77,7 +77,8 @@
 return;
 
   // Match expressions of type 'string' or 'string*'.
-  const auto StringDecl = cxxRecordDecl(hasName("::std::basic_string"));
+  const auto StringDecl = type(hasUnqualifiedDesugaredType(recordType(
+  hasDeclaration(cxxRecordDecl(hasName("::std::basic_string"));
   const auto StringExpr =
   expr(anyOf(hasType(StringDecl), hasType(qualType(pointsTo(StringDecl);
 
Index: clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===
--- clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -32,16 +32,18 @@
   if (!getLangOpts().CPlusPlus)
 return;
 
-  const auto ValidContainer = cxxRecordDecl(isSameOrDerivedFrom(
-  namedDecl(
-  has(cxxMethodDecl(
-  isConst(), parameterCountIs(0), isPublic(), hasName("size"),
-  returns(qualType(isInteger(), unless(booleanType()
-  .bind("size")),
-  has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
-hasName("empty"), returns(booleanType()))
-  .bind("empty")))
-  .bind("container")));
+  const auto ValidContainer = qualType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
+  namedDecl(
+  has(cxxMethodDecl(
+  isConst(), parameterCountIs(0), isPublic(),
+  hasName("size"),
+  returns(qualType(isInteger(), unless(booleanType()
+  .bind("size")),
+  has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+hasName("empty"), returns(booleanType()))
+  .bind("empty")))
+  .bind("container")));
 
   const auto WrongUse = anyOf(
   hasParent(binaryOperator(
Index: clang-tidy/performance/InefficientStringConcatenationCheck.cpp
===
--- clang-tidy/performance/InefficientStringConcatenationCheck.cpp
+++ clang-tidy/performance/InefficientStringConcatenationCheck.cpp
@@ -33,7 +33,8 @@
 return;
 
   const auto BasicStringType =
-  hasType(cxxRecordDecl(hasName("::std::basic_string")));
+  hasType(qualType(hasUnqualifiedDesugaredType(recordType(
+  hasDeclaration(cxxRecordDecl(hasName("::std::basic_string")));
 
   const auto BasicStringPlusOperator = cxxOperatorCallExpr(
   hasOve

[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

2017-08-02 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

ptal


https://reviews.llvm.org/D36154



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


[PATCH] D36156: [rename] Introduce symbol occurrences

2017-08-02 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr added a comment.

I was about to suggest SymbolLocation or even SymbolLoc, but it appears to keep 
track of more than locations. "Reference" sounds a little open-ended. No more 
ideas here, I'm afraid.


Repository:
  rL LLVM

https://reviews.llvm.org/D36156



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


[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

2017-08-02 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

looks good


https://reviews.llvm.org/D36154



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


r309799 - [StaticAnalyzer] Fix false positives for unreachable code in macros.

2017-08-02 Thread Daniel Marjamaki via cfe-commits
Author: danielmarjamaki
Date: Wed Aug  2 01:26:56 2017
New Revision: 309799

URL: http://llvm.org/viewvc/llvm-project?rev=309799&view=rev
Log:
[StaticAnalyzer] Fix false positives for unreachable code in macros.

Example:

#define MACRO(C)   if (C) { static int x; .. }
void foo() {
MACRO(0);
}

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


Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
cfe/trunk/test/Analysis/unreachable-code-path.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp?rev=309799&r1=309798&r2=309799&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp Wed Aug  2 
01:26:56 2017
@@ -112,7 +112,7 @@ void UnreachableCodeChecker::checkEndAna
   continue;
 
 // Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
   continue;
 
 // It is good practice to always have a "default" label in a "switch", even

Modified: cfe/trunk/test/Analysis/unreachable-code-path.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unreachable-code-path.c?rev=309799&r1=309798&r2=309799&view=diff
==
--- cfe/trunk/test/Analysis/unreachable-code-path.c (original)
+++ cfe/trunk/test/Analysis/unreachable-code-path.c Wed Aug  2 01:26:56 2017
@@ -213,3 +213,13 @@ void macro(void) {
   RETURN(1); // no-warning
 }
 
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+  if (!C) { \
+static int x;   \
+writeSomething(&x); \
+  }
+void macro2(void) {
+  MACRO(1);
+}


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


[PATCH] D36141: [StaticAnalyzer] Fix FP in UnreachableCodeChecker

2017-08-02 Thread Daniel Marjamäki via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309799: [StaticAnalyzer] Fix false positives for unreachable 
code in macros. (authored by danielmarjamaki).

Changed prior to commit:
  https://reviews.llvm.org/D36141?vs=109086&id=109294#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36141

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
  cfe/trunk/test/Analysis/unreachable-code-path.c


Index: cfe/trunk/test/Analysis/unreachable-code-path.c
===
--- cfe/trunk/test/Analysis/unreachable-code-path.c
+++ cfe/trunk/test/Analysis/unreachable-code-path.c
@@ -213,3 +213,13 @@
   RETURN(1); // no-warning
 }
 
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+  if (!C) { \
+static int x;   \
+writeSomething(&x); \
+  }
+void macro2(void) {
+  MACRO(1);
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -112,7 +112,7 @@
   continue;
 
 // Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
   continue;
 
 // It is good practice to always have a "default" label in a "switch", even


Index: cfe/trunk/test/Analysis/unreachable-code-path.c
===
--- cfe/trunk/test/Analysis/unreachable-code-path.c
+++ cfe/trunk/test/Analysis/unreachable-code-path.c
@@ -213,3 +213,13 @@
   RETURN(1); // no-warning
 }
 
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+  if (!C) { \
+static int x;   \
+writeSomething(&x); \
+  }
+void macro2(void) {
+  MACRO(1);
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -112,7 +112,7 @@
   continue;
 
 // Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
   continue;
 
 // It is good practice to always have a "default" label in a "switch", even
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r309800 - [clangd] Capitalized descriptions of clangd options. NFC.

2017-08-02 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Aug  2 01:53:48 2017
New Revision: 309800

URL: http://llvm.org/viewvc/llvm-project?rev=309800&view=rev
Log:
[clangd] Capitalized descriptions of clangd options. NFC.

To follow the style of other options shown on `clangd -help`.

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=309800&r1=309799&r2=309800&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed Aug  2 01:53:48 2017
@@ -22,12 +22,12 @@ using namespace clang::clangd;
 
 static llvm::cl::opt
 RunSynchronously("run-synchronously",
- llvm::cl::desc("parse on main thread"),
+ llvm::cl::desc("Parse on main thread"),
  llvm::cl::init(false), llvm::cl::Hidden);
 
 static llvm::cl::opt
 ResourceDir("resource-dir",
-llvm::cl::desc("directory for system clang headers"),
+llvm::cl::desc("Directory for system clang headers"),
 llvm::cl::init(""), llvm::cl::Hidden);
 
 int main(int argc, char *argv[]) {


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


[clang-tools-extra] r309801 - [clangd] Run clang-format on all clangd sources. NFC.

2017-08-02 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Aug  2 02:08:39 2017
New Revision: 309801

URL: http://llvm.org/viewvc/llvm-project?rev=309801&view=rev
Log:
[clangd] Run clang-format on all clangd sources. NFC.

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=309801&r1=309800&r2=309801&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Wed Aug  2 02:08:39 2017
@@ -70,7 +70,7 @@ public:
   void onCompletion(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput &Out) override;
   void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
-JSONOutput &Out) override;
+JSONOutput &Out) override;
 
 private:
   ClangdLSPServer &LangServer;
@@ -181,9 +181,11 @@ void ClangdLSPServer::LSPProtocolCallbac
 void ClangdLSPServer::LSPProtocolCallbacks::onCompletion(
 TextDocumentPositionParams Params, StringRef ID, JSONOutput &Out) {
 
-  auto Items = LangServer.Server.codeComplete(
-  Params.textDocument.uri.file,
-  Position{Params.position.line, Params.position.character}).Value;
+  auto Items = LangServer.Server
+   .codeComplete(Params.textDocument.uri.file,
+ Position{Params.position.line,
+  Params.position.character})
+   .Value;
 
   std::string Completions;
   for (const auto &Item : Items) {
@@ -200,9 +202,11 @@ void ClangdLSPServer::LSPProtocolCallbac
 void ClangdLSPServer::LSPProtocolCallbacks::onGoToDefinition(
 TextDocumentPositionParams Params, StringRef ID, JSONOutput &Out) {
 
-  auto Items = LangServer.Server.findDefinitions(
-  Params.textDocument.uri.file,
-  Position{Params.position.line, Params.position.character}).Value;
+  auto Items = LangServer.Server
+   .findDefinitions(Params.textDocument.uri.file,
+Position{Params.position.line,
+ Params.position.character})
+   .Value;
 
   std::string Locations;
   for (const auto &Item : Items) {

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=309801&r1=309800&r2=309801&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Aug  2 02:08:39 2017
@@ -26,8 +26,8 @@ class JSONOutput;
 /// dispatch and ClangdServer together.
 class ClangdLSPServer {
 public:
- ClangdLSPServer(JSONOutput &Out, bool RunSynchronously,
- llvm::Optional ResourceDir);
+  ClangdLSPServer(JSONOutput &Out, bool RunSynchronously,
+  llvm::Optional ResourceDir);
 
   /// Run LSP server loop, receiving input for it from \p In. \p In must be
   /// opened in binary mode. Output will be written using Out variable passed 
to

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=309801&r1=309800&r2=309801&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Aug  2 02:08:39 2017
@@ -878,7 +878,8 @@ CppFile::deferRebuild(StringRef NewConte
 return Diagnostics; // Our rebuild request was cancelled, don't set
 // ASTPromise.
 
-  
That->ASTPromise.set_value(std::make_shared(std::move(NewAST)));
+  That->ASTPromise.set_value(
+  std::make_shared(std::move(NewAST)));
 } // unlock Mutex
 
 return Diagnostics;

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=309801&r1=309800&r2=309801&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Wed Aug  2 02:08:39 2017
@@ -16,11 +16,10 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"

[PATCH] D36156: [rename] Introduce symbol occurrences

2017-08-02 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:32
+/// a macro expansion.
+class SymbolOccurrence {
+public:

I understand the exact vs. non-exact idea, but can you expand a bit on how 
Obj-C code looks where a rename needs to touch more than one location? (I have 
no idea about Obj-C unfortunately)



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:50
+///
+/// The user will have to fixup their code manually after performing such a
+/// rename.

Nit: s/fixup/fix/?



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:65-68
+  ArrayRef getNameLocations() const { return Locations; }
+  ArrayRef getNameLengths() const {
+return llvm::makeArrayRef(NameLengths, Locations.size());
+  }

Any reason not to return ranges instead here?



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:72-73
+  OccurrenceKind Kind;
+  ArrayRef Locations;
+  const unsigned *NameLengths;
+};

Can we store ranges instead?



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:78
+/// unit.
+class SymbolOccurrences {
+public:

Why can't we put all data in SymbolOccurrence and just use a normal container 
here?


Repository:
  rL LLVM

https://reviews.llvm.org/D36156



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


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment.

Some comments:

Currently there is no support in the backend for the interrupt attribute for 
mips64 / using N32 & N64 abis, it will give a fatal error. Previously the 
backend lacked support for the static relocation model which is an expected 
requirement for interrupt handlers.

microMIPS32r(3|6) is only supported for the O32 ABI, as microMIPS64R6 is 
deprecated and going to be removed. There is no support for the published 
microMIPS64R3 ISA.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


[PATCH] D36191: [CodeGen] Don't make availability attributes imply default visibility on macos

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM. Please document this change in the release notes.


https://reviews.llvm.org/D36191



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


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Simon Atanasyan via Phabricator via cfe-commits
atanasyan added a comment.

In https://reviews.llvm.org/D36208#828853, @sdardis wrote:

> Currently there is no support in the backend for the interrupt attribute for 
> mips64 / using N32 & N64 abis, it will give a fatal error. Previously the 
> backend lacked support for the static relocation model which is an expected 
> requirement for interrupt handlers.
>
> microMIPS32r(3|6) is only supported for the O32 ABI, as microMIPS64R6 is 
> deprecated and going to be removed. There is no support for the published 
> microMIPS64R3 ISA.


I see. What is the better solution to handle unsupported attributes: a) 
silently ignore them; b) show a warning; c) show an error?


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


[PATCH] D36112: [ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

That makes sense. It's kinda weird not to report the `null`, but I guess it 
makes sense if the `null` sanitiser is off. It's not actually UB unless it's 
dereferenced, right, so casts are allowed?




Comment at: test/CodeGenCXX/ubsan-type-checks.cpp:73
+// VPTR_NO_NULL-LABEL: define void @_Z13invalid_cast3P3Cat
+void invalid_cast3(Cat *cat = nullptr) {
+  // Fall back to the vptr sanitizer's null check when -fsanitize=null isn't

Can you use reuse the first `invalid_cast` with different checks since the code 
is the same?


https://reviews.llvm.org/D36112



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


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D36208#828881, @atanasyan wrote:

> In https://reviews.llvm.org/D36208#828853, @sdardis wrote:
>
> > Currently there is no support in the backend for the interrupt attribute 
> > for mips64 / using N32 & N64 abis, it will give a fatal error. Previously 
> > the backend lacked support for the static relocation model which is an 
> > expected requirement for interrupt handlers.
> >
> > microMIPS32r(3|6) is only supported for the O32 ABI, as microMIPS64R6 is 
> > deprecated and going to be removed. There is no support for the published 
> > microMIPS64R3 ISA.
>
>
> I see. What is the better solution to handle unsupported attributes: a) 
> silently ignore them; b) show a warning; c) show an error?


If the attribute is semantically critical, an error is reasonable. Otherwise, 
we typically warn the user under the ignored attributes group and then drop the 
attribute from the AST.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:263
+def err_attribute_supported_by_o32: Error<
+  "%0 attribute is supported by o32 ABI only">;
 def warn_arm_interrupt_calling_convention : Warning<

This diagnostic could be more helpful by telling the user how to enable the o32 
ABI.



Comment at: lib/Sema/SemaDeclAttr.cpp:5979-5981
+if (S.Context.getTargetInfo().getABI() != "o32")
+  S.Diag(Attr.getLoc(), diag::err_attribute_supported_by_o32)
+  << Attr.getName() << D->getLocation();

Please do not put logic into the switch statement, but instead sink it into a 
helper function. We hope to remove the boilerplate switch someday, so keeping 
with the `handleFooAttr()` pattern is preferred. Same below.

Further, why is this not handled by a TargetSpecificAttr in Attr.td? See 
`TargetMicrosoftCXXABI` and `MSNoVTable` for an example.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


[PATCH] D36143: [clang-format] Fix indent of 'key <...>' in text protos

2017-08-02 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir updated this revision to Diff 109322.
krasimir added a comment.

- Fix similar issue for 'key {...}'


https://reviews.llvm.org/D36143

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestProto.cpp
  unittests/Format/FormatTestTextProto.cpp


Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -245,6 +245,50 @@
">\n"
"field: OK,\n"
"field_c >>");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "head_id: 1\n"
+   "data ");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "head_id: 1\n"
+   "data \n"
+   "tail_id: 2");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "head_id: 1\n"
+   "data \n"
+   "data {key: value}");
+
+  verifyFormat("app {\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  head_id: 1\n"
+   "  data \n"
+   "}");
+
+  verifyFormat("app: {\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  head_id: 1\n"
+   "  data \n"
+   "}");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "headheadheadheadheadhead_id: 1\n"
+   "product_data {product {1}}");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "headheadheadheadheadhead_id: 1\n"
+   "product_data ");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "headheadheadheadheadhead_id: 1\n"
+   "product_data >");
+
+  verifyFormat("app <\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  headheadheadheadheadhead_id: 1\n"
+   "  product_data \n"
+   ">");
 }
 } // end namespace tooling
 } // end namespace clang
Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -356,6 +356,19 @@
"  }\n"
"  field_g: OK\n"
">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  head_id: 1\n"
+   "  data \n"
+   ">;");
+
+  verifyFormat("option (MyProto.options) = {\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  head_id: 1\n"
+   "  headheadheadheadheadhead_id: 1\n"
+   "  product_data {product {1}}\n"
+   "};");
 }
 
 TEST_F(FormatTestProto, FormatsService) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -731,7 +731,10 @@
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (Current.is(tok::identifier) && Current.Next &&
-  Current.Next->is(TT_DictLiteral))
+  (Current.Next->is(TT_DictLiteral) ||
+   ((Style.Language == FormatStyle::LK_Proto ||
+ Style.Language == FormatStyle::LK_TextProto) &&
+Current.Next->isOneOf(TT_TemplateOpener, tok::l_brace
 return State.Stack.back().Indent;
   if (NextNonComment->is(TT_ObjCStringLiteral) &&
   State.StartOfStringLiteral != 0)


Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -245,6 +245,50 @@
">\n"
"field: OK,\n"
"field_c >>");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "head_id: 1\n"
+   "data ");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "head_id: 1\n"
+   "data \n"
+   "tail_id: 2");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "head_id: 1\n"
+   "data \n"
+   "data {key: value}");
+
+  verifyFormat("app {\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  head_id: 1\n"
+   "  data \n"
+   "}");
+
+  verifyFormat("app: {\n"
+   "  app_id: 'com.javax.swing.salsa.latino'\n"
+   "  head_id: 1\n"
+   "  data \n"
+   "}");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "headheadheadheadheadhead_id: 1\n"
+   "product_data {product {1}}");
+
+  verifyFormat("app_id: 'com.javax.swing.salsa.latino'\n"
+   "headheadhead

[PATCH] D36156: [rename] Introduce symbol occurrences

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:65-68
+  ArrayRef getNameLocations() const { return Locations; }
+  ArrayRef getNameLengths() const {
+return llvm::makeArrayRef(NameLengths, Locations.size());
+  }

klimek wrote:
> Any reason not to return ranges instead here?
I used ranges before, but I found that it was easier to create the atomic 
changes with location+lengths. Should I go back to ranges?



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:72-73
+  OccurrenceKind Kind;
+  ArrayRef Locations;
+  const unsigned *NameLengths;
+};

klimek wrote:
> Can we store ranges instead?
We could, but see above for my current reasoning.


Repository:
  rL LLVM

https://reviews.llvm.org/D36156



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


[PATCH] D36176: [clang-diff] Fix some errors and inconsistencies

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:96
   : TreeImpl(llvm::make_unique(this, Node, AST)) {}
+  SyntaxTree(const SyntaxTree &Tree) = delete;
   ~SyntaxTree();

arphaman wrote:
> It might be better to add a move constructor which would disable copy 
> constructors.
ASTDiff::getMapped internally uses the address of the SyntaxTree that is passed 
as parameter.
So the tree must be exactly the same as the one that is passed to the 
constructor of ASTDiff. 

This is quite bad. I will change ASTDiff::getMapped to accept a boolean or 
split it into two methods
Should I still add a move constructor?


https://reviews.llvm.org/D36176



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


[PATCH] D36176: [clang-diff] Fix some errors and inconsistencies

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:96
   : TreeImpl(llvm::make_unique(this, Node, AST)) {}
+  SyntaxTree(const SyntaxTree &Tree) = delete;
   ~SyntaxTree();

johannes wrote:
> arphaman wrote:
> > It might be better to add a move constructor which would disable copy 
> > constructors.
> ASTDiff::getMapped internally uses the address of the SyntaxTree that is 
> passed as parameter.
> So the tree must be exactly the same as the one that is passed to the 
> constructor of ASTDiff. 
> 
> This is quite bad. I will change ASTDiff::getMapped to accept a boolean or 
> split it into two methods
> Should I still add a move constructor?
Can't you just use the `TreeImpl` pointer value? That should be the same even 
after you've moved `SyntaxTree` with the move constructor.


https://reviews.llvm.org/D36176



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


[PATCH] D36156: [rename] Introduce symbol occurrences

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:65-68
+  ArrayRef getNameLocations() const { return Locations; }
+  ArrayRef getNameLengths() const {
+return llvm::makeArrayRef(NameLengths, Locations.size());
+  }

arphaman wrote:
> klimek wrote:
> > Any reason not to return ranges instead here?
> I used ranges before, but I found that it was easier to create the atomic 
> changes with location+lengths. Should I go back to ranges?
Oops, this is embarrassing! I didn't realize that `AtomicChange` had an 
additional overload that accepted `CharSourceRange`. I'll go back to ranges.


Repository:
  rL LLVM

https://reviews.llvm.org/D36156



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


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment.

I think for the interrupt attribute, it should be an error. Currently it's an 
implementation detail that it errors out in the backend but in principal it can 
be supported (I haven't gotten around to addressing it.)

For the micromips attribute, I believe it should be an error. My rational there 
is that the user has requested something that the compiler cannot perform.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D36208#828955, @sdardis wrote:

> I think for the interrupt attribute, it should be an error. Currently it's an 
> implementation detail that it errors out in the backend but in principal it 
> can be supported (I haven't gotten around to addressing it.)


For the interrupt attribute, I think it should behave the same as the other 
target-specific interrupt attributes unless there's a very good reason to be 
inconsistent.

> For the micromips attribute, I believe it should be an error. My rational 
> there is that the user has requested something that the compiler cannot 
> perform.

That's not really sufficient rationale, because the same logic can be said of 
any ignored attribute. As best I can tell from the docs on this attribute, this 
controls code generation to turn on or off micromips code generation, which 
sounds like the code otherwise generated may still work just fine. e.g., `void 
f(void) __attribute__((micromips)) {}` -- if you compile that code on a 
non-MIPS target, why should it produce an error instead of merely warning the 
user that the attribute is ignored?


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


r309809 - Unify and simplify the behavior of the hasDeclaration matcher.

2017-08-02 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Wed Aug  2 06:04:44 2017
New Revision: 309809

URL: http://llvm.org/viewvc/llvm-project?rev=309809&view=rev
Log:
Unify and simplify the behavior of the hasDeclaration matcher.

Originally, we weren't able to match on Type nodes themselves (only QualType),
so the hasDeclaration matcher was initially written to give what we thought are
reasonable results for QualType matches.

When we chagned the matchers to allow matching on Type nodes, it turned out
that the hasDeclaration matcher was by chance written templated enough to now
allow hasDeclaration to also match on (some) Type nodes.

This patch change the hasDeclaration matcher to:
a) work the same on Type and QualType nodes,
b) be completely explicit about what nodes we can match instead of just allowing
   anything with a getDecl() to match,
c) explicitly control desugaring only one level in very specific instances.
d) adds hasSpecializedTemplate and tagType matchers to allow migrating
  existing use cases that now need more explicit matchers

Note: This patch breaks clang-tools-extra. The corresponding patch there
is approved and will land in a subsequent patch.

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
cfe/trunk/unittests/AST/ASTImporterTest.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=309809&r1=309808&r2=309809&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Wed Aug  2 06:04:44 2017
@@ -1729,6 +1729,21 @@ substTemplateTypeParmType() matches the
 
 
 
+MatcherType>tagTypeMatcherTagType>...
+Matches tag types (record 
and enum types).
+
+Given
+  enum E {};
+  class C {};
+
+  E e;
+  C c;
+
+tagType() matches the type of the variable declarations of both e
+and c.
+
+
+
 MatcherType>templateSpecializationTypeMatcherTemplateSpecializationType>...
 Matches 
template specialization types.
 
@@ -4546,6 +4561,18 @@ functionDecl(hasAnyTemplateArgument(refe
 
 
 
+MatcherClassTemplateSpecializationDecl>hasSpecializedTemplateMatcherClassTemplateDecl>
 InnerMatcher
+Matches the 
specialized template of a specialization declaration.
+
+Given
+  tempalate class A {};
+  typedef A B;
+classTemplateSpecializationDecl(hasSpecializedTemplate(classTemplateDecl()))
+  matches 'B' with classTemplateDecl() matching the class template
+  declaration of 'A'.
+
+
+
 MatcherClassTemplateSpecializationDecl>hasTemplateArgumentunsigned N, 
MatcherTemplateArgument>
 InnerMatcher
 Matches 
classTemplateSpecializations, templateSpecializationType and
 functionDecl where the n'th TemplateArgument matches the given InnerMatcher.

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=309809&r1=309808&r2=309809&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Wed Aug  2 06:04:44 2017
@@ -151,8 +151,33 @@ this section should help get you past th
 AST Matchers
 
 
-...
+The hasDeclaration matcher now works the same for Type and QualType and only
+ever looks through one level of sugaring in a limited number of cases.
 
+There are two main patterns affected by this:
+
+-  qualType(hasDeclaration(recordDecl(...))): previously, we would look through
+   sugar like TypedefType to get at the underlying recordDecl; now, we need
+   to explicitly remove the sugaring:
+   qualType(hasUnqualifiedDesugaredType(hasDeclaration(recordDecl(...
+
+-  hasType(recordDecl(...)): hasType internally uses hasDeclaration; 
previously,
+   this matcher used to match for example TypedefTypes of the RecordType, but
+   after the change they don't; to fix, use:
+
+::
+   hasType(hasUnqualifiedDesugaredType(
+   recordType(hasDeclaration(recordDecl(...)
+
+-  templateSpecializationType(hasDeclaration(classTemplateDecl(...))):
+   previously, we would directly match the underlying Clas

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

2017-08-02 Thread Manuel Klimek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309809: Unify and simplify the behavior of the 
hasDeclaration matcher. (authored by klimek).

Changed prior to commit:
  https://reviews.llvm.org/D27104?vs=108608&id=109325#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27104

Files:
  cfe/trunk/docs/LibASTMatchersReference.html
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
  cfe/trunk/unittests/AST/ASTImporterTest.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -230,6 +230,17 @@
 "Expected TemplateSpecializationType to *not* have a getDecl.");
 }
 
+TEST(HasDeclaration, ElaboratedType) {
+  EXPECT_TRUE(matches(
+  "namespace n { template  struct X {}; }"
+  "void f(n::X);",
+  parmVarDecl(hasType(qualType(hasDeclaration(cxxRecordDecl()));
+  EXPECT_TRUE(matches(
+  "namespace n { template  struct X {}; }"
+  "void f(n::X);",
+  parmVarDecl(hasType(elaboratedType(hasDeclaration(cxxRecordDecl()));
+}
+
 TEST(HasDeclaration, HasDeclarationOfTypeWithDecl) {
   EXPECT_TRUE(matches("typedef int X; X a;",
   varDecl(hasName("a"),
@@ -242,14 +253,27 @@
   EXPECT_TRUE(matches("template  class A {}; A a;",
   varDecl(hasType(templateSpecializationType(
 hasDeclaration(namedDecl(hasName("A";
+  EXPECT_TRUE(matches("template  class A {};"
+  "template  class B { A a; };",
+  fieldDecl(hasType(templateSpecializationType(
+hasDeclaration(namedDecl(hasName("A";
+  EXPECT_TRUE(matches("template  class A {}; A a;",
+  varDecl(hasType(templateSpecializationType(
+  hasDeclaration(cxxRecordDecl()));
 }
 
 TEST(HasDeclaration, HasDeclarationOfCXXNewExpr) {
   EXPECT_TRUE(
   matches("int *A = new int();",
   cxxNewExpr(hasDeclaration(functionDecl(parameterCountIs(1));
 }
 
+TEST(HasDeclaration, HasDeclarationOfTypeAlias) {
+  EXPECT_TRUE(matches("template  using C = T; C c;",
+  varDecl(hasType(templateSpecializationType(
+  hasDeclaration(typeAliasTemplateDecl()));
+}
+
 TEST(HasUnqualifiedDesugaredType, DesugarsUsing) {
   EXPECT_TRUE(
   matches("struct A {}; using B = A; B b;",
@@ -2211,8 +2235,7 @@
functionDecl(hasName("bar"));
 }
 
-TEST(SubstTemplateTypeParmType, HasReplacementType)
-{
+TEST(SubstTemplateTypeParmType, HasReplacementType) {
   std::string Fragment = "template"
  "double F(T t);"
  "int i;"
@@ -2228,5 +2251,13 @@
  substTemplateTypeParmType(hasReplacementType(qualType();
 }
 
+TEST(ClassTemplateSpecializationDecl, HasSpecializedTemplate) {
+  auto Matcher = classTemplateSpecializationDecl(
+  hasSpecializedTemplate(classTemplateDecl()));
+  EXPECT_TRUE(
+  matches("template class A {}; typedef A B;", Matcher));
+  EXPECT_TRUE(notMatches("template class A {};", Matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -252,35 +252,19 @@
 
 TEST(ImportExpr, ImportParenListExpr) {
   MatchVerifier Verifier;
-  EXPECT_TRUE(
-testImport(
-  "template class dummy { void f() { dummy X(*this); } };"
-  "typedef dummy declToImport;"
-  "template class dummy;",
-  Lang_CXX, "", Lang_CXX, Verifier,
-  typedefDecl(
-hasType(
-  templateSpecializationType(
-hasDeclaration(
-  classTemplateDecl(
-hasTemplateDecl(
-  cxxRecordDecl(
-hasMethod(
-allOf(
-  hasName("f"),
-  hasBody(
-compoundStmt(
-  has(
-declStmt(
-  hasSingleDecl(
-varDecl(
-  hasInitializer(
-parenListExpr(
-  has(
-unaryOperator(
-  hasOperatorName("*"),
- 

[PATCH] D36184: [clang-diff] Filter AST nodes

2017-08-02 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment.

Just as a general note: adding cfe-commits after the fact is usually not a good 
idea, as we lose the history of the review in the email list (which is the 
source of truth).


https://reviews.llvm.org/D36184



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


[PATCH] D36156: [rename] Introduce symbol occurrences

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 109326.
arphaman added a comment.

Address review comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D36156

Files:
  include/clang/Tooling/Refactoring/Rename/RenamingAction.h
  include/clang/Tooling/Refactoring/Rename/SymbolName.h
  include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h
  include/clang/Tooling/Refactoring/Rename/USRLocFinder.h
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/Rename/RenamingAction.cpp
  lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
  lib/Tooling/Refactoring/Rename/USRLocFinder.cpp

Index: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -23,6 +23,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Core/Lookup.h"
 #include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolName.h"
 #include "clang/Tooling/Refactoring/Rename/USRFinder.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -68,11 +69,9 @@
 
   // Non-visitors:
 
-  // \brief Returns a list of unique locations. Duplicate or overlapping
-  // locations are erroneous and should be reported!
-  const std::vector &getLocationsFound() const {
-return LocationsFound;
-  }
+  /// \brief Returns a set of unique symbol occurrences. Duplicate or
+  /// overlapping occurrences are erroneous and should be reported!
+  SymbolOccurrences &getOccurrences() { return Occurrences; }
 
 private:
   void checkAndAddLocation(SourceLocation Loc) {
@@ -82,17 +81,18 @@
 StringRef TokenName =
 Lexer::getSourceText(CharSourceRange::getTokenRange(BeginLoc, EndLoc),
  Context.getSourceManager(), Context.getLangOpts());
-size_t Offset = TokenName.find(PrevName);
+size_t Offset = TokenName.find(PrevName.getNamePieces()[0]);
 
 // The token of the source location we find actually has the old
 // name.
 if (Offset != StringRef::npos)
-  LocationsFound.push_back(BeginLoc.getLocWithOffset(Offset));
+  Occurrences.emplace_back(PrevName, SymbolOccurrence::MatchingSymbol,
+   BeginLoc.getLocWithOffset(Offset));
   }
 
   const std::set USRSet;
-  const std::string PrevName;
-  std::vector LocationsFound;
+  const SymbolName PrevName;
+  SymbolOccurrences Occurrences;
   const ASTContext &Context;
 };
 
@@ -391,12 +391,11 @@
 
 } // namespace
 
-std::vector
-getLocationsOfUSRs(const std::vector &USRs, StringRef PrevName,
-   Decl *Decl) {
+SymbolOccurrences getOccurrencesOfUSRs(ArrayRef USRs,
+   StringRef PrevName, Decl *Decl) {
   USRLocFindingASTVisitor Visitor(USRs, PrevName, Decl->getASTContext());
   Visitor.TraverseDecl(Decl);
-  return Visitor.getLocationsFound();
+  return std::move(Visitor.getOccurrences());
 }
 
 std::vector
Index: lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
===
--- /dev/null
+++ lib/Tooling/Refactoring/Rename/SymbolOccurrences.cpp
@@ -0,0 +1,37 @@
+//===--- SymbolOccurrences.cpp - Clang refactoring library ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h"
+#include "clang/Tooling/Refactoring/Rename/SymbolName.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang;
+using namespace tooling;
+
+SymbolOccurrence::SymbolOccurrence(const SymbolName &Name, OccurrenceKind Kind,
+   ArrayRef Locations)
+: Kind(Kind) {
+  ArrayRef NamePieces = Name.getNamePieces();
+  assert(Locations.size() == NamePieces.size() &&
+ "mismatching number of locations and lengths");
+  assert(!Locations.empty() && "no locations");
+  if (Locations.size() == 1) {
+RangeOrNumRanges = SourceRange(
+Locations[0], Locations[0].getLocWithOffset(NamePieces[0].size()));
+return;
+  }
+  MultipleRanges = llvm::make_unique(Locations.size());
+  RangeOrNumRanges.setBegin(
+  SourceLocation::getFromRawEncoding(Locations.size()));
+  for (const auto &Loc : llvm::enumerate(Locations)) {
+MultipleRanges[Loc.index()] = SourceRange(
+Loc.value(),
+Loc.value().getLocWithOffset(NamePieces[Loc.index()].size()));
+  }
+}
Index: lib/Tooling/Refactoring/Rename/RenamingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/RenamingAction.cpp
+++ lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -24,14 +24,52 @@
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling

[clang-tools-extra] r309810 - Adapt clang-tidy checks to changing semantics of hasDeclaration.

2017-08-02 Thread Manuel Klimek via cfe-commits
Author: klimek
Date: Wed Aug  2 06:13:11 2017
New Revision: 309810

URL: http://llvm.org/viewvc/llvm-project?rev=309810&view=rev
Log:
Adapt clang-tidy checks to changing semantics of hasDeclaration.

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

Modified:
clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/InaccurateEraseCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/MakeSharedCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/FasterStringFindCheck.cpp

clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringInitCheck.cpp
clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp?rev=309810&r1=309809&r2=309810&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp 
Wed Aug  2 06:13:11 2017
@@ -27,8 +27,8 @@ void StringReferenceMemberCheck::registe
 return;
 
   // Look for const references to std::string or ::string.
-  auto String = anyOf(recordDecl(hasName("::std::basic_string")),
-  recordDecl(hasName("::string")));
+  auto String = anyOf(namedDecl(hasName("::std::string")),
+  namedDecl(hasName("::string")));
   auto ConstString = qualType(isConstQualified(), hasDeclaration(String));
 
   // Ignore members in template instantiations.

Modified: clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp?rev=309810&r1=309809&r2=309810&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp Wed Aug  2 
06:13:11 2017
@@ -71,11 +71,13 @@ ast_matchers::internal::BindableMatcher<
   // For sequences: assign, push_back, resize.
   cxxMemberCallExpr(
   callee(functionDecl(hasAnyName("assign", "push_back", 
"resize"))),
-  on(expr(hasType(recordDecl(isASequence()),
+  on(expr(hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(recordDecl(isASequence(),
   // For sequences and sets: insert.
-  cxxMemberCallExpr(
-  callee(functionDecl(hasName("insert"))),
-  on(expr(hasType(recordDecl(anyOf(isASequence(), isASet())),
+  cxxMemberCallExpr(callee(functionDecl(hasName("insert"))),
+on(expr(hasType(hasUnqualifiedDesugaredType(
+recordType(hasDeclaration(recordDecl(
+anyOf(isASequence(), isASet()),
   // For maps: operator[].
   cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(isAMap(,
   hasOverloadedOperatorName("[]";
@@ -103,7 +105,8 @@ void DanglingHandleCheck::registerMatche
 
   // Find 'Handle foo(ReturnsAValue());'
   Finder->addMatcher(
-  varDecl(hasType(cxxRecordDecl(IsAHandle)),
+  varDecl(hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(cxxRecordDecl(IsAHandle),
   hasInitializer(
   exprWithCleanups(has(ignoringParenImpCasts(ConvertedHandle)))
   .bind("bad_stmt"))),
@@ -112,7 +115,9 @@ void DanglingHandleCheck::registerMatche
   // Find 'Handle foo = ReturnsAValue();'
   Finder->addMatcher(
   varDecl(
-  hasType(cxxRecordDecl(IsAHandle)), unless(parmVarDecl()),
+  hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(cxxRecordDecl(IsAHandle),
+  unless(parmVarDecl()),
   hasInitializer(exprWithCleanups(has(ignoringParenImpCasts(handleFrom(
   IsAHandle, ConvertedHandle
  .bind("bad_stmt"))),
@@ -139,13 +144,15 @@ void DanglingHandleCheck::registerMatche
   // We have to match both.
   has(ignoringImp

[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

2017-08-02 Thread Manuel Klimek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309810: Adapt clang-tidy checks to changing semantics of 
hasDeclaration. (authored by klimek).

Repository:
  rL LLVM

https://reviews.llvm.org/D36154

Files:
  clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/InaccurateEraseCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/MakeSharedCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/MakeUniqueCheck.cpp
  clang-tools-extra/trunk/clang-tidy/performance/FasterStringFindCheck.cpp
  
clang-tools-extra/trunk/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/RedundantStringInitCheck.cpp
  clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp

Index: clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
===
--- clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ clang-tools-extra/trunk/include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -230,7 +230,8 @@
   MatchFinder->addMatcher(
   typeLoc(isExpansionInMainFile(),
   loc(templateSpecializationType(hasDeclaration(
-  classTemplateDecl(has(CXXRecords.bind("use"))),
+  classTemplateSpecializationDecl(hasSpecializedTemplate(
+  classTemplateDecl(has(CXXRecords.bind("use"),
   this);
 }
 
Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -161,17 +161,18 @@
   // overloaded operator*(). If the operator*() returns by value instead of by
   // reference then the return type is tagged with DerefByValueResultName.
   internal::Matcher TestDerefReturnsByValue =
-  hasType(cxxRecordDecl(hasMethod(allOf(
-  hasOverloadedOperatorName("*"),
-  anyOf(
-  // Tag the return type if it's by value.
-  returns(qualType(unless(hasCanonicalType(referenceType(
-  .bind(DerefByValueResultName)),
-  returns(
-  // Skip loops where the iterator's operator* returns an
-  // rvalue reference. This is just weird.
-  qualType(unless(hasCanonicalType(rValueReferenceType(
-  .bind(DerefByRefResultName)));
+  hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(cxxRecordDecl(hasMethod(allOf(
+  hasOverloadedOperatorName("*"),
+  anyOf(
+  // Tag the return type if it's by value.
+  returns(qualType(unless(hasCanonicalType(referenceType(
+  .bind(DerefByValueResultName)),
+  returns(
+  // Skip loops where the iterator's operator* returns an
+  // rvalue reference. This is just weird.
+  qualType(unless(hasCanonicalType(rValueReferenceType(
+  .bind(DerefByRefResultName));
 
   return forStmt(
  unless(isInTemplateInstantiation()),
@@ -242,16 +243,17 @@
   // functions called begin() and end() taking the container as an argument
   // are also allowed.
   TypeMatcher RecordWithBeginEnd = qualType(anyOf(
-  qualType(isConstQualified(),
-   hasDeclaration(cxxRecordDecl(
-   hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
-   hasMethod(cxxMethodDecl(hasName("end"),
-   isConst() // hasDeclaration
-   ),// qualType
   qualType(
-  unless(isConstQualified()),
-  hasDeclaration(cxxRecordDecl(hasMethod(hasName("begin")),
-   hasMethod(hasName("end") // qualType
+  isConstQualified(),
+  hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
+  hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
+  hasMethod(cxxMethodDecl(hasName("end"),
+  isConst()   // hasDeclaration
+ ))), // qualType
+  qualType(unless(isConstQualified()),
+   hasUnqualifiedDesugaredType(recordType(hasDeclarati

[PATCH] D36217: Fix parsing of <>-style proto options

2017-08-02 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir created this revision.

This patch fixes the parsing of proto option fields like `option op = <...>`.
Previously the parser did not enter the right code path inside the angle braces,
causing the contents to be split into several unwrapped lines inside.


https://reviews.llvm.org/D36217

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestProto.cpp


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -356,6 +356,11 @@
"  }\n"
"  field_g: OK\n"
">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+   "  data1 \n"
+   "  data2 {key2: value2}\n"
+   ">;");
 }
 
 TEST_F(FormatTestProto, FormatsService) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1450,6 +1450,15 @@
   nextToken();
   parseBracedList();
   break;
+case tok::less:
+  if (Style.Language == FormatStyle::LK_Proto) {
+nextToken();
+parseBracedList(/*ContinueOnSemicolons=*/false,
+/*ClosingBraceKind=*/tok::greater);
+  } else {
+nextToken();
+  }
+  break;
 case tok::semi:
   // JavaScript (or more precisely TypeScript) can have semicolons in 
braced
   // lists (in so-called TypeMemberLists). Thus, the semicolon cannot be


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -356,6 +356,11 @@
"  }\n"
"  field_g: OK\n"
">;");
+
+  verifyFormat("option (MyProto.options) = <\n"
+   "  data1 \n"
+   "  data2 {key2: value2}\n"
+   ">;");
 }
 
 TEST_F(FormatTestProto, FormatsService) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1450,6 +1450,15 @@
   nextToken();
   parseBracedList();
   break;
+case tok::less:
+  if (Style.Language == FormatStyle::LK_Proto) {
+nextToken();
+parseBracedList(/*ContinueOnSemicolons=*/false,
+/*ClosingBraceKind=*/tok::greater);
+  } else {
+nextToken();
+  }
+  break;
 case tok::semi:
   // JavaScript (or more precisely TypeScript) can have semicolons in braced
   // lists (in so-called TypeMemberLists). Thus, the semicolon cannot be
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment.

@aaron.ballman I missed your first comments when I'd submitted mine.

In https://reviews.llvm.org/D36208#828957, @aaron.ballman wrote:

> In https://reviews.llvm.org/D36208#828955, @sdardis wrote:
>
> > I think for the interrupt attribute, it should be an error. Currently it's 
> > an implementation detail that it errors out in the backend but in principal 
> > it can be supported (I haven't gotten around to addressing it.)
>
>
> For the interrupt attribute, I think it should behave the same as the other 
> target-specific interrupt attributes unless there's a very good reason to be 
> inconsistent.


My primary thought there was that the backend will error out with "LLVM ERROR: 
"interrupt" attribute is only supported for the O32 ABI on MIPS32R2+ at the 
present time." if the attribute is seen anywhere by the backend for MIPS64. 
Having clang detect this case and being able to point out where in the code 
being compiled the attribute exists is better than having LLVM declaring 
there's an error somewhere and giving up. It is semantically critical as 
interrupt handlers need specific prologue+epilog sequences. I don't object to 
preserving the current behaviour of warning + ignoring it though.

> 
> 
>> For the micromips attribute, I believe it should be an error. My rational 
>> there is that the user has requested something that the compiler cannot 
>> perform.
> 
> That's not really sufficient rationale, because the same logic can be said of 
> any ignored attribute. As best I can tell from the docs on this attribute, 
> this controls code generation to turn on or off micromips code generation, 
> which sounds like the code otherwise generated may still work just fine. 
> e.g., `void f(void) __attribute__((micromips)) {}` -- if you compile that 
> code on a non-MIPS target, why should it produce an error instead of merely 
> warning the user that the attribute is ignored?

I stand corrected, we should just warn + ignore the micromips attribute for 
MIPS64 then.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


[PATCH] D36208: [mips] Enable target-specific attributes for MIPS64

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D36208#829006, @sdardis wrote:

> @aaron.ballman I missed your first comments when I'd submitted mine.
>
> In https://reviews.llvm.org/D36208#828957, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D36208#828955, @sdardis wrote:
> >
> > > I think for the interrupt attribute, it should be an error. Currently 
> > > it's an implementation detail that it errors out in the backend but in 
> > > principal it can be supported (I haven't gotten around to addressing it.)
> >
> >
> > For the interrupt attribute, I think it should behave the same as the other 
> > target-specific interrupt attributes unless there's a very good reason to 
> > be inconsistent.
>
>
> My primary thought there was that the backend will error out with "LLVM 
> ERROR: "interrupt" attribute is only supported for the O32 ABI on MIPS32R2+ 
> at the present time." if the attribute is seen anywhere by the backend for 
> MIPS64.


Having the backend error out is not the best user experience, so I agree that 
Clang should help prevent that. However, it seems just as likely the issue 
should be solved in the backend instead.

> Having clang detect this case and being able to point out where in the code 
> being compiled the attribute exists is better than having LLVM declaring 
> there's an error somewhere and giving up. It is semantically critical as 
> interrupt handlers need specific prologue+epilog sequences. I don't object to 
> preserving the current behaviour of warning + ignoring it though.

I agree that Clang should definitely let the user know what's going on rather 
than leaving it to a backend error, but I think the behavior where we warn that 
the attribute is ignored and drop it is the best approach. I believe that if we 
gate the attribute on the ABI, that should happen automatically.


Repository:
  rL LLVM

https://reviews.llvm.org/D36208



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


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

2017-08-02 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment.

In https://reviews.llvm.org/D35894#828702, @ilya-biryukov wrote:

> I just wanted to take a step back and discuss what we want to get from code 
> hover in the first place.
>
> I would expect a typical code hover to be a bit more involved and include:
>
> - "kind" of a symbol under hover (i.e. a MACRO, global function, typedef, 
> class/struct),
> - type of the symbol, if applicable (i.e. types for local vars, typedefs, 
> etc.),
> - name of the symbol,
> - containing namespace/class information (i.e. "member of namespace std", 
> "member of class std::vector"),
> - doc comment, if any.
>
>   We could start with a subset of those.


I think all of those would be great. Our objective is to bring basic but 
correct features that will put us close to parity with Eclipse CDT, so that our 
users can transition. In CDT only the "raw" source is shown, similar to this 
patch (except in CDT it attaches comment nodes so you get the few lines of 
comments before). I think priority wise, we would likely want to tackle other 
LSP features first before adding more things to the hover, (References, Open 
Definition, etc).
I do find that just showing the raw code is lacking in many situations for 
example I have found myself having to figure out which namespace a symbol was 
in by hand so it's very much desirable to add more to the hover.


https://reviews.llvm.org/D35894



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


r309813 - [rename] NFC, extract symbol canonicalization logic into function

2017-08-02 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Wed Aug  2 07:15:27 2017
New Revision: 309813

URL: http://llvm.org/viewvc/llvm-project?rev=309813&view=rev
Log:
[rename] NFC, extract symbol canonicalization logic into function

This function will be used by the clang-refactor's rename actions

Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Rename/USRFindingAction.h
cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp

Modified: cfe/trunk/include/clang/Tooling/Refactoring/Rename/USRFindingAction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Rename/USRFindingAction.h?rev=309813&r1=309812&r2=309813&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/Rename/USRFindingAction.h 
(original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Rename/USRFindingAction.h Wed 
Aug  2 07:15:27 2017
@@ -28,6 +28,15 @@ class NamedDecl;
 
 namespace tooling {
 
+/// Returns the canonical declaration that best represents a symbol that can be
+/// renamed.
+///
+/// The following canonicalization rules are currently used:
+///
+/// - A constructor is canonicalized to its class.
+/// - A destructor is canonicalized to its class.
+const NamedDecl *getCanonicalSymbolDeclaration(const NamedDecl *FoundDecl);
+
 struct USRFindingAction {
   USRFindingAction(ArrayRef SymbolOffsets,
ArrayRef QualifiedNames, bool Force)

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp?rev=309813&r1=309812&r2=309813&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp Wed Aug  2 
07:15:27 2017
@@ -39,6 +39,21 @@ using namespace llvm;
 namespace clang {
 namespace tooling {
 
+const NamedDecl *getCanonicalSymbolDeclaration(const NamedDecl *FoundDecl) {
+  // If FoundDecl is a constructor or destructor, we want to instead take
+  // the Decl of the corresponding class.
+  if (const auto *CtorDecl = dyn_cast(FoundDecl))
+FoundDecl = CtorDecl->getParent();
+  else if (const auto *DtorDecl = dyn_cast(FoundDecl))
+FoundDecl = DtorDecl->getParent();
+  // FIXME: (Alex L): Canonicalize implicit template instantions, just like
+  // the indexer does it.
+
+  // Note: please update the declaration's doc comment every time the
+  // canonicalization rules are changed.
+  return FoundDecl;
+}
+
 namespace {
 // \brief NamedDeclFindingConsumer should delegate finding USRs of given Decl 
to
 // AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
@@ -193,13 +208,7 @@ private:
   return false;
 }
 
-// If FoundDecl is a constructor or destructor, we want to instead take
-// the Decl of the corresponding class.
-if (const auto *CtorDecl = dyn_cast(FoundDecl))
-  FoundDecl = CtorDecl->getParent();
-else if (const auto *DtorDecl = dyn_cast(FoundDecl))
-  FoundDecl = DtorDecl->getParent();
-
+FoundDecl = getCanonicalSymbolDeclaration(FoundDecl);
 SpellingNames.push_back(FoundDecl->getNameAsString());
 AdditionalUSRFinder Finder(FoundDecl, Context);
 USRList.push_back(Finder.Find());


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


[PATCH] D33676: Place implictly declared functions at block scope

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Parse/ParseCXXInlineMethods.cpp:521-522
   // to be re-used for method bodies as well.
-  ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope);
+  ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope|
+   Scope::CompoundStmtScope);
   Actions.ActOnStartOfFunctionDef(getCurScope(), LM.D);

Did clang-format produce this? The lack of spaces between the `|` operators is 
surprising. Same comment applies elsewhere, so you might want to run 
clang-format over the patch if you've not already done so.


https://reviews.llvm.org/D33676



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


[PATCH] D36083: [utils] Add a script that runs clang in LLDB and stops it when a specified diagnostic is emitted

2017-08-02 Thread don hinton via Phabricator via cfe-commits
hintonda added a comment.

In https://reviews.llvm.org/D36083#827745, @arphaman wrote:

> Makes sense. I'll see if I can get somewhere with the regex idea.


Btw, I created a quick prototype with sed and found that the diag strings 
aren't unique -- which isn't surprising since there's no requirement for that.  
In fact, 23 of them are just "%0".

I suppose you could add a breakpoint for multiple values.


Repository:
  rL LLVM

https://reviews.llvm.org/D36083



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


r309815 - Define _GNU_SOURCE for RTEMS c++

2017-08-02 Thread Walter Lee via cfe-commits
Author: waltl
Date: Wed Aug  2 07:36:52 2017
New Revision: 309815

URL: http://llvm.org/viewvc/llvm-project?rev=309815&view=rev
Log:
Define _GNU_SOURCE for RTEMS c++

Summary: This is required by the libc++ locale support.

Reviewers: jyknight

Subscribers: fedor.sergeev

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

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=309815&r1=309814&r2=309815&view=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Wed Aug  2 07:36:52 2017
@@ -505,6 +505,8 @@ protected:
 
 Builder.defineMacro("__rtems__");
 Builder.defineMacro("__ELF__");
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
   }
 
 public:

Modified: cfe/trunk/test/Preprocessor/init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=309815&r1=309814&r2=309815&view=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Wed Aug  2 07:36:52 2017
@@ -8955,6 +8955,7 @@
 // KFREEBSDI686-DEFINE:#define __GLIBC__ 1
 //
 // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM 
< /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
+// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix GNUSOURCE %s
 // GNUSOURCE:#define _GNU_SOURCE 1
 //
 // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix NORTTI %s


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


[PATCH] D36200: [Sema] Improve some -Wunguarded-availability diagnostic messages

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

This needs a test for the fixits as well, see test/FixIt/fixit-availability*


https://reviews.llvm.org/D36200



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


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

2017-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

> I think all of those would be great. Our objective is to bring basic but 
> correct features that will put us close to parity with Eclipse CDT, so that 
> our users can transition. In CDT only the "raw" source is shown, similar to 
> this patch (except in CDT it attaches comment nodes so you get the few lines 
> of comments before). I think priority wise, we would likely want to tackle 
> other LSP features first before adding more things to the hover, (References, 
> Open Definition, etc).

Don't get me wrong, I'm not trying to get all of what I described in the first 
CL, but I definitely think that's the experience we want from hovers and we 
should lay out foundation to make it happen.
I.e. here is a meta-description of algorithm for hover that is easy to 
implement and extend later:

1. Resolve reference under cursor. Result: `Decl` or `MacroDefinition` that the 
symbol under cursor refers to (possibly a list of results).
  - This part should be reused between `findDefinitions` and hovers. It's 
already there, we just need to extract an interface that provides `Decl` and 
`MacroDefinition` instead of their `Range`.
2. Transform result of step 1 (`Decl` or `MacroDefinition`) into a hover 
description (i.e. into a MarkedString).
  - This part can be super-simple in the first commit, i.e. it may only output 
a source code of declaration and a symbol kind (i.e. local variable, parameter, 
class, macro, etc). That could be easily extended in the future.

> I do find that just showing the raw code is lacking in many situations for 
> example I have found myself having to figure out which namespace a symbol was 
> in by hand so it's very much desirable to add more to the hover.

Sure, even though showing source code is still fine most of the time.
I also wanted to stress that we shouldn't too much. Let's not show the body of 
the function we're referring to as it may be very large and it's irrelevant 
most of the time. If a user really wants to see definition, he can jump to it 
using `findDefinitions`.


https://reviews.llvm.org/D35894



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


[libclc] r309820 - configure.py: Make python3 friendly

2017-08-02 Thread Jan Vesely via cfe-commits
Author: jvesely
Date: Wed Aug  2 08:00:59 2017
New Revision: 309820

URL: http://llvm.org/viewvc/llvm-project?rev=309820&view=rev
Log:
configure.py: Make python3 friendly

mostly prints and exceptions.
Few behavioral changes are documented in the text
Generated Makefile is identical between python2 and python3

Signed-off-by: Jan Vesely 
Reviewed-by: Aaron Watry 

Modified:
libclc/trunk/build/metabuild.py
libclc/trunk/configure.py

Modified: libclc/trunk/build/metabuild.py
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/build/metabuild.py?rev=309820&r1=309819&r2=309820&view=diff
==
--- libclc/trunk/build/metabuild.py (original)
+++ libclc/trunk/build/metabuild.py Wed Aug  2 08:00:59 2017
@@ -97,4 +97,4 @@ def from_name(name):
 return Make()
   if name == 'ninja':
 return Ninja()
-  raise LookupError, 'unknown generator: %s; supported generators are make and 
ninja' % name
+  raise LookupError('unknown generator: %s; supported generators are make and 
ninja' % name)

Modified: libclc/trunk/configure.py
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/configure.py?rev=309820&r1=309819&r2=309820&view=diff
==
--- libclc/trunk/configure.py (original)
+++ libclc/trunk/configure.py Wed Aug  2 08:00:59 2017
@@ -1,4 +1,11 @@
 #!/usr/bin/python
+from __future__ import print_function
+
+# We only need this for int() cast, which works by default in python 2
+try:
+from builtins import int
+except:
+pass
 
 def c_compiler_rule(b, name, description, compiler, flags):
   command = "%s -MMD -MF $out.d %s -c -o $out $in" % (compiler, flags)
@@ -58,19 +65,21 @@ if not pkgconfigdir:
 
 def llvm_config(args):
   try:
-proc = Popen([llvm_config_exe] + args, stdout=PIPE)
+# Universal newlines translate different newline formats to '\n'
+# it also force the input to be string instead of bytes in python 3
+proc = Popen([llvm_config_exe] + args, stdout=PIPE, 
universal_newlines=True)
 return proc.communicate()[0].rstrip().replace('\n', ' ')
   except OSError:
-print "Error executing llvm-config."
-print "Please ensure that llvm-config is in your $PATH, or use 
--with-llvm-config."
+print("Error executing llvm-config.")
+print("Please ensure that llvm-config is in your $PATH, or use 
--with-llvm-config.")
 sys.exit(1)
 
-llvm_version = string.split(string.replace(llvm_config(['--version']), 'svn', 
''), '.')
+llvm_version = llvm_config(['--version']).replace('svn', '').split('.')
 llvm_int_version = int(llvm_version[0]) * 100 + int(llvm_version[1]) * 10
 llvm_string_version = 'LLVM' + llvm_version[0] + '.' + llvm_version[1]
 
 if llvm_int_version < 400:
-print "libclc requires LLVM >= 4.0"
+print("libclc requires LLVM >= 4.0")
 sys.exit(1)
 
 llvm_system_libs = llvm_config(['--system-libs'])
@@ -180,6 +189,10 @@ for target in targets:
   libdirs = filter(lambda d: os.path.isfile(os.path.join(d, 'SOURCES')),
[os.path.join(srcdir, subdir, 'lib') for subdir in subdirs])
 
+  # The above are iterables in python3 but we might use them multiple times
+  # if more then one device is supported.
+  incdirs = list(incdirs)
+  libdirs = list(libdirs)
   clang_cl_includes = ' '.join(["-I%s" % incdir for incdir in incdirs])
 
   for device in available_targets[target]['devices']:


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


[PATCH] D31814: Add option to add command-line options and version info to output file

2017-08-02 Thread Christof Douma via Phabricator via cfe-commits
christof commandeered this revision.
christof added a reviewer: salari01.
christof added a comment.
Herald added a subscriber: aheejin.

With the ok of the author, I'll try to get some movement on this work again.


https://reviews.llvm.org/D31814



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


[PATCH] D33676: Place implictly declared functions at block scope

2017-08-02 Thread Momchil Velikov via Phabricator via cfe-commits
chill added inline comments.



Comment at: lib/Parse/ParseCXXInlineMethods.cpp:521-522
   // to be re-used for method bodies as well.
-  ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope);
+  ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope|
+   Scope::CompoundStmtScope);
   Actions.ActOnStartOfFunctionDef(getCurScope(), LM.D);

aaron.ballman wrote:
> Did clang-format produce this? The lack of spaces between the `|` operators 
> is surprising. Same comment applies elsewhere, so you might want to run 
> clang-format over the patch if you've not already done so.
No, I wrote it manually like this to match the already existing style. Will be 
glad to run clang-format on all the touched files, though.


https://reviews.llvm.org/D33676



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


Lab is down

2017-08-02 Thread Victor Leschuk via cfe-commits
Hello all, buildbot is currently down due to hardware failure. We are
currently working on it. Sorry for the inconvenience.

-- 
Best Regards,
Victor

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


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

2017-08-02 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment.

In https://reviews.llvm.org/D35894#829109, @ilya-biryukov wrote:

> > I think all of those would be great. Our objective is to bring basic but 
> > correct features that will put us close to parity with Eclipse CDT, so that 
> > our users can transition. In CDT only the "raw" source is shown, similar to 
> > this patch (except in CDT it attaches comment nodes so you get the few 
> > lines of comments before). I think priority wise, we would likely want to 
> > tackle other LSP features first before adding more things to the hover, 
> > (References, Open Definition, etc).
>
> Don't get me wrong, I'm not trying to get all of what I described in the 
> first CL, but I definitely think that's the experience we want from hovers 
> and we should lay out foundation to make it happen.
>  I.e. here is a meta-description of algorithm for hover that is easy to 
> implement and extend later:
>
> 1. Resolve reference under cursor. Result: `Decl` or `MacroDefinition` that 
> the symbol under cursor refers to (possibly a list of results).
>   - This part should be reused between `findDefinitions` and hovers. It's 
> already there, we just need to extract an interface that provides `Decl` and 
> `MacroDefinition` instead of their `Range`.
> 2. Transform result of step 1 (`Decl` or `MacroDefinition`) into a hover 
> description (i.e. into a MarkedString).
>   - This part can be super-simple in the first commit, i.e. it may only 
> output a source code of declaration and a symbol kind (i.e. local variable, 
> parameter, class, macro, etc). That could be easily extended in the future.
>
> > I do find that just showing the raw code is lacking in many situations for 
> > example I have found myself having to figure out which namespace a symbol 
> > was in by hand so it's very much desirable to add more to the hover.
>
> Sure, even though showing source code is still fine most of the time.
>  I also wanted to stress that we shouldn't too much. Let's not show the body 
> of the function we're referring to as it may be very large and it's 
> irrelevant most of the time. If a user really wants to see definition, he can 
> jump to it using `findDefinitions`.


As a user I want to be able to peek at the code without changing context, not 
all the time though for sure. Eclipse actually has two hovers: a "semantic" one 
which in Java includes the package, type, visibility, javadoc, etc, this is 
what you describe and the default in Java (and it should be for C++ if it 
existed!). And there is also a source hover which just outputs the source code 
of that symbols, which you enable by holding shift (the only one implemented in 
the case of C++). The objective is to have both in Clangd but the latter is 
much more low hanging. Whether of not the body of a function is large doesn't 
matter because clients either crop it or add scollbars to the hover (Eclipse, 
VS Code, Theia).


https://reviews.llvm.org/D35894



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


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

2017-08-02 Thread Marc-Andre Laperle via Phabricator via cfe-commits
malaperle added a comment.

@Nebiroth I think it's OK to put this on hold until we make the "semantic" 
hover and figure out how to have both. From our perspective, this is going 
beyond parity of what we had before but it's seems like the right thing to do.


https://reviews.llvm.org/D35894



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


Re: Lab is down

2017-08-02 Thread Victor Leschuk via cfe-commits
There is power outage in the area where the servers are located. They
say the power will be restored at 10:45 AM PDT, I'd estimate buildbot
online time about ~12:00.


On 08/02/2017 06:31 PM, Victor Leschuk wrote:
> Hello all, buildbot is currently down due to hardware failure. We are
> currently working on it. Sorry for the inconvenience.
>

-- 
Best Regards,
Victor

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


Re: Lab is down

2017-08-02 Thread Victor Leschuk via cfe-commits
Buildbot is up and running. Sorry for the delay.


On 08/02/2017 06:31 PM, Victor Leschuk wrote:
> Hello all, buildbot is currently down due to hardware failure. We are
> currently working on it. Sorry for the inconvenience.
>

-- 
Best Regards,
Victor

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


[PATCH] D36191: [CodeGen] Don't make availability attributes imply default visibility on macos

2017-08-02 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Awesome, thanks! Maybe mention the PR# for this in the commit message.


https://reviews.llvm.org/D36191



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


Re: [PATCH] D36202: [Driver] Disable static C++ library support on Fuchsia

2017-08-02 Thread Nico Weber via cfe-commits
Should clang warn if you request static libc++ on fuchsia? It now silently
ignores the flag, right?

On Aug 1, 2017 9:18 PM, "Petr Hosek via Phabricator via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

This revision was automatically updated to reflect the committed changes.
Closed by commit rL309778: [Driver] Disable static C++ library support on
Fuchsia (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D36202?vs=109255&id=109258#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36202

Files:
  cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
  cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
  cfe/trunk/test/Driver/fuchsia.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
@@ -107,15 +107,8 @@
   CmdArgs.push_back("-Bdynamic");

 if (D.CCCIsCXX()) {
-  if (ToolChain.ShouldLinkCXXStdlib(Args)) {
-bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx)
&&
-   !Args.hasArg(options::OPT_static);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bstatic");
+  if (ToolChain.ShouldLinkCXXStdlib(Args))
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
-if (OnlyLibstdcxxStatic)
-  CmdArgs.push_back("-Bdynamic");
-  }
   CmdArgs.push_back("-lm");
 }

Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -31,6 +31,7 @@
   set(BUILTINS_${target}-fuchsia_CMAKE_SYSROOT
${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
   set(BUILTINS_${target}-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
 endforeach()
+
 if(NOT APPLE)
   list(APPEND LLVM_BUILTIN_TARGETS "default")
 endif()
@@ -45,8 +46,10 @@
   set(RUNTIMES_${target}-fuchsia_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL
"")
   set(RUNTIMES_${target}-fuchsia_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL
"")
   set(RUNTIMES_${target}-fuchsia_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL
"")
+  set(RUNTIMES_${target}-fuchsia_LIBCXXABI_ENABLE_STATIC OFF CACHE BOOL "")
   set(RUNTIMES_${target}-fuchsia_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
   set(RUNTIMES_${target}-fuchsia_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}-fuchsia_LIBCXX_ENABLE_STATIC OFF CACHE BOOL "")
 endforeach()

 # Setup toolchain.
Index: cfe/trunk/test/Driver/fuchsia.cpp
===
--- cfe/trunk/test/Driver/fuchsia.cpp
+++ cfe/trunk/test/Driver/fuchsia.cpp
@@ -28,8 +28,8 @@

 // RUN: %clangxx %s -### --target=x86_64-unknown-fuchsia -static-libstdc++
2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-STATIC
-// CHECK-STATIC: "-Bstatic"
+// CHECK-STATIC-NOT: "-Bstatic"
 // CHECK-STATIC: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-STATIC: "-Bdynamic"
+// CHECK-STATIC-NOT: "-Bdynamic"
 // CHECK-STATIC: "-lm"
 // CHECK-STATIC: "-lc"



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


[PATCH] D33676: Place implictly declared functions at block scope

2017-08-02 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.

This looks reasonable to me, once it's been formatted. Let's give @rsmith a few 
days to comment before committing, though.


https://reviews.llvm.org/D33676



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


[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

2017-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.

Allows to have multiple instances of RealFileSystem that have
different working directories.


https://reviews.llvm.org/D36226

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -12,6 +12,7 @@
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 #include 
@@ -477,6 +478,75 @@
 }
 #endif
 
+TEST(VirtualFileSystemTest, ThreadFriendlyRealFSWorkingDirTest) {
+  ScopedDir TestDirectory("thread-friendly-vfs-test", /*Unique*/ true);
+
+  SmallString<128> PathA = TestDirectory.Path;
+  llvm::sys::path::append(PathA, "a");
+  SmallString<128> PathAC = PathA;
+  llvm::sys::path::append(PathAC, "c");
+  SmallString<128> PathB = TestDirectory.Path;
+  llvm::sys::path::append(PathB, "b");
+  SmallString<128> PathBD = PathB;
+  llvm::sys::path::append(PathBD, "d");
+
+  ScopedDir A(PathA);
+  ScopedDir AC(PathAC);
+  ScopedDir B(PathB);
+  ScopedDir BD(PathBD);
+
+  IntrusiveRefCntPtr FSA = vfs::createThreadFriendlyRealFS();
+  // setCurrentWorkingDirectory should fininsh without error
+  ASSERT_TRUE(!FSA->setCurrentWorkingDirectory(Twine(A)));
+
+  IntrusiveRefCntPtr FSB = vfs::createThreadFriendlyRealFS();
+  // setCurrentWorkingDirectory should fininsh without error
+  ASSERT_TRUE(!FSB->setCurrentWorkingDirectory(Twine(B)));
+
+  ASSERT_TRUE(FSA->getCurrentWorkingDirectory());
+  EXPECT_EQ(*FSA->getCurrentWorkingDirectory(), StringRef(A));
+
+  ASSERT_TRUE(FSB->getCurrentWorkingDirectory());
+  EXPECT_EQ(*FSB->getCurrentWorkingDirectory(), StringRef(B));
+
+  auto C = FSA->status("c");
+  // a/c should be found in FSA
+  ASSERT_TRUE(!!C);
+  // name of 'c' must be relative
+  EXPECT_EQ(C->getName(), "c");
+  EXPECT_TRUE(C->isDirectory());
+
+  // "a", "b" and "d" should not be found in FSA.
+  EXPECT_FALSE(!!FSA->status("a"));
+  EXPECT_FALSE(!!FSA->status("b"));
+  EXPECT_FALSE(!!FSA->status("d"));
+
+  auto D = FSB->status("d");
+  // b/d should be found in FSB
+  ASSERT_TRUE(!!D);
+  // name of 'd' must be relative
+  EXPECT_EQ(D->getName(), "d");
+  EXPECT_TRUE(D->isDirectory());
+
+  // "a", "b" and "c" should not be found in FSB.
+  EXPECT_FALSE(!!FSB->status("a"));
+  EXPECT_FALSE(!!FSB->status("b"));
+  EXPECT_FALSE(!!FSB->status("c"));
+
+  SmallString<128> RelPathC = StringRef("..");
+  llvm::sys::path::append(RelPathC, "a", "c");
+  auto RelCFromFSA = FSA->status(RelPathC);
+  ASSERT_TRUE(!!RelCFromFSA);
+  EXPECT_EQ(RelCFromFSA->getName(), StringRef(RelPathC));
+
+  auto RelCFromFSB = FSB->status(RelPathC);
+  ASSERT_TRUE(!!RelCFromFSB);
+  EXPECT_EQ(RelCFromFSB->getName(), StringRef(RelPathC));
+
+  EXPECT_TRUE(C->equivalent(*RelCFromFSA));
+  EXPECT_TRUE(C->equivalent(*RelCFromFSB));
+}
+
 template 
 static void checkContents(DirIter I, ArrayRef ExpectedOut) {
   std::error_code EC;
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -136,6 +136,7 @@
   Status S;
   std::string RealName;
   friend class RealFileSystem;
+  friend class ThreadFriendlyRealFileSystem;
   RealFile(int FD, StringRef NewName, StringRef NewRealPathName)
   : FD(FD), S(NewName, {}, {}, {}, {}, {},
   llvm::sys::fs::file_type::status_error, {}),
@@ -239,7 +240,9 @@
 
 namespace {
 class RealFSDirIter : public clang::vfs::detail::DirIterImpl {
+protected:
   llvm::sys::fs::directory_iterator Iter;
+
 public:
   RealFSDirIter(const Twine &Path, std::error_code &EC) : Iter(Path, EC) {
 if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
@@ -271,6 +274,131 @@
   return directory_iterator(std::make_shared(Dir, EC));
 }
 
+//===---===/
+// ThreadFriendlyRealFileSystem implementation
+//===---===/
+
+namespace {
+/// \brief A thread-friendly version of RealFileSystem. Does not call
+/// llvm::sys::fs::set_current_path, stores CurrentWorkingDirectory as a field
+/// instead. This class is not thread-safe, though, it merely gets rid of
+/// manipulating global state.
+class ThreadFriendlyRealFileSystem : public RealFileSystem {
+public:
+  ThreadFriendlyRealFileSystem()
+  : CurrentWorkingDir(RealFileSystem::getCurrentWorkingDirectory()) {}
+
+  ErrorOr status(const Twine &Path) override;
+  ErrorOr> openFileForRead(const Twine &Path) override;
+  directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
+
+  llvm::ErrorOr getCurrentWorkingDirectory() const o

[PATCH] D36226: Added a RealFileSystem implementation that does not rely on global CWD.

2017-08-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Also tested by temporarily replacing `getRealFileSystem()` body with `return 
createThreadFriendlyRealFS();` and running `check-all`, all tests passed.
To be more specific, there was one crash in clang-format, as there is a piece 
of code that uses raw pointer from `getRealFileSystem` instead of ref-counted 
(i.e. it calls `getRealFileSystem().get()`). After working around that, all 
tests passed.


https://reviews.llvm.org/D36226



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


[PATCH] D33676: Place implictly declared functions at block scope

2017-08-02 Thread Momchil Velikov via Phabricator via cfe-commits
chill updated this revision to Diff 109366.
chill added a comment.

Updated with only whitespace changes after formating each changed line with 
clang-format.


https://reviews.llvm.org/D33676

Files:
  include/clang/Sema/Scope.h
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/ParsePragma.cpp
  lib/Parse/ParseStmt.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  test/Sema/implicit-decl-c90.c
  test/Sema/implicit-decl.c

Index: test/Sema/implicit-decl.c
===
--- test/Sema/implicit-decl.c
+++ test/Sema/implicit-decl.c
@@ -9,16 +9,15 @@
int32_t *vector[16];
const char compDesc[16 + 1];
int32_t compCount = 0;
-   if (_CFCalendarDecomposeAbsoluteTimeV(compDesc, vector, compCount)) { // expected-note {{previous implicit declaration is here}} \
- expected-error {{implicit declaration of function '_CFCalendarDecomposeAbsoluteTimeV' is invalid in C99}}
+   if (_CFCalendarDecomposeAbsoluteTimeV(compDesc, vector, compCount)) { // expected-error {{implicit declaration of function '_CFCalendarDecomposeAbsoluteTimeV' is invalid in C99}}
}
 
printg("Hello, World!\n"); // expected-error{{implicit declaration of function 'printg' is invalid in C99}} \
   // expected-note{{did you mean 'printf'?}}
 
   __builtin_is_les(1, 3); // expected-error{{use of unknown builtin '__builtin_is_les'}}
 }
-Boolean _CFCalendarDecomposeAbsoluteTimeV(const char *componentDesc, int32_t **vector, int32_t count) { // expected-error{{conflicting types for '_CFCalendarDecomposeAbsoluteTimeV'}}
+Boolean _CFCalendarDecomposeAbsoluteTimeV(const char *componentDesc, int32_t **vector, int32_t count) {
  return 0;
 }
 
Index: test/Sema/implicit-decl-c90.c
===
--- /dev/null
+++ test/Sema/implicit-decl-c90.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 %s -std=c90 -verify -fsyntax-only
+void t0(int x) {
+  int (*p)();
+  if(x > 0)
+x = g() + 1;
+  p = g;
+  if(x < 0) {
+extern void u(int (*)[h()]);
+int (*q)() = h;
+  }
+  p = h; /* expected-error {{use of undeclared identifier 'h'}} */
+}
+
+void t1(int x) {
+  int (*p)();
+  switch (x) {
+g();
+  case 0:
+x = h() + 1;
+break;
+  case 1:
+p = g;
+p = h;
+break;
+  }
+  p = g; /* expected-error {{use of undeclared identifier 'g'}} */
+  p = h; /* expected-error {{use of undeclared identifier 'h'}} */
+}
+
+int t2(int x) {
+  int y = ({ if (x > 0) x = g() + 1; 2*x; });
+  int (*p)() = g; /* expected-error {{use of undeclared identifier 'g'}} */
+  return y;
+}
+
+int (*p)() = g; /* expected-error {{use of undeclared identifier 'g'}} */
+int (*q)() = h; /* expected-error {{use of undeclared identifier 'h'}} */
+
+float g(); /* not expecting conflicting types diagnostics here */
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12677,12 +12677,16 @@
 SourceLocation());
   D.SetIdentifier(&II, Loc);
 
-  // Insert this function into translation-unit scope.
+  // Insert this function into the enclosing block scope.
+  while (S && !S->isCompoundStmtScope())
+S = S->getParent();
+  if (S == nullptr)
+S = TUScope;
 
   DeclContext *PrevDC = CurContext;
   CurContext = Context.getTranslationUnitDecl();
 
-  FunctionDecl *FD = cast(ActOnDeclarator(TUScope, D));
+  FunctionDecl *FD = cast(ActOnDeclarator(S, D));
   FD->setImplicit();
 
   CurContext = PrevDC;
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -1075,8 +1075,9 @@
   TemplateInfo.Kind == ParsedTemplateInfo::Template &&
   Actions.canDelayFunctionBody(D)) {
 MultiTemplateParamsArg TemplateParameterLists(*TemplateInfo.TemplateParams);
-
-ParseScope BodyScope(this, Scope::FnScope|Scope::DeclScope);
+
+ParseScope BodyScope(this, Scope::FnScope | Scope::DeclScope |
+   Scope::CompoundStmtScope);
 Scope *ParentScope = getCurScope()->getParent();
 
 D.setFunctionDefinitionKind(FDK_Definition);
@@ -1106,7 +1107,8 @@
(Tok.is(tok::l_brace) || Tok.is(tok::kw_try) ||
 Tok.is(tok::colon)) && 
   Actions.CurContext->isTranslationUnit()) {
-ParseScope BodyScope(this, Scope::FnScope|Scope::DeclScope);
+ParseScope BodyScope(this, Scope::FnScope | Scope::DeclScope |
+   Scope::CompoundStmtScope);
 Scope *ParentScope = getCurScope()->getParent();
 
 D.setFunctionDefinitionKind(FDK_Definition);
@@ -1124,7 +1126,8 @@
   }
 
   // Enter a scope for the function body.
-  ParseScope BodyScope(this, Scope::FnScope|Scope::DeclScope);
+  ParseSc

[PATCH] D33676: Place implictly declared functions at block scope

2017-08-02 Thread Momchil Velikov via Phabricator via cfe-commits
chill added a comment.

Thanks for the review, I'll wait a few days.


https://reviews.llvm.org/D33676



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


[PATCH] D36200: [Sema] Improve some -Wunguarded-availability diagnostic messages

2017-08-02 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

In https://reviews.llvm.org/D36200#829104, @arphaman wrote:

> This needs a test for the fixits as well, see test/FixIt/fixit-availability*


Why? This patch doesn't change the behavior of the fixits, so there isn't any 
new behavior to test.


https://reviews.llvm.org/D36200



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


[PATCH] D36200: [Sema] Improve some -Wunguarded-availability diagnostic messages

2017-08-02 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

Right, sorry, I didn't realize that code was moved.


https://reviews.llvm.org/D36200



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


[PATCH] D36044: [OpenCL] -cl-ext option can overwrite OpenCL features imported from a module

2017-08-02 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: include/clang/Basic/OpenCLOptions.h:132
 
-  void disableAll() {
+  void enableAll(bool On = true) {
 for (llvm::StringMap::iterator I = OptMap.begin(),

May be the name could be `setAll` since it's doing both enable and disable too.



Comment at: test/SemaOpenCL/extensions-import.cl:6
+// No PCH
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 %s -fsyntax-only -verify -DFP64
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL1.2 %s -fsyntax-only -verify 
-cl-ext=-cl_khr_fp64

Do we need to test w/o PCH here? I think we have similar test coverage in 
`test/SemaOpenCL/extensions.cl`.

Also we normally put RUN lines first.


https://reviews.llvm.org/D36044



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


[PATCH] D35937: [clang-tidy] Add new readability non-idiomatic static access

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:23
+  memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStaticStorageClass()),
+  varDecl(hasStaticStorageDuration(,
+ unless(isInTemplateInstantiation()))

barancsuk wrote:
> aaron.ballman wrote:
> > Why not use `isStaticStorageClass()` here as well?
> Unfortunately, `isStaticStorageClass()` misses variable declarations that do 
> not contain the static keyword, including definitions of static variables 
> outside of their class.
> However, `hasStaticStorageDuration()` has no problem finding all static 
> variable declarations correctly. 
Under what circumstances would that make a difference? If the variable is 
declared within the class with the static storage specifier, that declaration 
should be sufficient for the check, regardless of how the definition looks, no?

If it does make a difference, can you add a test case that demonstrates that?



Comment at: clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:30
+return NameSpecifierNestingLevel;
+  } else
+return 0;

No `else` after a `return`.



Comment at: clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp:79
+  if (BaseExpr->HasSideEffects(*AstContext) ||
+  (getNameSpecifierNestingLevel(BaseType) > NameSpecifierNestingThreshold))
+return;

No need for the extra parens here.



Comment at: clang-tidy/readability/StaticAccessedThroughInstanceCheck.h:20
+/// \@brief Checks for member expressions that access static members through 
instances
+/// and replaces them with calls to the preferred, more readable `::` operator.
+///

aaron.ballman wrote:
> `::` isn't an operator -- I would say: "and replaces them with uses of the 
> appropriate qualified-id."
Reflow the comments.



Comment at: test/clang-tidy/readability-static-accessed-through-instance.cpp:34
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member accessed through 
instance  [readability-static-accessed-through-instance]
+  // CHECK-FIXES: {{^}}  C::x;{{$}}
+}

barancsuk wrote:
> aaron.ballman wrote:
> > baloghadamsoftware wrote:
> > > xazax.hun wrote:
> > > > baloghadamsoftware wrote:
> > > > > aaron.ballman wrote:
> > > > > > This fix-it worries me because it changes the semantics of the 
> > > > > > code. The function `f()` is no longer called, and so this isn't a 
> > > > > > valid source transformation.
> > > > > Maybe the correct fix would be here f(1, 2, 3, 4); C::x;
> > > > Maybe for now we should just skip this cases. Expr::HasSideEffects 
> > > > might be a  good candidate to filter these cases. 
> > > I think including the expression with side effect before the member 
> > > access (as I suggested) is not more complicated than skipping these cases.
> > Please ensure you handle the more complex cases then, such as:
> > ```
> > struct S {
> >   static int X;
> > };
> > 
> > void g(int &);
> > S h();
> > 
> > void f(S s) {
> >   g(h().X);
> >   if ([s]() { return s; }().X == 15)
> > ;
> > }
> > ```
> Expressions with side effects introduce some troublesome cases.
> For example, in the following code, the static member expression, `h().x` 
> does not get evaluated if a() returns false. 
> 
> ```
> struct C {
>   static int x;
> };
> 
> void j(int);
> int k(bool);
> bool a();
> C h();
> 
> void g(){
>   j(k(a() && h().x));
> }
> ```
> 
> Maybe for now, the check should filter these expressions with side effects.
> They might better be addressed in a follow-up patch.
> Maybe for now, the check should filter these expressions with side effects.
> They might better be addressed in a follow-up patch.

I think that's a good approach.



Comment at: test/clang-tidy/readability-static-accessed-through-instance.cpp:90
+
+  // u.u is not changed to N::V::T::U::u; because the nesting level is over 3.
+  u.u = 12;

It'd be good to have an additional RUN line that sets the nesting level to 
something other than the default and ensure the behavior is consistent.


https://reviews.llvm.org/D35937



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


[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D33672#827513, @xazax.hun wrote:

> Even though it is not undefined behavior in C, it can still cause surprising 
> behavior for the users. I think maybe putting it into the optin package 
> instead of cplusplus is better. What do you think?


I think it's reasonable to diagnose in C when converting the value would lose 
precision, so long as the check implements that behavior properly for C. That 
would suggest it belongs somewhere other than cplusplus.


https://reviews.llvm.org/D33672



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


[PATCH] D36230: [X86][Asm] Allow negative immediate to appear before bracketed expression

2017-08-02 Thread coby via Phabricator via cfe-commits
coby created this revision.
Herald added a subscriber: eraman.

Currently, only non-negative immediate is allowed prior to a brac expression 
(memory reference).
MASM / GAS does not have any problem cope with the left side of the real line, 
so we should be able to as well.

llvm: https://reviews.llvm.org/D36229


Repository:
  rL LLVM

https://reviews.llvm.org/D36230

Files:
  test/CodeGen/ms-inline-asm.c


Index: test/CodeGen/ms-inline-asm.c
===
--- test/CodeGen/ms-inline-asm.c
+++ test/CodeGen/ms-inline-asm.c
@@ -484,13 +484,13 @@
   __asm mov eax, (4 + 4) * 16
 // CHECK: mov eax, $$128
   __asm mov eax, 4 + 8 * -16
-// CHECK: mov eax, $$4294967172
+// CHECK: mov eax, $$-124
   __asm mov eax, 4 + 16 / -8
 // CHECK: mov eax, $$2
   __asm mov eax, (16 + 16) / -8
-// CHECK: mov eax, $$4294967292
+// CHECK: mov eax, $$-4
   __asm mov eax, ~15
-// CHECK: mov eax, $$4294967280
+// CHECK: mov eax, $$-16
   __asm mov eax, 6 ^ 3
 // CHECK: mov eax, $$5
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
@@ -651,6 +651,12 @@
   // CHECK: call void asm sideeffect inteldialect "mov dr0, eax\0A\09mov dr1, 
ebx\0A\09mov dr2, ebx\0A\09mov dr3, ecx\0A\09mov dr6, edx\0A\09mov dr7, ecx", 
"~{dr0},~{dr1},~{dr2},~{dr3},~{dr6},~{dr7},~{dirflag},~{fpsr},~{flags}"()
 }
 
+void t46() {
+  // CHECK-LABEL: define void @t46
+  __asm add eax, -128[eax]
+  // CHECK: call void asm sideeffect inteldialect "add eax, $$-128[eax]", 
"~{eax},~{flags},~{dirflag},~{fpsr},~{flags}"()
+}
+
 void dot_operator(){
 // CHECK-LABEL: define void @dot_operator
__asm { mov eax, 3[ebx]A.b}


Index: test/CodeGen/ms-inline-asm.c
===
--- test/CodeGen/ms-inline-asm.c
+++ test/CodeGen/ms-inline-asm.c
@@ -484,13 +484,13 @@
   __asm mov eax, (4 + 4) * 16
 // CHECK: mov eax, $$128
   __asm mov eax, 4 + 8 * -16
-// CHECK: mov eax, $$4294967172
+// CHECK: mov eax, $$-124
   __asm mov eax, 4 + 16 / -8
 // CHECK: mov eax, $$2
   __asm mov eax, (16 + 16) / -8
-// CHECK: mov eax, $$4294967292
+// CHECK: mov eax, $$-4
   __asm mov eax, ~15
-// CHECK: mov eax, $$4294967280
+// CHECK: mov eax, $$-16
   __asm mov eax, 6 ^ 3
 // CHECK: mov eax, $$5
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
@@ -651,6 +651,12 @@
   // CHECK: call void asm sideeffect inteldialect "mov dr0, eax\0A\09mov dr1, ebx\0A\09mov dr2, ebx\0A\09mov dr3, ecx\0A\09mov dr6, edx\0A\09mov dr7, ecx", "~{dr0},~{dr1},~{dr2},~{dr3},~{dr6},~{dr7},~{dirflag},~{fpsr},~{flags}"()
 }
 
+void t46() {
+  // CHECK-LABEL: define void @t46
+  __asm add eax, -128[eax]
+  // CHECK: call void asm sideeffect inteldialect "add eax, $$-128[eax]", "~{eax},~{flags},~{dirflag},~{fpsr},~{flags}"()
+}
+
 void dot_operator(){
 // CHECK-LABEL: define void @dot_operator
 	__asm { mov eax, 3[ebx]A.b}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[libcxx] r309838 - Fix PR33727: std::basic_stringbuf only works with DefaultConstructible allocators. Thanks to Jonathan Wakely for the report and suggested fix

2017-08-02 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Wed Aug  2 10:31:09 2017
New Revision: 309838

URL: http://llvm.org/viewvc/llvm-project?rev=309838&view=rev
Log:
Fix PR33727: std::basic_stringbuf only works with DefaultConstructible 
allocators. Thanks to Jonathan Wakely for the report and suggested fix

Modified:
libcxx/trunk/include/sstream

libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp

Modified: libcxx/trunk/include/sstream
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/sstream?rev=309838&r1=309837&r2=309838&view=diff
==
--- libcxx/trunk/include/sstream (original)
+++ libcxx/trunk/include/sstream Wed Aug  2 10:31:09 2017
@@ -249,7 +249,8 @@ basic_stringbuf<_CharT, _Traits, _Alloca
 template 
 basic_stringbuf<_CharT, _Traits, _Allocator>::basic_stringbuf(const 
string_type& __s,
  ios_base::openmode __wch)
-: __hm_(0),
+: __str_(__s.get_allocator()),
+  __hm_(0),
   __mode_(__wch)
 {
 str(__s);

Modified: 
libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp?rev=309838&r1=309837&r2=309838&view=diff
==
--- 
libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp
 Wed Aug  2 10:31:09 2017
@@ -18,6 +18,16 @@
 #include 
 #include 
 
+template
+struct NoDefaultAllocator : std::allocator
+{
+  template struct rebind { using other = NoDefaultAllocator; };
+  NoDefaultAllocator(int id) : id(id) { }
+  template NoDefaultAllocator(const NoDefaultAllocator& a) : 
id(a.id) { }
+  int id;
+};
+
+
 int main()
 {
 {
@@ -46,4 +56,13 @@ int main()
 ss << i << ' ' << 123;
 assert(ss.str() == L"456 1236 ");
 }
+{ // This is https://bugs.llvm.org/show_bug.cgi?id=33727
+   typedef std::basic_string   , 
NoDefaultAllocator > S;
+   typedef std::basic_stringbuf, 
NoDefaultAllocator > SB;
+
+   S s(NoDefaultAllocator(1));
+   SB sb(s);
+   //  This test is not required by the standard, but *where else* 
could it get the allocator?
+   assert(sb.str().get_allocator() == s.get_allocator());
+}
 }


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


[PATCH] D35755: [Solaris] gcc toolchain handling revamp

2017-08-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1511
+  StringRef Suff64 = "/64";
+  // Solaris uses platform-specific suffixes instead of /64
+  if (TargetTriple.getOS() == llvm::Triple::Solaris) {

Add a period at the end of the comment.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1732
 
-// Then look for distribution supplied gcc installations.
+// Now prefix(es) that correspond to distribution-supplied gcc 
installations
 if (D.SysRoot.empty()) {

s/Now/Next, look for
Add a period at the end of the comment.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1734
 if (D.SysRoot.empty()) {
-  // Look for RHEL devtoolsets.
-  Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
-  Prefixes.push_back("/opt/rh/devtoolset-4/root/usr");
-  Prefixes.push_back("/opt/rh/devtoolset-3/root/usr");
-  Prefixes.push_back("/opt/rh/devtoolset-2/root/usr");
-  // And finally in /usr.
-  Prefixes.push_back("/usr");
+  // typically /usr
+  AddDefaultGCCPrefixes(TargetTriple, Prefixes, D.SysRoot);

s/typically/Typically
Add a period at the end of the comment.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1807
+void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
+const llvm::Triple &TargetTriple, SmallVectorImpl &Prefixes,
+StringRef SysRoot) {

I'd drop the `llvm::` here.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1813-1814
+//   /usr/gcc/./lib/gcc//../
+// so we need to find those /usr/gcc/*/lib/gcc libdirs
+// and go with /usr/gcc/ as a prefix
+

This comment can probably be re-flowed, and needs a period at the end of it.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1837
+
+  // non-Solaris is much simpler - most systems just go with "/usr"
+  if (SysRoot.empty()) {

s/non/Non
Needs a period at the end of the comment.



Comment at: lib/Driver/ToolChains/Gnu.cpp:1840
+// Yet, still look for RHEL devtoolsets
+// (should it be done Linux-only??)
+Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");

This is a good question to get an answer to before committing. :-) I don't know 
the answer myself, unfortunately.



Comment at: lib/Driver/ToolChains/Gnu.h:253
 
+void AddDefaultGCCPrefixes(const llvm::Triple &TargetTriple,
+   SmallVectorImpl &Prefixes,

Might as well drop the `llvm::` since the namespace isn't used for 
`SmallVectorImpl`.



Comment at: lib/Driver/ToolChains/Gnu.h:309
+  // FIXME: This should be final, but CrossWindows toolchain does weird
+  // things that cant be easily generalized
   void AddClangCXXStdlibIncludeArgs(

s/but CrossWindows/but the CrossWindows
s/cant/can't
Add a period at the end of the comment.



Comment at: lib/Driver/ToolChains/Solaris.cpp:151
+  if (GCCInstallation.isValid()) {
+// On Solaris gcc uses both architecture-specific path with triple in it
+// as well as more generic lib path (+arch suffix)

s/both architecture/both an architecture



Comment at: lib/Driver/ToolChains/Solaris.cpp:152
+// On Solaris gcc uses both architecture-specific path with triple in it
+// as well as more generic lib path (+arch suffix)
+addPathIfExists(D,

s/as more/as a more

Add a period at the end of the comment.



Comment at: lib/Driver/ToolChains/Solaris.cpp:160
 
-  addPathIfExists(D, getDriver().SysRoot + LibPath, Paths);
+  // if we are currently running Clang inside of the requested system root,
+  // add its parent library path to those searched.

s/if/If



Comment at: lib/Driver/ToolChains/Solaris.cpp:208
   if (GCCInstallation.isValid()) {
-GCCVersion Version = GCCInstallation.getVersion();
-addSystemInclude(DriverArgs, CC1Args,
- getDriver().SysRoot + "/usr/gcc/" +
- Version.MajorStr + "." +
- Version.MinorStr +
- "/include/c++/" + Version.Text);
-addSystemInclude(DriverArgs, CC1Args,
- getDriver().SysRoot + "/usr/gcc/" + Version.MajorStr +
- "." + Version.MinorStr + "/include/c++/" +
- Version.Text + "/" +
- GCCInstallation.getTriple().str());
+const auto &Callback = Multilibs.includeDirsCallback();
+if (Callback) {

Shouldn't use `auto` here because the type is not spelled out in the 
initialization.


https://reviews.llvm.org/D35755



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


Re: r309523 - Also pass -pie back to the linker when linking on OpenBSD.

2017-08-02 Thread Hans Wennborg via cfe-commits
Merged to 5.0 in r309844.

On Sun, Jul 30, 2017 at 2:14 PM, Brad Smith via cfe-commits
 wrote:
> Author: brad
> Date: Sun Jul 30 14:13:59 2017
> New Revision: 309523
>
> URL: http://llvm.org/viewvc/llvm-project?rev=309523&view=rev
> Log:
> Also pass -pie back to the linker when linking on OpenBSD.
>
> Modified:
> cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
> cfe/trunk/test/Driver/openbsd.c
>
> Modified: cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp?rev=309523&r1=309522&r2=309523&view=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp Sun Jul 30 14:13:59 2017
> @@ -133,6 +133,8 @@ void openbsd::Linker::ConstructJob(Compi
>  }
>}
>
> +  if (Args.hasArg(options::OPT_pie))
> +CmdArgs.push_back("-pie");
>if (Args.hasArg(options::OPT_nopie))
>  CmdArgs.push_back("-nopie");
>
>
> Modified: cfe/trunk/test/Driver/openbsd.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/openbsd.c?rev=309523&r1=309522&r2=309523&view=diff
> ==
> --- cfe/trunk/test/Driver/openbsd.c (original)
> +++ cfe/trunk/test/Driver/openbsd.c Sun Jul 30 14:13:59 2017
> @@ -77,7 +77,9 @@
>  // Check linking against correct startup code when (not) using PIE
>  // RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd %s -### 2>&1 \
>  // RUN:   | FileCheck -check-prefix=CHECK-PIE %s
> -// RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd %s -fno-pie %s 
> -### 2>&1 \
> +// RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd -pie %s -### 
> 2>&1 \
> +// RUN:   | FileCheck -check-prefix=CHECK-PIE-FLAG %s
> +// RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd -fno-pie %s 
> -### 2>&1 \
>  // RUN:   | FileCheck -check-prefix=CHECK-PIE %s
>  // RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd -static %s 
> -### 2>&1 \
>  // RUN:   | FileCheck -check-prefix=CHECK-STATIC-PIE %s
> @@ -93,6 +95,7 @@
>  // RUN:   | FileCheck -check-prefix=CHECK-NOPIE %s
>  // CHECK-PIE: "{{.*}}crt0.o"
>  // CHECK-PIE-NOT: "-nopie"
> +// CHECK-PIE-FLAG: "-pie"
>  // CHECK-STATIC-PIE: "{{.*}}rcrt0.o"
>  // CHECK-STATIC-PIE-NOT: "-nopie"
>  // CHECK-NOPIE: "-nopie" "{{.*}}crt0.o"
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

2017-08-02 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added inline comments.



Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:136
+if (ThreadPointer == ReadTPMode::Invalid &&
+!StringRef(A->getValue()).empty()) {
+  D.Diag(diag::err_drv_invalid_mtp) << A->getAsString(Args);

What happens if you pass an empty "-mtp="  to the driver? Will it silently 
assume soft? Shouldn't it be an error too?



Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:138
+  D.Diag(diag::err_drv_invalid_mtp) << A->getAsString(Args);
+  ThreadPointer = ReadTPMode::Soft;
+}

Won't this assignment be covered by the code below anyway? Maybe remove it?


https://reviews.llvm.org/D34878



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


[PATCH] D36112: [ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available

2017-08-02 Thread Vedant Kumar via Phabricator via cfe-commits
vsk marked an inline comment as done.
vsk added a comment.

In https://reviews.llvm.org/D36112#828891, @arphaman wrote:

> That makes sense. It's kinda weird not to report the `null`, but I guess it 
> makes sense if the `null` sanitiser is off.


It is kinda weird, but any such diagnostic would fit better under 
-fsanitize=null, and that's explicitly not enabled.

> It's not actually UB unless it's dereferenced, right, so casts are allowed?

Right, even the null sanitizer doesn't issue reports when it finds an 
{up,down}cast of null.

@arphaman thanks for taking a look! I've fixed the test case issue you pointed 
out and added minor comment and doc updates. I'll go ahead and commit to 
unblock Nico.


https://reviews.llvm.org/D36112



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


Re: r305903 - Function with unparsed body is a definition

2017-08-02 Thread Serge Pavlov via cfe-commits
At first thank you for the nice test case.

This crash is not caused by r305903. The root cause is that instantiation
of f is triggered when parse of f is not finished yet. This is the
case just addressed by that change.
The instantiation is requested by the code (https://github.com/llvm-mirro
r/clang/blob/master/lib/Sema/SemaExpr.cpp#L13622):

else if (Func->isConstexpr())
// Do not defer instantiations of constexpr functions, to avoid the
// expression evaluator needing to call back into Sema if it sees a
// call to such a function.
InstantiateFunctionDefinition(PointOfInstantiation, Func);


It looks like a violation of C++ Standard, as function is instantiated when
there is no use of it. However if this check is removed and instantiation
is postponed as for non-constexpr functions, lots of tests fail.

Without r305903 the call to `getDefinition` for templated declaration of
f returned null, declaration f was moved to the list of pending
instantiations and crash is not observed. It does not mean however that
original source code would be compiled correctly.

It this is urgent problem, I can prepare a patch that will mimic behavior
before r305903 for instantiation of constexpr functions. If there is some
time I would investigate this problem in depth, as it manifests itself in
number of cases, for instance in https://bugs.llvm.org/show_bug.cgi?id=33561
.

Thanks,
--Serge

2017-08-01 23:28 GMT+07:00 Serge Pavlov :

> Yes, sure, I will investigate it.
>
> Thanks,
> --Serge
>
> 2017-08-01 21:32 GMT+07:00 Alexander Kornienko :
>
>> This change causes an assertion failure on valid code. Could you take a
>> look at fixing this?
>>
>> A reduced test case:
>>
>> $ cat /tmp/SemaTemplateInstantiateDecl-crash2.cpp
>> template 
>> constexpr void f(T) {
>>   f(0);
>> }
>> $ clang -fsyntax-only -std=c++11 /tmp/SemaTemplateInstantiateDe
>> cl-crash2.cpp
>> assert.h assertion failed at llvm/tools/clang/lib/Sema/Sema
>> TemplateInstantiateDecl.cpp:3840 in void clang::Sema::InstantiateFuncti
>> onDefinition(clang::SourceLocation, clang::FunctionDecl *, bool, bool,
>> bool): (Pattern |
>> | PatternDecl->isDefaulted()) && "unexpected kind of function template
>> definition"
>> *** Check failure stack trace: ***
>> @  0x6094c4a  __assert_fail
>> @  0x2705bfe  clang::Sema::InstantiateFunctionDefinition()
>> @  0x2c1fb13  clang::Sema::MarkFunctionReferenced()
>> @  0x2bec07b  clang::Sema::MarkAnyDeclReferenced()
>> @  0x2c2327f  MarkExprReferenced()
>> @  0x2be8f0a  clang::Sema::MarkDeclRefReferenced()
>> @  0x2980ac0  clang::Sema::FixOverloadedFunctionReference()
>> @  0x2982e66  FinishOverloadedCallExpr()
>> @  0x2982b39  clang::Sema::BuildOverloadedCallExpr()
>> @  0x2be461b  clang::Sema::ActOnCallExpr()
>> @  0x2571e90  clang::Parser::ParsePostfixExpressionSuffix()
>> @  0x25784cb  clang::Parser::ParseCastExpression()
>> @  0x2570865  clang::Parser::ParseCastExpression()
>> @  0x256f1e3  clang::Parser::ParseAssignmentExpression()
>> @  0x256f0bf  clang::Parser::ParseExpression()
>> @  0x2517eeb  clang::Parser::ParseExprStatement()
>> @  0x2517074  clang::Parser::ParseStatement
>> OrDeclarationAfterAttributes()
>> @  0x2516b50  clang::Parser::ParseStatementOrDeclaration()
>> @  0x251deb4  clang::Parser::ParseCompoundStatementBody()
>> @  0x251ea53  clang::Parser::ParseFunctionStatementBody()
>> @  0x24f5b23  clang::Parser::ParseFunctionDefinition()
>> @  0x25082a5  clang::Parser::ParseSingleDec
>> larationAfterTemplate()
>> @  0x2507652  clang::Parser::ParseTemplateD
>> eclarationOrSpecialization()
>> @  0x2506fa5  clang::Parser::ParseDeclarati
>> onStartingWithTemplate()
>> @  0x25b6853  clang::Parser::ParseDeclaration()
>> @  0x24f36d5  clang::Parser::ParseExternalDeclaration()
>> @  0x24f2739  clang::Parser::ParseTopLevelDecl()
>> @  0x24f220e  clang::Parser::ParseFirstTopLevelDecl()
>> @  0x24ed582  clang::ParseAST()
>>
>> On Wed, Jun 21, 2017 at 2:46 PM, Serge Pavlov via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: sepavloff
>>> Date: Wed Jun 21 07:46:57 2017
>>> New Revision: 305903
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=305903&view=rev
>>> Log:
>>> Function with unparsed body is a definition
>>>
>>> While a function body is being parsed, the function declaration is not
>>> considered
>>> as a definition because it does not have a body yet. In some cases it
>>> leads to
>>> incorrect interpretation, the case is presented in
>>> https://bugs.llvm.org/show_bug.cgi?id=14785:
>>> ```
>>> template struct Somewhat {
>>>   void internal() const {}
>>>   friend void operator+(int const &, Somewhat const &)

r309846 - [ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available

2017-08-02 Thread Vedant Kumar via cfe-commits
Author: vedantk
Date: Wed Aug  2 11:10:31 2017
New Revision: 309846

URL: http://llvm.org/viewvc/llvm-project?rev=309846&view=rev
Log:
[ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't 
available

In r309007, I made -fsanitize=null a hard prerequisite for -fsanitize=vptr. I
did not see the need for the two checks to have separate null checking logic
for the same pointer. I expected the two checks to either always be enabled
together, or to be mutually compatible.

In the mailing list discussion re: r309007 it became clear that that isn't the
case. If a codebase is -fsanitize=vptr clean but not -fsanitize=null clean,
it's useful to have -fsanitize=vptr emit its own null check. That's what this
patch does: with it, -fsanitize=vptr can be used without -fsanitize=null.

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

Modified:
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
cfe/trunk/test/Driver/fsanitize.c
cfe/trunk/test/Driver/rtti-options.cpp

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=309846&r1=309845&r2=309846&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Wed Aug  2 11:10:31 2017
@@ -198,9 +198,7 @@ Static Analyzer
 Undefined Behavior Sanitizer (UBSan)
 
 
-The C++ dynamic type check now requires run-time null checking (i.e,
-`-fsanitize=vptr` cannot be used without `-fsanitize=null`). This change does
-not impact users who rely on UBSan check groups (e.g `-fsanitize=undefined`).
+...
 
 Core Analysis Improvements
 ==

Modified: cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UndefinedBehaviorSanitizer.rst?rev=309846&r1=309845&r2=309846&view=diff
==
--- cfe/trunk/docs/UndefinedBehaviorSanitizer.rst (original)
+++ cfe/trunk/docs/UndefinedBehaviorSanitizer.rst Wed Aug  2 11:10:31 2017
@@ -132,10 +132,10 @@ Available checks are:
   -  ``-fsanitize=vla-bound``: A variable-length array whose bound
  does not evaluate to a positive value.
   -  ``-fsanitize=vptr``: Use of an object whose vptr indicates that it is of
- the wrong dynamic type, or that its lifetime has not begun or has ended.
- Incompatible with ``-fno-rtti`` and ``-fno-sanitize=null``. Link must be
- performed by ``clang++``, not ``clang``, to make sure C++-specific parts 
of
- the runtime library and C++ standard libraries are present.
+the wrong dynamic type, or that its lifetime has not begun or has ended.
+Incompatible with ``-fno-rtti``. Link must be performed by ``clang++``, not
+``clang``, to make sure C++-specific parts of the runtime library and C++
+standard libraries are present.
 
 You can also use the following check groups:
   -  ``-fsanitize=undefined``: All of the checks listed above other than

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=309846&r1=309845&r2=309846&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Wed Aug  2 11:10:31 
2017
@@ -232,9 +232,6 @@ def warn_drv_enabling_rtti_with_exceptio
 def warn_drv_disabling_vptr_no_rtti_default : Warning<
   "implicitly disabling vptr sanitizer because rtti wasn't enabled">,
   InGroup;
-def warn_drv_disabling_vptr_no_null_check : Warning<
-  "implicitly disabling vptr sanitizer because null checking wasn't enabled">,
-  InGroup;
 def warn_drv_object_size_disabled_O0 : Warning<
   "the object size sanitizer has no effect at -O0, but is explicitly enabled: 
%0">,
   InGroup;

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=309846&r1=309845&r2=309846&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Aug  2 11:10:31 2017
@@ -694,17 +694,17 @@ void CodeGenFunction::EmitTypeCheck(Type
   //-- the [pointer or glvalue] is used to access a non-static data member
   //   or call a non-static member function
   CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
-  bool HasNullCheck = IsGuaranteedNonNull || IsNonNull;
   if (SanOpts.has(SanitizerKind::Vptr) &&
-  !S

[PATCH] D36112: [ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available

2017-08-02 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309846: [ubsan] Have -fsanitize=vptr emit a null check if 
-fsanitize=null isn't… (authored by vedantk).

Changed prior to commit:
  https://reviews.llvm.org/D36112?vs=108988&id=109385#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36112

Files:
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
  cfe/trunk/test/CodeGenCXX/ubsan-type-checks.cpp
  cfe/trunk/test/Driver/fsanitize.c
  cfe/trunk/test/Driver/rtti-options.cpp

Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -694,17 +694,17 @@
   //-- the [pointer or glvalue] is used to access a non-static data member
   //   or call a non-static member function
   CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
-  bool HasNullCheck = IsGuaranteedNonNull || IsNonNull;
   if (SanOpts.has(SanitizerKind::Vptr) &&
-  !SkippedChecks.has(SanitizerKind::Vptr) && HasNullCheck &&
+  !SkippedChecks.has(SanitizerKind::Vptr) &&
   (TCK == TCK_MemberAccess || TCK == TCK_MemberCall ||
TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference ||
TCK == TCK_UpcastToVirtualBase) &&
   RD && RD->hasDefinition() && RD->isDynamicClass()) {
 // Ensure that the pointer is non-null before loading it. If there is no
-// compile-time guarantee, reuse the run-time null check.
+// compile-time guarantee, reuse the run-time null check or emit a new one.
 if (!IsGuaranteedNonNull) {
-  assert(IsNonNull && "Missing run-time null check");
+  if (!IsNonNull)
+IsNonNull = Builder.CreateIsNotNull(Ptr);
   if (!Done)
 Done = createBasicBlock("vptr.null");
   llvm::BasicBlock *VptrNotNull = createBasicBlock("vptr.not.null");
Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -307,13 +307,6 @@
 Kinds &= ~Vptr;
   }
 
-  // Disable -fsanitize=vptr if -fsanitize=null is not enabled (the vptr
-  // instrumentation is broken without run-time null checks).
-  if ((Kinds & Vptr) && !(Kinds & Null)) {
-Kinds &= ~Vptr;
-D.Diag(diag::warn_drv_disabling_vptr_no_null_check);
-  }
-
   // Check that LTO is enabled if we need it.
   if ((Kinds & NeedsLTO) && !D.isUsingLTO()) {
 D.Diag(diag::err_drv_argument_only_allowed_with)
Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -198,9 +198,7 @@
 Undefined Behavior Sanitizer (UBSan)
 
 
-The C++ dynamic type check now requires run-time null checking (i.e,
-`-fsanitize=vptr` cannot be used without `-fsanitize=null`). This change does
-not impact users who rely on UBSan check groups (e.g `-fsanitize=undefined`).
+...
 
 Core Analysis Improvements
 ==
Index: cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
===
--- cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
+++ cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
@@ -132,10 +132,10 @@
   -  ``-fsanitize=vla-bound``: A variable-length array whose bound
  does not evaluate to a positive value.
   -  ``-fsanitize=vptr``: Use of an object whose vptr indicates that it is of
- the wrong dynamic type, or that its lifetime has not begun or has ended.
- Incompatible with ``-fno-rtti`` and ``-fno-sanitize=null``. Link must be
- performed by ``clang++``, not ``clang``, to make sure C++-specific parts of
- the runtime library and C++ standard libraries are present.
+the wrong dynamic type, or that its lifetime has not begun or has ended.
+Incompatible with ``-fno-rtti``. Link must be performed by ``clang++``, not
+``clang``, to make sure C++-specific parts of the runtime library and C++
+standard libraries are present.
 
 You can also use the following check groups:
   -  ``-fsanitize=undefined``: All of the checks listed above other than
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -232,9 +232,6 @@
 def warn_drv_disabling_vptr_no_rtti_default : Warning<
   "implicitly disabling vptr sanitizer because rtti wasn't enabled">,
   InGroup;
-def warn_drv_disabling_vptr_no_null_check : Warning<
-  "implicitly disabling vptr sanitizer because null checking wasn'

[libcxx] r309851 - Fix shadowing warning

2017-08-02 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Wed Aug  2 11:21:34 2017
New Revision: 309851

URL: http://llvm.org/viewvc/llvm-project?rev=309851&view=rev
Log:
Fix shadowing warning

Modified:

libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp

Modified: 
libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp?rev=309851&r1=309850&r2=309851&view=diff
==
--- 
libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/input.output/string.streams/stringstream.cons/string.pass.cpp
 Wed Aug  2 11:21:34 2017
@@ -22,7 +22,7 @@ template
 struct NoDefaultAllocator : std::allocator
 {
   template struct rebind { using other = NoDefaultAllocator; };
-  NoDefaultAllocator(int id) : id(id) { }
+  NoDefaultAllocator(int id_) : id(id_) { }
   template NoDefaultAllocator(const NoDefaultAllocator& a) : 
id(a.id) { }
   int id;
 };


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


r309852 - [docs] Remove accidental unindent to appease the sphinx bot

2017-08-02 Thread Vedant Kumar via cfe-commits
Author: vedantk
Date: Wed Aug  2 11:24:12 2017
New Revision: 309852

URL: http://llvm.org/viewvc/llvm-project?rev=309852&view=rev
Log:
[docs] Remove accidental unindent to appease the sphinx bot

Bot failure:
http://lab.llvm.org:8011/builders/clang-sphinx-docs/builds/12043/steps/docs-clang-html/logs/stdio

Modified:
cfe/trunk/docs/UndefinedBehaviorSanitizer.rst

Modified: cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UndefinedBehaviorSanitizer.rst?rev=309852&r1=309851&r2=309852&view=diff
==
--- cfe/trunk/docs/UndefinedBehaviorSanitizer.rst (original)
+++ cfe/trunk/docs/UndefinedBehaviorSanitizer.rst Wed Aug  2 11:24:12 2017
@@ -132,10 +132,10 @@ Available checks are:
   -  ``-fsanitize=vla-bound``: A variable-length array whose bound
  does not evaluate to a positive value.
   -  ``-fsanitize=vptr``: Use of an object whose vptr indicates that it is of
-the wrong dynamic type, or that its lifetime has not begun or has ended.
-Incompatible with ``-fno-rtti``. Link must be performed by ``clang++``, not
-``clang``, to make sure C++-specific parts of the runtime library and C++
-standard libraries are present.
+ the wrong dynamic type, or that its lifetime has not begun or has ended.
+ Incompatible with ``-fno-rtti``. Link must be performed by ``clang++``, 
not
+ ``clang``, to make sure C++-specific parts of the runtime library and C++
+ standard libraries are present.
 
 You can also use the following check groups:
   -  ``-fsanitize=undefined``: All of the checks listed above other than


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


[PATCH] D35922: [Driver] Enable AddressSanitizer for Fuchsia targets

2017-08-02 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

One more thing, would it be possible add a test case to `test/Driver/fuchsia.c` 
covering this?


Repository:
  rL LLVM

https://reviews.llvm.org/D35922



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


[PATCH] D35955: clang-format: Add preprocessor directive indentation

2017-08-02 Thread Erik Uhlmann via Phabricator via cfe-commits
euhlmann added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:383
+  Current.Previous->is(tok::hash) && State.FirstIndent > 0) {
+// subtract 1 so indent lines up with non-preprocessor code
+Spaces += State.FirstIndent;

djasper wrote:
> Same here and use full sentences.
> 
> Also, this does not seem to be what the example in the style option suggests 
> and I'd rather not do it (subtracting 1).
I apologize, the style option documentation was a typo. The patch summary has 
the intended behavior, and I mentioned there that I count the hash as a column. 
Part of the reasoning for this is because it would look the same visually with 
spaces or tabs.


https://reviews.llvm.org/D35955



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


[PATCH] D36237: Reland "Thread Safety Analysis: fix assert_capability.", add warnings

2017-08-02 Thread Josh Gao via Phabricator via cfe-commits
jmgao updated this revision to Diff 109411.
jmgao added a comment.

Remove accidental trailing backslash.


https://reviews.llvm.org/D36237

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/ThreadSafety.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-capabilities.c
  test/SemaCXX/warn-thread-safety-analysis.cpp
  test/SemaCXX/warn-thread-safety-parsing.cpp

Index: test/SemaCXX/warn-thread-safety-parsing.cpp
===
--- test/SemaCXX/warn-thread-safety-parsing.cpp
+++ test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -571,11 +571,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void elf_function() EXCLUSIVE_LOCK_FUNCTION();
+void elf_function() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without arguments cannot be applied to a free function}}
 
 void elf_function_args() EXCLUSIVE_LOCK_FUNCTION(mu1, mu2);
 
-int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION();
+int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without arguments cannot be applied to a free function}}
 
 int elf_testfn(int y) {
   int x EXCLUSIVE_LOCK_FUNCTION() = y; // \
@@ -590,7 +590,7 @@
  private:
   int test_field EXCLUSIVE_LOCK_FUNCTION(); // \
 // expected-warning {{'exclusive_lock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_LOCK_FUNCTION();
+  void test_method() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute requires type annotated with 'capability' attribute; type here is 'ElfFoo *'}}
 };
 
 class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
@@ -643,11 +643,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void slf_function() SHARED_LOCK_FUNCTION();
+void slf_function() SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without arguments cannot be applied to a free function}}
 
 void slf_function_args() SHARED_LOCK_FUNCTION(mu1, mu2);
 
-int slf_testfn(int y) SHARED_LOCK_FUNCTION();
+int slf_testfn(int y) SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without arguments cannot be applied to a free function}}
 
 int slf_testfn(int y) {
   int x SHARED_LOCK_FUNCTION() = y; // \
@@ -665,7 +665,8 @@
  private:
   int test_field SHARED_LOCK_FUNCTION(); // \
 // expected-warning {{'shared_lock_function' attribute only applies to functions}}
-  void test_method() SHARED_LOCK_FUNCTION();
+  void test_method() SHARED_LOCK_FUNCTION(); // \
+// expected-warning {{'shared_lock_function' attribute requires type annotated with 'capability' attribute; type here is 'SlfFoo *'}}
 };
 
 class SHARED_LOCK_FUNCTION() SlfTestClass { // \
@@ -716,29 +717,32 @@
 // takes a mandatory boolean or integer argument specifying the retval
 // plus an optional list of locks (vars/fields)
 
-void etf_function() __attribute__((exclusive_trylock_function));  // \
-  // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}}
+void etf_function() __attribute__((exclusive_trylock_function)); // \
+  // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}} \
 
 void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
 
-void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1);
+void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute without arguments cannot be applied to a free function}}
 
-int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1);
+int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute without arguments cannot be applied to a free function}}
 
 int etf_testfn(int y) {
   int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \
 // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
   return x;
 };
 
 int etf_test_var EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
+  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} \
 
 class EtfFoo {
  private:
   int test_field EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
 // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1);
+  void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+// expected-warning {{'exclusive_trylock_function' attribute requires type annotated with 'capability' attribute; type here is 'EtfFoo *'}}
 };
 
 class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \
@@ -759,7 +763,8 @@
 int etf_function_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, muRef);
 int etf_function_7() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoubleWrapper.getWrapper()->getMu());
 int etf_functetfn_8() EXCLUSIVE_TRYLOCK_FUNCTION(1, muPointer);
-int etf_

[libcxx] r309881 - Rename a couple variables to eliminate a shadow warning. No functionality change

2017-08-02 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Wed Aug  2 13:29:26 2017
New Revision: 309881

URL: http://llvm.org/viewvc/llvm-project?rev=309881&view=rev
Log:
Rename a couple variables to eliminate a shadow warning. No functionality change

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

Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/operations.cpp?rev=309881&r1=309880&r2=309881&view=diff
==
--- libcxx/trunk/src/experimental/filesystem/operations.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/operations.cpp Wed Aug  2 13:29:26 
2017
@@ -182,20 +182,20 @@ void __copy(const path& from, const path
 const bool sym_status2 = bool(options &
 copy_options::copy_symlinks);
 
-std::error_code m_ec;
+std::error_code m_ec1;
 struct ::stat f_st = {};
 const file_status f = sym_status || sym_status2
- ? detail::posix_lstat(from, f_st, &m_ec)
- : detail::posix_stat(from,  f_st, &m_ec);
-if (m_ec)
-return set_or_throw(m_ec, ec, "copy", from, to);
+ ? detail::posix_lstat(from, f_st, &m_ec1)
+ : detail::posix_stat(from,  f_st, &m_ec1);
+if (m_ec1)
+return set_or_throw(m_ec1, ec, "copy", from, to);
 
 struct ::stat t_st = {};
-const file_status t = sym_status ? detail::posix_lstat(to, t_st, &m_ec)
- : detail::posix_stat(to, t_st, &m_ec);
+const file_status t = sym_status ? detail::posix_lstat(to, t_st, &m_ec1)
+ : detail::posix_stat(to, t_st, &m_ec1);
 
 if (not status_known(t))
-return set_or_throw(m_ec, ec, "copy", from, to);
+return set_or_throw(m_ec1, ec, "copy", from, to);
 
 if (!exists(f) || is_other(f) || is_other(t)
 || (is_directory(f) && is_regular_file(t))
@@ -249,9 +249,9 @@ void __copy(const path& from, const path
 directory_iterator it = ec ? directory_iterator(from, *ec)
: directory_iterator(from);
 if (ec && *ec) { return; }
-std::error_code m_ec;
-for (; it != directory_iterator(); it.increment(m_ec)) {
-if (m_ec) return set_or_throw(m_ec, ec, "copy", from, to);
+std::error_code m_ec2;
+for (; it != directory_iterator(); it.increment(m_ec2)) {
+if (m_ec2) return set_or_throw(m_ec2, ec, "copy", from, to);
 __copy(it->path(), to / it->path().filename(),
options | copy_options::__in_recursive_copy, ec);
 if (ec && *ec) { return; }


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


[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

2017-08-02 Thread Nico Weber via Phabricator via cfe-commits
thakis created this revision.

This helps some tools that do things based on the output's extension.

For example, we got reports from users on Windows that have a tool that scan a 
build output dir (but skip .obj files). The tool would keep the "foo.obj-12345" 
file open, and then when clang tried to rename the temp file to the final 
output filename, that would fail. By making the tempfile end in ".obj", tools 
like this will now skip the temp files as well.


https://reviews.llvm.org/D36238

Files:
  lib/Frontend/CompilerInstance.cpp


Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -759,9 +759,13 @@
 
   if (UseTemporary) {
 // Create a temporary file.
-SmallString<128> TempPath;
-TempPath = OutFile;
+// Insert - before the extension (if any), so that tools doing
+// things based on the file extension do the right thing.
+StringRef OutputExtension = llvm::sys::path::extension(OutFile);
+SmallString<128> TempPath =
+StringRef(OutFile).drop_back(OutputExtension.size());
 TempPath += "-";
+TempPath += OutputExtension;
 int fd;
 std::error_code EC =
 llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath);


Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -759,9 +759,13 @@
 
   if (UseTemporary) {
 // Create a temporary file.
-SmallString<128> TempPath;
-TempPath = OutFile;
+// Insert - before the extension (if any), so that tools doing
+// things based on the file extension do the right thing.
+StringRef OutputExtension = llvm::sys::path::extension(OutFile);
+SmallString<128> TempPath =
+StringRef(OutFile).drop_back(OutputExtension.size());
 TempPath += "-";
+TempPath += OutputExtension;
 int fd;
 std::error_code EC =
 llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36237: Reland "Thread Safety Analysis: fix assert_capability.", add warnings

2017-08-02 Thread Josh Gao via Phabricator via cfe-commits
jmgao updated this revision to Diff 109412.
jmgao added a comment.

Fix commit messages.


https://reviews.llvm.org/D36237

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/ThreadSafety.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-capabilities.c
  test/SemaCXX/warn-thread-safety-analysis.cpp
  test/SemaCXX/warn-thread-safety-parsing.cpp

Index: test/SemaCXX/warn-thread-safety-parsing.cpp
===
--- test/SemaCXX/warn-thread-safety-parsing.cpp
+++ test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -571,11 +571,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void elf_function() EXCLUSIVE_LOCK_FUNCTION();
+void elf_function() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without arguments cannot be applied to a free function}}
 
 void elf_function_args() EXCLUSIVE_LOCK_FUNCTION(mu1, mu2);
 
-int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION();
+int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without arguments cannot be applied to a free function}}
 
 int elf_testfn(int y) {
   int x EXCLUSIVE_LOCK_FUNCTION() = y; // \
@@ -590,7 +590,7 @@
  private:
   int test_field EXCLUSIVE_LOCK_FUNCTION(); // \
 // expected-warning {{'exclusive_lock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_LOCK_FUNCTION();
+  void test_method() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute requires type annotated with 'capability' attribute; type here is 'ElfFoo *'}}
 };
 
 class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
@@ -643,11 +643,11 @@
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void slf_function() SHARED_LOCK_FUNCTION();
+void slf_function() SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without arguments cannot be applied to a free function}}
 
 void slf_function_args() SHARED_LOCK_FUNCTION(mu1, mu2);
 
-int slf_testfn(int y) SHARED_LOCK_FUNCTION();
+int slf_testfn(int y) SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without arguments cannot be applied to a free function}}
 
 int slf_testfn(int y) {
   int x SHARED_LOCK_FUNCTION() = y; // \
@@ -665,7 +665,8 @@
  private:
   int test_field SHARED_LOCK_FUNCTION(); // \
 // expected-warning {{'shared_lock_function' attribute only applies to functions}}
-  void test_method() SHARED_LOCK_FUNCTION();
+  void test_method() SHARED_LOCK_FUNCTION(); // \
+// expected-warning {{'shared_lock_function' attribute requires type annotated with 'capability' attribute; type here is 'SlfFoo *'}}
 };
 
 class SHARED_LOCK_FUNCTION() SlfTestClass { // \
@@ -716,29 +717,32 @@
 // takes a mandatory boolean or integer argument specifying the retval
 // plus an optional list of locks (vars/fields)
 
-void etf_function() __attribute__((exclusive_trylock_function));  // \
-  // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}}
+void etf_function() __attribute__((exclusive_trylock_function)); // \
+  // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}} \
 
 void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
 
-void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1);
+void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute without arguments cannot be applied to a free function}}
 
-int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1);
+int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute without arguments cannot be applied to a free function}}
 
 int etf_testfn(int y) {
   int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \
 // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
   return x;
 };
 
 int etf_test_var EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
+  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} \
 
 class EtfFoo {
  private:
   int test_field EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
 // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1);
+  void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+// expected-warning {{'exclusive_trylock_function' attribute requires type annotated with 'capability' attribute; type here is 'EtfFoo *'}}
 };
 
 class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \
@@ -759,7 +763,8 @@
 int etf_function_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, muRef);
 int etf_function_7() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoubleWrapper.getWrapper()->getMu());
 int etf_functetfn_8() EXCLUSIVE_TRYLOCK_FUNCTION(1, muPointer);
-int etf_function_9() EXCL

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

2017-08-02 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 109406.
yaxunl marked 29 inline comments as done.
yaxunl added a comment.

Revised by reviewers' comments.


https://reviews.llvm.org/D28691

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Expr.h
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/SyncScope.h
  lib/AST/ASTContext.cpp
  lib/AST/Expr.cpp
  lib/AST/StmtPrinter.cpp
  lib/Basic/Targets/AMDGPU.cpp
  lib/CodeGen/CGAtomic.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/CodeGen/TargetInfo.h
  lib/Headers/opencl-c.h
  lib/Sema/SemaChecking.cpp
  test/CodeGenOpenCL/atomic-ops-libcall.cl
  test/CodeGenOpenCL/atomic-ops.cl
  test/SemaOpenCL/atomic-ops.cl

Index: test/SemaOpenCL/atomic-ops.cl
===
--- /dev/null
+++ test/SemaOpenCL/atomic-ops.cl
@@ -0,0 +1,156 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
+
+// Basic parsing/Sema tests for __opencl_atomic_*
+
+#pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
+#pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
+
+struct S { char c[3]; };
+
+char i8;
+short i16;
+int i32;
+int8 i64;
+
+atomic_int gn;
+
+void f(atomic_int *i, const atomic_int *ci,
+   atomic_intptr_t *p, atomic_float *d,
+   int *I, const int *CI,
+   intptr_t *P, float *D, struct S *s1, struct S *s2,
+   global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
+   constant atomic_int *i_c) {
+  __opencl_atomic_init(I, 5); // expected-error {{address argument to atomic operation must be a pointer to _Atomic type ('__generic int *' invalid)}}
+  __opencl_atomic_init(ci, 5); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const __generic atomic_int *' (aka 'const __generic _Atomic(int) *') invalid)}}
+
+  __opencl_atomic_load(0); // expected-error {{too few arguments to function call, expected 3, have 1}}
+  __opencl_atomic_load(0, 0, 0, 0); // expected-error {{too many arguments to function call, expected 3, have 4}}
+  __opencl_atomic_store(0,0,0,0); // expected-error {{address argument to atomic builtin must be a pointer}}
+  __opencl_atomic_store((int *)0, 0, 0, 0); // expected-error {{address argument to atomic operation must be a pointer to _Atomic type ('__generic int *' invalid)}}
+  __opencl_atomic_store(i, 0, memory_order_relaxed, memory_scope_work_item);
+  __opencl_atomic_store(ci, 0, memory_order_relaxed, memory_scope_work_item); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const __generic atomic_int *' (aka 'const __generic _Atomic(int) *') invalid)}}
+  __opencl_atomic_store(i_g, 0, memory_order_relaxed, memory_scope_work_item);
+  __opencl_atomic_store(i_l, 0, memory_order_relaxed, memory_scope_work_item);
+  __opencl_atomic_store(i_p, 0, memory_order_relaxed, memory_scope_work_item);
+  __opencl_atomic_store(i_c, 0, memory_order_relaxed, memory_scope_work_item); // expected-error {{address argument to atomic operation must be a pointer to non-constant _Atomic type ('__constant atomic_int *' (aka '__constant _Atomic(int) *') invalid)}}
+
+  __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_item);
+  __opencl_atomic_load(p, memory_order_seq_cst, memory_scope_work_item);
+  __opencl_atomic_load(d, memory_order_seq_cst, memory_scope_work_item);
+  __opencl_atomic_load(ci, memory_order_seq_cst, memory_scope_work_item); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const __generic atomic_int *' (aka 'const __generic _Atomic(int) *') invalid)}}
+
+  __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_item);
+  __opencl_atomic_store(p, 1, memory_order_seq_cst, memory_scope_work_item);
+  (int)__opencl_atomic_store(d, 1, memory_order_seq_cst, memory_scope_work_item); // expected-error {{operand of type 'void' where arithmetic or pointer type is required}}
+
+  int exchange_1 = __opencl_atomic_exchange(i, 1, memory_order_seq_cst, memory_scope_work_item);
+  int exchange_2 = __opencl_atomic_exchange(I, 1, memory_order_seq_cst, memory_scope_work_item); // expected-error {{address argument to atomic operation must be a pointer to _Atomic}}
+
+  __opencl_atomic_fetch_add(i, 1, memory_order_seq_cst, memory_scope_work_item);
+  __opencl_atomic_fetch_add(p, 1, memory_order_seq_cst, memory_scope_work_item);
+  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_item); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_item);
+  _

[PATCH] D36176: [clang-diff] Fix some errors and inconsistencies

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109414.
johannes added a comment.

move most functional changes to other commits
move constructor for Syntaxtree


https://reviews.llvm.org/D36176

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp

Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -27,6 +27,7 @@
 namespace clang {
 namespace diff {
 
+namespace {
 /// Maps nodes of the left tree to ones on the right, and vice versa.
 class Mapping {
 public:
@@ -76,6 +77,7 @@
 private:
   std::unique_ptr[]> SrcToDst, DstToSrc;
 };
+} // end anonymous namespace
 
 class ASTDiff::Impl {
 public:
@@ -110,10 +112,6 @@
   // Returns false if the nodes must not be mached.
   bool isMatchingPossible(NodeId Id1, NodeId Id2) const;
 
-  // Adds all corresponding subtrees of the two nodes to the mapping.
-  // The two nodes must be identical.
-  void addIsomorphicSubTrees(Mapping &M, NodeId Id1, NodeId Id2) const;
-
   // Uses an optimal albeit slow algorithm to compute a mapping between two
   // subtrees, but only if both have fewer nodes than MaxSize.
   void addOptimalMapping(Mapping &M, NodeId Id1, NodeId Id2) const;
@@ -254,7 +252,10 @@
 Parent = PreviousParent;
 --Depth;
 Node &N = Tree.getMutableNode(MyId);
-N.RightMostDescendant = Id;
+N.RightMostDescendant = Id - 1;
+assert(N.RightMostDescendant >= 0 &&
+   N.RightMostDescendant < Tree.getSize() &&
+   "Rightmost descendant must be a valid tree node.");
 if (N.isLeaf())
   Tree.Leaves.push_back(MyId);
 N.Height = 1;
@@ -358,9 +359,7 @@
 }
 
 bool SyntaxTree::Impl::isInSubtree(NodeId Id, NodeId SubtreeRoot) const {
-  NodeId Lower = SubtreeRoot;
-  NodeId Upper = getNode(SubtreeRoot).RightMostDescendant;
-  return Id >= Lower && Id <= Upper;
+  return Id >= SubtreeRoot && Id <= getNode(SubtreeRoot).RightMostDescendant;
 }
 
 std::string SyntaxTree::Impl::getNodeValue(NodeId Id) const {
@@ -625,12 +624,11 @@
   }
 
 private:
-  /// Simple cost model for edit actions.
+  /// We use a simple cost model for edit actions, which seems good enough.
+  /// Simple cost model for edit actions. This seems to make the matching
+  /// algorithm perform reasonably well.
   /// The values range between 0 and 1, or infinity if this edit action should
   /// always be avoided.
-
-  /// These costs could be modified to better model the estimated cost of /
-  /// inserting / deleting the current node.
   static constexpr double DeletionCost = 1;
   static constexpr double InsertionCost = 1;
 
@@ -687,6 +685,7 @@
 };
 } // end anonymous namespace
 
+namespace {
 // Priority queue for nodes, sorted descendingly by their height.
 class PriorityList {
   const SyntaxTree::Impl &Tree;
@@ -723,6 +722,7 @@
   push(Child);
   }
 };
+} // end anonymous namespace
 
 bool ASTDiff::Impl::identical(NodeId Id1, NodeId Id2) const {
   const Node &N1 = T1.getNode(Id1);
@@ -759,16 +759,6 @@
T2.getNode(Id2).ASTNode);
 }
 
-void ASTDiff::Impl::addIsomorphicSubTrees(Mapping &M, NodeId Id1,
-  NodeId Id2) const {
-  assert(identical(Id1, Id2) && "Can only be called on identical subtrees.");
-  M.link(Id1, Id2);
-  const Node &N1 = T1.getNode(Id1);
-  const Node &N2 = T2.getNode(Id2);
-  for (size_t Id = 0, E = N1.Children.size(); Id < E; ++Id)
-addIsomorphicSubTrees(M, N1.Children[Id], N2.Children[Id]);
-}
-
 void ASTDiff::Impl::addOptimalMapping(Mapping &M, NodeId Id1,
   NodeId Id2) const {
   if (std::max(T1.getNumberOfDescendants(Id1),
@@ -805,7 +795,7 @@
 if (M.hasDst(Id2))
   continue;
 double Similarity = getSimilarity(M, Id1, Id2);
-if (Similarity > HighestSimilarity) {
+if (Similarity >= Options.MinSimilarity && Similarity > HighestSimilarity) {
   HighestSimilarity = Similarity;
   Candidate = Id2;
 }
@@ -816,7 +806,8 @@
 void ASTDiff::Impl::matchBottomUp(Mapping &M) const {
   std::vector Postorder = getSubtreePostorder(T1, T1.getRootId());
   for (NodeId Id1 : Postorder) {
-if (Id1 == T1.getRootId()) {
+if (Id1 == T1.getRootId() && !M.hasSrc(T1.getRootId()) &&
+!M.hasDst(T2.getRootId())) {
   if (isMatchingPossible(T1.getRootId(), T2.getRootId())) {
 M.link(T1.getRootId(), T2.getRootId());
 addOptimalMapping(M, T1.getRootId(), T2.getRootId());
@@ -831,11 +822,10 @@
 if (Matched || !MatchedChildren)
   continue;
 NodeId Id2 = findCandidate(M, Id1);
-if (Id2.isInvalid() || !canBeAddedToMapping(M, Id1, Id2) ||
-getSimilarity(M, Id1, Id2) < Options.MinSimilarity)
-  continue;
-M.link(Id1, Id2);
-addOptimalMapping(M, Id1, Id2);
+if (Id2.isValid() && canBeAddedToMapping(M, Id1, Id2)) {
+  M.link(Id1, Id2);
+

r309885 - Update for llvm change.

2017-08-02 Thread Rafael Espindola via cfe-commits
Author: rafael
Date: Wed Aug  2 13:32:35 2017
New Revision: 309885

URL: http://llvm.org/viewvc/llvm-project?rev=309885&view=rev
Log:
Update for llvm change.

Modified:
cfe/trunk/lib/Parse/ParseStmtAsm.cpp
cfe/trunk/tools/driver/cc1as_main.cpp

Modified: cfe/trunk/lib/Parse/ParseStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmtAsm.cpp?rev=309885&r1=309884&r2=309885&view=diff
==
--- cfe/trunk/lib/Parse/ParseStmtAsm.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmtAsm.cpp Wed Aug  2 13:32:35 2017
@@ -578,8 +578,7 @@ StmtResult Parser::ParseMicrosoftAsmStat
 
   llvm::SourceMgr TempSrcMgr;
   llvm::MCContext Ctx(MAI.get(), MRI.get(), MOFI.get(), &TempSrcMgr);
-  MOFI->InitMCObjectFileInfo(TheTriple, /*PIC*/ false, 
llvm::CodeModel::Default,
- Ctx);
+  MOFI->InitMCObjectFileInfo(TheTriple, /*PIC*/ false, Ctx);
   std::unique_ptr Buffer =
   llvm::MemoryBuffer::getMemBuffer(AsmString, "");
 

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=309885&r1=309884&r2=309885&view=diff
==
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Wed Aug  2 13:32:35 2017
@@ -356,7 +356,7 @@ static bool ExecuteAssembler(AssemblerIn
 PIC = false;
   }
 
-  MOFI->InitMCObjectFileInfo(Triple(Opts.Triple), PIC, CodeModel::Default, 
Ctx);
+  MOFI->InitMCObjectFileInfo(Triple(Opts.Triple), PIC, Ctx);
   if (Opts.SaveTemporaryLabels)
 Ctx.setAllowTemporaryLabels(false);
   if (Opts.GenDwarfForAssembly)


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


[PATCH] D27827: [ObjC] CodeGen support for @available on macOS

2017-08-02 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

We just noticed that if you call __builtin_available() for the first time after 
activating your app's sandbox, the function will fail:

SandboxViolation: crdmg(15489) deny file-read-data 
/System/Library/CoreServices/SystemVersion.plist
Violation:   deny file-read-data 
/System/Library/CoreServices/SystemVersion.plist 
Process: crdmg [15489]
Path:/Volumes/Build/src/./out/release/crdmg

Thread 0 (id: 421251):
0   libsystem_kernel.dylib  0x7fffe94a1a86 __open_nocancel + 10
1   crdmg   0x00010444be98 
parseSystemVersionPList + 360
2   0xec83485354415541


Repository:
  rL LLVM

https://reviews.llvm.org/D27827



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


[PATCH] D36177: [clang-diff] Add commandline arguments.

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109415.
johannes added a comment.

add some test, replace -no-compilation-database with --


https://reviews.llvm.org/D36177

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-args.sh
  test/Tooling/clang-diff-basic.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -28,12 +28,6 @@
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
-static cl::opt NoCompilationDatabase(
-"no-compilation-database",
-cl::desc(
-"Do not attempt to load build settings from a compilation database"),
-cl::init(false), cl::cat(ClangDiffCategory));
-
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
cl::Required,
cl::cat(ClangDiffCategory));
@@ -43,43 +37,88 @@
 cl::Optional,
 cl::cat(ClangDiffCategory));
 
-static std::unique_ptr getAST(const StringRef Filename) {
+static cl::opt StopAfter("stop-after",
+  cl::desc(""),
+  cl::Optional, cl::init(""),
+  cl::cat(ClangDiffCategory));
+
+static cl::opt MaxSize("s", cl::desc(""), cl::Optional,
+cl::init(-1), cl::cat(ClangDiffCategory));
+
+static cl::opt BuildPath("p", cl::desc("Build path"), cl::init(""),
+  cl::Optional, cl::cat(ClangDiffCategory));
+
+static cl::list ArgsAfter(
+"extra-arg",
+cl::desc("Additional argument to append to the compiler command line"),
+cl::cat(ClangDiffCategory));
+
+static cl::list ArgsBefore(
+"extra-arg-before",
+cl::desc("Additional argument to prepend to the compiler command line"),
+cl::cat(ClangDiffCategory));
+
+static void addExtraArgs(std::unique_ptr &Compilations) {
+  if (!Compilations)
+return;
+  auto AdjustingCompilations =
+  llvm::make_unique(
+  std::move(Compilations));
+  AdjustingCompilations->appendArgumentsAdjuster(
+  getInsertArgumentAdjuster(ArgsBefore, ArgumentInsertPosition::BEGIN));
+  AdjustingCompilations->appendArgumentsAdjuster(
+  getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
+  Compilations = std::move(AdjustingCompilations);
+}
+
+static std::unique_ptr
+getAST(const std::unique_ptr &CommonCompilations,
+   const StringRef Filename) {
   std::string ErrorMessage;
   std::unique_ptr Compilations;
-  if (!NoCompilationDatabase)
-Compilations =
-CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
-  if (!Compilations) {
-if (!NoCompilationDatabase)
+  if (!CommonCompilations) {
+Compilations = CompilationDatabase::autoDetectFromSource(
+BuildPath.empty() ? Filename : BuildPath, ErrorMessage);
+if (!Compilations) {
   llvm::errs()
   << "Error while trying to load a compilation database, running "
  "without flags.\n"
   << ErrorMessage;
-Compilations = llvm::make_unique(
-".", std::vector());
+  Compilations =
+  llvm::make_unique(
+  ".", std::vector());
+}
   }
+  addExtraArgs(Compilations);
   std::array Files = {{Filename}};
-  ClangTool Tool(*Compilations, Files);
+  ClangTool Tool(Compilations ? *Compilations : *CommonCompilations, Files);
   std::vector> ASTs;
   Tool.buildASTs(ASTs);
   if (ASTs.size() != Files.size())
 return nullptr;
   return std::move(ASTs[0]);
 }
 
 int main(int argc, const char **argv) {
+  std::string ErrorMessage;
+  std::unique_ptr CommonCompilations =
+  FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage);
+  if (!CommonCompilations && !ErrorMessage.empty())
+llvm::errs() << ErrorMessage;
   cl::HideUnrelatedOptions(ClangDiffCategory);
   if (!cl::ParseCommandLineOptions(argc, argv)) {
 cl::PrintOptionValues();
 return 1;
   }
 
+  addExtraArgs(CommonCompilations);
+
   if (ASTDump) {
 if (!DestinationPath.empty()) {
   llvm::errs() << "Error: Please specify exactly one filename.\n";
   return 1;
 }
-std::unique_ptr AST = getAST(SourcePath);
+std::unique_ptr AST = getAST(CommonCompilations, SourcePath);
 if (!AST)
   return 1;
 diff::SyntaxTree Tree(AST->getASTContext());
@@ -92,12 +131,22 @@
 return 1;
   }
 
-  std::unique_ptr Src = getAST(SourcePath);
-  std::unique_ptr Dst = getAST(DestinationPath);
+  std::unique_ptr Src = getAST(CommonCompilations, SourcePath);
+  std::unique_ptr Dst = getAST(CommonCompilations, DestinationPath);
   if (!Src || !Dst)
 return 1;
 
   diff::ComparisonOptions Options;
+ 

[PATCH] D36178: [clang-diff] Move the JSON export function to clang-diff

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109416.
johannes added a comment.

getSourceRangeOffsets, test


https://reviews.llvm.org/D36178

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-json.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -99,6 +99,65 @@
   return std::move(ASTs[0]);
 }
 
+static char hexdigit(int N) { return N &= 0xf, N + (N < 10 ? '0' : 'a' - 10); }
+
+static void printJsonString(raw_ostream &OS, const StringRef Str) {
+  for (char C : Str) {
+switch (C) {
+case '"':
+  OS << R"(\")";
+  break;
+case '\\':
+  OS << R"(\\)";
+  break;
+case '\n':
+  OS << R"(\n)";
+  break;
+case '\t':
+  OS << R"(\t)";
+  break;
+default:
+  if ('\x00' <= C && C <= '\x1f') {
+OS << R"(\u00)" << hexdigit(C >> 4) << hexdigit(C);
+  } else {
+OS << C;
+  }
+}
+  }
+}
+
+static void printNodeAttributes(raw_ostream &OS, diff::SyntaxTree &Tree,
+diff::NodeId Id) {
+  const diff::Node &N = Tree.getNode(Id);
+  OS << R"("id":)" << int(Id);
+  OS << R"(,"type":")" << N.getTypeLabel() << '"';
+  auto Offsets = Tree.getSourceRangeOffsets(N);
+  OS << R"(,"begin":)" << Offsets.first;
+  OS << R"(,"end":)" << Offsets.second;
+  std::string Value = Tree.getNodeValue(N.ASTNode);
+  if (!Value.empty()) {
+OS << R"(,"value":")";
+printJsonString(OS, Value);
+OS << '"';
+  }
+}
+
+static void printNodeAsJson(raw_ostream &OS, diff::SyntaxTree &Tree,
+diff::NodeId Id) {
+  const diff::Node &N = Tree.getNode(Id);
+  OS << "{";
+  printNodeAttributes(OS, Tree, Id);
+  OS << R"(,"children":[)";
+  if (N.Children.size() > 0) {
+printNodeAsJson(OS, Tree, N.Children[0]);
+for (size_t I = 1, E = N.Children.size(); I < E; ++I) {
+  OS << ",";
+  printNodeAsJson(OS, Tree, N.Children[I]);
+}
+  }
+  OS << "]}";
+}
+
 int main(int argc, const char **argv) {
   std::string ErrorMessage;
   std::unique_ptr CommonCompilations =
@@ -122,7 +181,11 @@
 if (!AST)
   return 1;
 diff::SyntaxTree Tree(AST->getASTContext());
-Tree.printAsJson(llvm::outs());
+llvm::outs() << R"({"filename":")";
+printJsonString(llvm::outs(), SourcePath);
+llvm::outs() << R"(","root":)";
+printNodeAsJson(llvm::outs(), Tree, Tree.getRootId());
+llvm::outs() << "}\n";
 return 0;
   }
 
Index: test/Tooling/clang-diff-json.cpp
===
--- /dev/null
+++ test/Tooling/clang-diff-json.cpp
@@ -0,0 +1,21 @@
+// RUN: clang-diff -ast-dump %s -- | python -m json.tool | FileCheck %s
+
+// CHECK: "type": "CXXRecordDecl",
+// CHECK: "begin": 185,
+// CHECK: "end": 205,
+// CHECK: "type": "FieldDecl",
+class A {
+  int x;
+};
+
+// CHECK: "type": "VarDecl",
+// CHECK: "children": [
+// CHECK: "type": "CharacterLiteral",
+// CHECK: "children": []
+// CHECK-NOT: "children": [
+// CHECK: ]
+char nl = '\n';
+
+// CHECK: "value": "abc \n\t\u\u001f\udcc4\udca3 \udceb\udc8d\udcb0\udceb\udcb0\udc95"
+char s[] = "abc \n\t\0\x1f\u0123 데박";
+
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -176,9 +176,6 @@
   void printTree(NodeId Root) const;
   void printTree(raw_ostream &OS, NodeId Root) const;
 
-  void printAsJsonImpl(raw_ostream &OS) const;
-  void printNodeAsJson(raw_ostream &OS, NodeId Id) const;
-
 private:
   /// Nodes in preorder.
   std::vector Nodes;
@@ -438,28 +435,6 @@
   OS << "(" << PostorderIds[Id] << ")";
 }
 
-void SyntaxTree::Impl::printNodeAsJson(raw_ostream &OS, NodeId Id) const {
-  auto N = getNode(Id);
-  OS << R"({"type":")" << N.getTypeLabel() << R"(")";
-  if (getNodeValue(Id) != "")
-OS << R"(,"value":")" << getNodeValue(Id) << R"(")";
-  OS << R"(,"children":[)";
-  if (N.Children.size() > 0) {
-printNodeAsJson(OS, N.Children[0]);
-for (size_t I = 1, E = N.Children.size(); I < E; ++I) {
-  OS << ",";
-  printNodeAsJson(OS, N.Children[I]);
-}
-  }
-  OS << "]}";
-}
-
-void SyntaxTree::Impl::printAsJsonImpl(raw_ostream &OS) const {
-  OS << R"({"root":)";
-  printNodeAsJson(OS, getRootId());
-  OS << "}\n";
-}
-
 /// Identifies a node in a subtree by its postorder offset, starting at 1.
 struct SNodeId {
   int Id = 0;
@@ -674,6 +649,12 @@
   }
 };
 
+ast_type_traits::ASTNodeKind Node::getType() const {
+  return ASTNode.getNodeKind();
+}
+
+StringRef Node::getTypeLabel() const { return getType().asStringRef(); }
+
 namespace {
 // Compares nodes by their depth.
 struct HeightLess {
@@ -1001,7 +982,28 @@
 
 SyntaxTree::~SyntaxTree() = default;
 
-void SyntaxTree::printAsJson(raw_ostream &OS) { TreeImpl->p

[PATCH] D36179: [clang-diff] Move printing of matches and changes to clang-diff

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109417.
johannes added a comment.

remove unused SubtreeIterator


https://reviews.llvm.org/D36179

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-basic.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -134,7 +134,7 @@
   auto Offsets = Tree.getSourceRangeOffsets(N);
   OS << R"(,"begin":)" << Offsets.first;
   OS << R"(,"end":)" << Offsets.second;
-  std::string Value = Tree.getNodeValue(N.ASTNode);
+  std::string Value = Tree.getNodeValue(N);
   if (!Value.empty()) {
 OS << R"(,"value":")";
 printJsonString(OS, Value);
@@ -158,6 +158,52 @@
   OS << "]}";
 }
 
+static void printNode(raw_ostream &OS, diff::SyntaxTree &Tree,
+  diff::NodeId Id) {
+  if (Id.isInvalid()) {
+OS << "None";
+return;
+  }
+  OS << Tree.getNode(Id).getTypeLabel();
+  std::string Value = Tree.getNodeValue(Id);
+  if (!Value.empty())
+OS << ": " << Value;
+  OS << "(" << Id << ")";
+}
+
+static void printDstChange(raw_ostream &OS, diff::ASTDiff &Diff,
+   diff::SyntaxTree &SrcTree, diff::SyntaxTree &DstTree,
+   diff::NodeId Dst) {
+  const diff::Node &DstNode = DstTree.getNode(Dst);
+  diff::NodeId Src = Diff.getMapped(DstTree, Dst);
+  switch (DstNode.ChangeKind) {
+  case diff::None:
+break;
+  case diff::Delete:
+llvm_unreachable("The destination tree can't have deletions.");
+  case diff::Update:
+OS << "Update ";
+printNode(OS, SrcTree, Src);
+OS << " to " << DstTree.getNodeValue(Dst) << "\n";
+break;
+  case diff::Insert:
+  case diff::Move:
+  case diff::UpdateMove:
+if (DstNode.ChangeKind == diff::Insert)
+  OS << "Insert";
+else if (DstNode.ChangeKind == diff::Move)
+  OS << "Move";
+else if (DstNode.ChangeKind == diff::UpdateMove)
+  OS << "Update and Move";
+OS << " ";
+printNode(OS, DstTree, Dst);
+OS << " into ";
+printNode(OS, DstTree, DstNode.Parent);
+OS << " at " << DstTree.findPositionInParent(Dst) << "\n";
+break;
+  }
+}
+
 int main(int argc, const char **argv) {
   std::string ErrorMessage;
   std::unique_ptr CommonCompilations =
@@ -212,11 +258,26 @@
   }
   diff::SyntaxTree SrcTree(Src->getASTContext());
   diff::SyntaxTree DstTree(Dst->getASTContext());
-  diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
-  for (const auto &Match : DiffTool.getMatches())
-DiffTool.printMatch(llvm::outs(), Match);
-  for (const auto &Change : DiffTool.getChanges())
-DiffTool.printChange(llvm::outs(), Change);
+  diff::ASTDiff Diff(SrcTree, DstTree, Options);
+
+  for (diff::NodeId Dst : DstTree) {
+diff::NodeId Src = Diff.getMapped(DstTree, Dst);
+if (Src.isValid()) {
+  llvm::outs() << "Match ";
+  printNode(llvm::outs(), SrcTree, Src);
+  llvm::outs() << " to ";
+  printNode(llvm::outs(), DstTree, Dst);
+  llvm::outs() << "\n";
+}
+printDstChange(llvm::outs(), Diff, SrcTree, DstTree, Dst);
+  }
+  for (diff::NodeId Src : SrcTree) {
+if (Diff.getMapped(SrcTree, Src).isInvalid()) {
+  llvm::outs() << "Delete ";
+  printNode(llvm::outs(), SrcTree, Src);
+  llvm::outs() << "\n";
+}
+  }
 
   return 0;
 }
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -54,8 +54,8 @@
 typedef unsigned nat;
 
 // CHECK: Match VarDecl: p(int){{.*}} to VarDecl: prod(double)
-// CHECK: Match BinaryOperator: *{{.*}} to BinaryOperator: *
 // CHECK: Update VarDecl: p(int){{.*}} to prod(double)
+// CHECK: Match BinaryOperator: *{{.*}} to BinaryOperator: *
 double prod = 1 * 2 * 10;
 // CHECK: Update DeclRefExpr
 int squared = prod * prod;
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -82,26 +82,23 @@
 class ASTDiff::Impl {
 public:
   SyntaxTree::Impl &T1, &T2;
-  bool IsMappingDone = false;
   Mapping TheMapping;
 
   Impl(SyntaxTree::Impl &T1, SyntaxTree::Impl &T2,
-   const ComparisonOptions &Options)
-  : T1(T1), T2(T2), Options(Options) {}
+   const ComparisonOptions &Options);
 
   /// Matches nodes one-by-one based on their similarity.
   void computeMapping();
 
-  std::vector getMatches(Mapping &M);
+  // Compute ChangeKind for each node based on similarity.
+  void computeChangeKinds(Mapping &M);
 
-  /// Finds an edit script that converts T1 to T2.
-  std::vector computeChanges(Mapping &M);
-
-  void printChangeImpl(raw_ostream &OS, const Change &Chg) const;
-  void printMatchImpl(raw_ostream &OS, const Match &M) const;
-
-  

r309873 - [UBSan] Provide default blacklist filename for UBSan

2017-08-02 Thread Han Shen via cfe-commits
Author: shenhan
Date: Wed Aug  2 12:53:38 2017
New Revision: 309873

URL: http://llvm.org/viewvc/llvm-project?rev=309873&view=rev
Log:
[UBSan] Provide default blacklist filename for UBSan

Summary:
This is to provide a default blacklist filename for UBSan.

While UBSan is turned on, it's better that clang pick up a blacklist file (when 
exists), just as what ASan / MSan does, so we do not end up adding the 
"-fsanitize-blacklist" option to every command line.

Reviewers: chandlerc, echristo, vsk, eugenis

Reviewed By: vsk, eugenis

Subscribers: vsk, eugenis, echristo, cfe-commits

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

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=309873&r1=309872&r2=309873&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Aug  2 12:53:38 2017
@@ -101,6 +101,8 @@ static bool getDefaultBlacklist(const Dr
 BlacklistFile = "dfsan_abilist.txt";
   else if (Kinds & CFI)
 BlacklistFile = "cfi_blacklist.txt";
+  else if (Kinds & Undefined)
+BlacklistFile = "ubsan_blacklist.txt";
 
   if (BlacklistFile) {
 clang::SmallString<64> Path(D.ResourceDir);


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


[PATCH] D36181: [clang-diff] Make printing of matches optional

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109420.
johannes added a comment.

rename to -dump-matches, add a test


https://reviews.llvm.org/D36181

Files:
  test/Tooling/clang-diff-args.sh
  test/Tooling/clang-diff-basic.cpp
  tools/clang-diff/ClangDiff.cpp


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -33,6 +33,10 @@
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
+static cl::opt
+PrintMatches("dump-matches", cl::desc("Print the matched nodes."),
+ cl::init(false), cl::cat(ClangDiffCategory));
+
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
cl::Required,
cl::cat(ClangDiffCategory));
@@ -280,7 +284,7 @@
 
   for (diff::NodeId Dst : DstTree) {
 diff::NodeId Src = Diff.getMapped(DstTree, Dst);
-if (Src.isValid()) {
+if (PrintMatches && Src.isValid()) {
   llvm::outs() << "Match ";
   printNode(llvm::outs(), SrcTree, Src);
   llvm::outs() << " to ";
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %T/src.cpp
 // RUN: %clang_cc1 -E %s > %T/dst.cpp -DDEST
-// RUN: clang-diff %T/src.cpp %T/dst.cpp -- | FileCheck %s
+// RUN: clang-diff -dump-matches %T/src.cpp %T/dst.cpp -- | FileCheck %s
 
 #ifndef DEST
 namespace src {
Index: test/Tooling/clang-diff-args.sh
===
--- test/Tooling/clang-diff-args.sh
+++ test/Tooling/clang-diff-args.sh
@@ -6,3 +6,7 @@
 RUN: clang-diff -ast-dump -extra-arg=-Da=X%t.cpp -- 2>&1 | FileCheck 
%t1
 RUN: clang-diff -ast-dump -extra-arg-before=-Da=X %t.cpp -- 2>&1 | FileCheck 
%t1
 RUN: clang-diff -ast-dump %t.cpp -- 2>&1 -Da=X | FileCheck %t1
+
+RUN: echo "// CHECK-NOT: {{.}}" > %t-no-output
+RUN: clang-diff %S/clang-diff-ast.cpp %S/clang-diff-ast.cpp -- 2>&1 -std=c++11 
\
+RUN: | FileCheck -allow-empty %t-no-output


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -33,6 +33,10 @@
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
+static cl::opt
+PrintMatches("dump-matches", cl::desc("Print the matched nodes."),
+ cl::init(false), cl::cat(ClangDiffCategory));
+
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
cl::Required,
cl::cat(ClangDiffCategory));
@@ -280,7 +284,7 @@
 
   for (diff::NodeId Dst : DstTree) {
 diff::NodeId Src = Diff.getMapped(DstTree, Dst);
-if (Src.isValid()) {
+if (PrintMatches && Src.isValid()) {
   llvm::outs() << "Match ";
   printNode(llvm::outs(), SrcTree, Src);
   llvm::outs() << " to ";
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %T/src.cpp
 // RUN: %clang_cc1 -E %s > %T/dst.cpp -DDEST
-// RUN: clang-diff %T/src.cpp %T/dst.cpp -- | FileCheck %s
+// RUN: clang-diff -dump-matches %T/src.cpp %T/dst.cpp -- | FileCheck %s
 
 #ifndef DEST
 namespace src {
Index: test/Tooling/clang-diff-args.sh
===
--- test/Tooling/clang-diff-args.sh
+++ test/Tooling/clang-diff-args.sh
@@ -6,3 +6,7 @@
 RUN: clang-diff -ast-dump -extra-arg=-Da=X%t.cpp -- 2>&1 | FileCheck %t1
 RUN: clang-diff -ast-dump -extra-arg-before=-Da=X %t.cpp -- 2>&1 | FileCheck %t1
 RUN: clang-diff -ast-dump %t.cpp -- 2>&1 -Da=X | FileCheck %t1
+
+RUN: echo "// CHECK-NOT: {{.}}" > %t-no-output
+RUN: clang-diff %S/clang-diff-ast.cpp %S/clang-diff-ast.cpp -- 2>&1 -std=c++11 \
+RUN: | FileCheck -allow-empty %t-no-output
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36180: [clang-diff] Add option to dump the AST, one node per line

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109419.
johannes added a comment.
Herald added a subscriber: klimek.

add a test


https://reviews.llvm.org/D36180

Files:
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-json.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -25,9 +25,14 @@
 
 static cl::opt
 ASTDump("ast-dump",
-cl::desc("Print the internal representation of the AST as JSON."),
+cl::desc("Print the internal representation of the AST."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
+static cl::opt ASTDumpJson(
+"ast-dump-json",
+cl::desc("Print the internal representation of the AST as JSON."),
+cl::init(false), cl::cat(ClangDiffCategory));
+
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
cl::Required,
cl::cat(ClangDiffCategory));
@@ -171,6 +176,15 @@
   OS << "(" << Id << ")";
 }
 
+static void printTree(raw_ostream &OS, diff::SyntaxTree &Tree) {
+  for (diff::NodeId Id : Tree) {
+for (int I = 0; I < Tree.getNode(Id).Depth; ++I)
+  OS << " ";
+printNode(OS, Tree, Id);
+OS << "\n";
+  }
+}
+
 static void printDstChange(raw_ostream &OS, diff::ASTDiff &Diff,
diff::SyntaxTree &SrcTree, diff::SyntaxTree &DstTree,
diff::NodeId Dst) {
@@ -218,15 +232,19 @@
 
   addExtraArgs(CommonCompilations);
 
-  if (ASTDump) {
+  if (ASTDump || ASTDumpJson) {
 if (!DestinationPath.empty()) {
   llvm::errs() << "Error: Please specify exactly one filename.\n";
   return 1;
 }
 std::unique_ptr AST = getAST(CommonCompilations, SourcePath);
 if (!AST)
   return 1;
 diff::SyntaxTree Tree(AST->getASTContext());
+if (ASTDump) {
+  printTree(llvm::outs(), Tree);
+  return 0;
+}
 llvm::outs() << R"({"filename":")";
 printJsonString(llvm::outs(), SourcePath);
 llvm::outs() << R"(","root":)";
Index: test/Tooling/clang-diff-json.cpp
===
--- test/Tooling/clang-diff-json.cpp
+++ test/Tooling/clang-diff-json.cpp
@@ -1,8 +1,8 @@
-// RUN: clang-diff -ast-dump %s -- | python -m json.tool | FileCheck %s
+// RUN: clang-diff -ast-dump-json %s -- | python -m json.tool | FileCheck %s
 
 // CHECK: "type": "CXXRecordDecl",
-// CHECK: "begin": 185,
-// CHECK: "end": 205,
+// CHECK: "begin": 190,
+// CHECK: "end": 210,
 // CHECK: "type": "FieldDecl",
 class A {
   int x;
Index: test/Tooling/clang-diff-ast.cpp
===
--- /dev/null
+++ test/Tooling/clang-diff-ast.cpp
@@ -0,0 +1,50 @@
+// RUN: clang-diff -ast-dump %s -- -std=c++11 | FileCheck %s
+
+
+// CHECK: {{^}}TranslationUnitDecl(0)
+// CHECK: {{^}} NamespaceDecl: test;(
+namespace test {
+
+// CHECK: {{^}}  FunctionDecl: f(
+// CHECK: CompoundStmt(
+void f() {
+  // CHECK: VarDecl: i(int)(
+  // CHECK: IntegerLiteral: 1
+  auto i = 1;
+  // CHECK: CallExpr(
+  // CHECK: DeclRefExpr: f(
+  f();
+  // CHECK: BinaryOperator: =(
+  i = i;
+}
+
+} // end namespace test
+
+// CHECK: TypedefDecl: nat;unsigned int;(
+typedef unsigned nat;
+// CHECK: TypeAliasDecl: real;double;(
+using real = double;
+
+class Base {
+};
+
+// CHECK: CXXRecordDecl: X;class X;(
+class X : Base {
+  int m;
+  // CHECK: CXXMethodDecl: foo(const char *(int))(
+  // CHECK: ParmVarDecl: i(int)(
+  const char *foo(int i) {
+if (i == 0)
+  // CHECK: StringLiteral: foo(
+  return "foo";
+return 0;
+  }
+
+  // CHECK: AccessSpecDecl: public(
+public:
+  // CHECK: CXXConstructorDecl: X(void (char, int))(
+  X(char, int) : Base(), m(0) {
+// CHECK: MemberExpr(
+int x = m;
+  }
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36182: [clang-diff] Add HTML side-by-side diff output

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109421.
johannes added a comment.

-


https://reviews.llvm.org/D36182

Files:
  test/Tooling/Inputs/clang-diff-basic-src.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-html.py
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -37,6 +37,10 @@
 PrintMatches("dump-matches", cl::desc("Print the matched nodes."),
  cl::init(false), cl::cat(ClangDiffCategory));
 
+static cl::opt HtmlDiff("html",
+  cl::desc("Output a side-by-side diff in HTML."),
+  cl::init(false), cl::cat(ClangDiffCategory));
+
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
cl::Required,
cl::cat(ClangDiffCategory));
@@ -110,6 +114,161 @@
 
 static char hexdigit(int N) { return N &= 0xf, N + (N < 10 ? '0' : 'a' - 10); }
 
+static const char HtmlDiffHeader[] = R"(
+
+
+
+
+span.d { color: red; }
+span.u { color: #cc00cc; }
+span.i { color: green; }
+span.m { font-weight: bold; }
+span   { font-weight: normal; color: black; }
+div.code {
+  width: 48%;
+  height: 98%;
+  overflow: scroll;
+  float: left;
+  padding: 0 0 0.5% 0.5%;
+  border: solid 2px LightGrey;
+  border-radius: 5px;
+}
+
+
+
+highlightStack = []
+function clearHighlight() {
+  while (highlightStack.length) {
+let [l, r] = highlightStack.pop()
+document.getElementById(l).style.backgroundColor = 'white'
+document.getElementById(r).style.backgroundColor = 'white'
+  }
+}
+function highlight(event) {
+  id = event.target['id']
+  doHighlight(id)
+}
+function doHighlight(id) {
+  clearHighlight()
+  source = document.getElementById(id)
+  if (!source.attributes['tid'])
+return
+  tid = source.attributes['tid'].value
+  target = document.getElementById(tid)
+  if (!target || source.parentElement && source.parentElement.classList.contains('code'))
+return
+  source.style.backgroundColor = target.style.backgroundColor = 'lightgrey'
+  highlightStack.push([id, tid])
+  source.scrollIntoView()
+  target.scrollIntoView()
+  location.hash = '#' + id
+}
+function scrollToBoth() {
+  doHighlight(location.hash.substr(1))
+}
+window.onload = scrollToBoth
+
+
+
+)";
+
+static void printHtml(raw_ostream &OS, char C) {
+  switch (C) {
+  case '&':
+OS << "&";
+break;
+  case '<':
+OS << "<";
+break;
+  case '>':
+OS << ">";
+break;
+  case '\'':
+OS << "'";
+break;
+  case '"':
+OS << """;
+break;
+  default:
+OS << C;
+  }
+}
+
+static void printHtml(raw_ostream &OS, const StringRef Str) {
+  for (char C : Str)
+printHtml(OS, C);
+}
+
+static std::string getChangeKindAbbr(diff::ChangeKind Kind) {
+  switch (Kind) {
+  case diff::None:
+return "";
+  case diff::Delete:
+return "d";
+  case diff::Update:
+return "u";
+  case diff::Insert:
+return "i";
+  case diff::Move:
+return "m";
+  case diff::UpdateMove:
+return "u m";
+  }
+}
+
+static unsigned printHtmlForNode(raw_ostream &OS, const diff::ASTDiff &Diff,
+ diff::SyntaxTree &Tree, bool IsLeft,
+ diff::NodeId Id, unsigned Offset) {
+  const diff::Node &Node = Tree.getNode(Id);
+  char MyTag, OtherTag;
+  diff::NodeId LeftId, RightId;
+  diff::NodeId TargetId = Diff.getMapped(Tree, Id);
+  if (IsLeft) {
+MyTag = 'L';
+OtherTag = 'R';
+LeftId = Id;
+RightId = TargetId;
+  } else {
+MyTag = 'R';
+OtherTag = 'L';
+LeftId = TargetId;
+RightId = Id;
+  }
+  unsigned Begin, End;
+  std::tie(Begin, End) = Tree.getSourceRangeOffsets(Node);
+  const SourceManager &SrcMgr = Tree.getASTContext().getSourceManager();
+  auto Code = SrcMgr.getBuffer(SrcMgr.getMainFileID())->getBuffer();
+  for (; Offset < Begin; ++Offset)
+printHtml(OS, Code[Offset]);
+  OS << "";
+
+  for (diff::NodeId Child : Node.Children)
+Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Child, Offset);
+
+  for (; Offset < End; ++Offset)
+printHtml(OS, Code[Offset]);
+  if (Id == Tree.getRootId()) {
+End = Code.size();
+for (; Offset < End; ++Offset)
+  printHtml(OS, Code[Offset]);
+  }
+  OS << "";
+  return Offset;
+}
+
 static void printJsonString(raw_ostream &OS, const StringRef Str) {
   for (char C : Str) {
 switch (C) {
@@ -282,6 +441,19 @@
   diff::SyntaxTree DstTree(Dst->getASTContext());
   diff::ASTDiff Diff(SrcTree, DstTree, Options);
 
+  if (HtmlDiff) {
+llvm::outs() << HtmlDiffHeader << "";
+llvm::outs() << "";
+printHtmlForNode(llvm::outs(), Diff, SrcTree, true, SrcTree.getRootId(), 0);
+llvm::outs() << "";
+llvm::outs() << "";
+printHtmlForNode(llvm::outs(), Diff, DstTree, false, DstTree.getRootId(),
+  

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

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

Looks good, but let's confirm with Richard before we change behavior.


https://reviews.llvm.org/D36238



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


[PATCH] D36183: [clang-diff] Simplify mapping

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109422.
johannes edited the summary of this revision.
johannes added a comment.

-


https://reviews.llvm.org/D36183

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/Inputs/clang-diff-basic-src.cpp
  test/Tooling/clang-diff-basic.cpp

Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -38,8 +38,15 @@
   return "foo";
 return 0;
   }
-  // CHECK: Delete AccessSpecDecl: public
   X(){};
-  // CHECK: Delete CXXMethodDecl
 };
 }
+
+namespace {
+// match with parents of different type
+// CHECK: Match FunctionDecl: f1{{.*}} to FunctionDecl: f1
+void f1() {{ (void) __func__;;; }}
+}
+
+// CHECK: Delete AccessSpecDecl: public
+// CHECK: Delete CXXMethodDecl
Index: test/Tooling/Inputs/clang-diff-basic-src.cpp
===
--- test/Tooling/Inputs/clang-diff-basic-src.cpp
+++ test/Tooling/Inputs/clang-diff-basic-src.cpp
@@ -26,3 +26,5 @@
   int id(int i) { return i; }
 };
 }
+
+void f1() {{ (void) __func__;;; }}
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -34,48 +34,23 @@
   Mapping() = default;
   Mapping(Mapping &&Other) = default;
   Mapping &operator=(Mapping &&Other) = default;
-  Mapping(int Size1, int Size2) {
-// Maximum possible size after patching one tree.
-int Size = Size1 + Size2;
-SrcToDst = llvm::make_unique[]>(Size);
-DstToSrc = llvm::make_unique[]>(Size);
+
+  Mapping(size_t Size) {
+SrcToDst = llvm::make_unique(Size);
+DstToSrc = llvm::make_unique(Size);
   }
 
   void link(NodeId Src, NodeId Dst) {
-SrcToDst[Src].push_back(Dst);
-DstToSrc[Dst].push_back(Src);
-  }
-
-  NodeId getDst(NodeId Src) const {
-if (hasSrc(Src))
-  return SrcToDst[Src][0];
-return NodeId();
-  }
-  NodeId getSrc(NodeId Dst) const {
-if (hasDst(Dst))
-  return DstToSrc[Dst][0];
-return NodeId();
-  }
-  const SmallVector &getAllDsts(NodeId Src) const {
-return SrcToDst[Src];
-  }
-  const SmallVector &getAllSrcs(NodeId Dst) const {
-return DstToSrc[Dst];
-  }
-  bool hasSrc(NodeId Src) const { return !SrcToDst[Src].empty(); }
-  bool hasDst(NodeId Dst) const { return !DstToSrc[Dst].empty(); }
-  bool hasSrcDst(NodeId Src, NodeId Dst) const {
-for (NodeId DstId : SrcToDst[Src])
-  if (DstId == Dst)
-return true;
-for (NodeId SrcId : DstToSrc[Dst])
-  if (SrcId == Src)
-return true;
-return false;
+SrcToDst[Src] = Dst, DstToSrc[Dst] = Src;
   }
 
+  NodeId getDst(NodeId Src) const { return SrcToDst[Src]; }
+  NodeId getSrc(NodeId Dst) const { return DstToSrc[Dst]; }
+  bool hasSrc(NodeId Src) const { return getDst(Src).isValid(); }
+  bool hasDst(NodeId Dst) const { return getSrc(Dst).isValid(); }
+
 private:
-  std::unique_ptr[]> SrcToDst, DstToSrc;
+  std::unique_ptr SrcToDst, DstToSrc;
 };
 } // end anonymous namespace
 
@@ -104,8 +79,6 @@
   // Returns true if the two subtrees are identical.
   bool identical(NodeId Id1, NodeId Id2) const;
 
-  bool canBeAddedToMapping(const Mapping &M, NodeId Id1, NodeId Id2) const;
-
   // Returns false if the nodes must not be mached.
   bool isMatchingPossible(NodeId Id1, NodeId Id2) const;
 
@@ -711,23 +684,6 @@
   return true;
 }
 
-bool ASTDiff::Impl::canBeAddedToMapping(const Mapping &M, NodeId Id1,
-NodeId Id2) const {
-  assert(isMatchingPossible(Id1, Id2) &&
- "Matching must be possible in the first place.");
-  if (M.hasSrcDst(Id1, Id2))
-return false;
-  if (Options.EnableMatchingWithUnmatchableParents)
-return true;
-  const Node &N1 = T1.getNode(Id1);
-  const Node &N2 = T2.getNode(Id2);
-  NodeId P1 = N1.Parent;
-  NodeId P2 = N2.Parent;
-  // Only allow matching if parents can be matched.
-  return (P1.isInvalid() && P2.isInvalid()) ||
- (P1.isValid() && P2.isValid() && isMatchingPossible(P1, P2));
-}
-
 bool ASTDiff::Impl::isMatchingPossible(NodeId Id1, NodeId Id2) const {
   return Options.isMatchingAllowed(T1.getNode(Id1), T2.getNode(Id2));
 }
@@ -750,7 +706,7 @@
   for (const auto Tuple : R) {
 NodeId Src = Tuple.first;
 NodeId Dst = Tuple.second;
-if (canBeAddedToMapping(M, Src, Dst))
+if (!M.hasSrc(Src) && !M.hasDst(Dst))
   M.link(Src, Dst);
   }
 }
@@ -803,7 +759,7 @@
 if (Matched || !MatchedChildren)
   continue;
 NodeId Id2 = findCandidate(M, Id1);
-if (Id2.isValid() && canBeAddedToMapping(M, Id1, Id2)) {
+if (Id2.isValid()) {
   M.link(Id1, Id2);
   addOptimalMapping(M, Id1, Id2);
 }
@@ -814,7 +770,7 @@
   PriorityList L1(T1);
   PriorityList L2(T2);
 
-  Mapping M(T1.getSize(), T2.getSize());
+  Mapping M(T1.getSize() + T2.ge

[PATCH] D36184: [clang-diff] Filter AST nodes

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109423.
johannes added a comment.

tests


https://reviews.llvm.org/D36184

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-ast.cpp

Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -12,7 +12,8 @@
   // CHECK: IntegerLiteral: 1
   auto i = 1;
   // CHECK: CallExpr(
-  // CHECK: DeclRefExpr: f(
+  // CHECK-NOT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr: f(
   f();
   // CHECK: BinaryOperator: =(
   i = i;
@@ -37,6 +38,7 @@
 if (i == 0)
   // CHECK: StringLiteral: foo(
   return "foo";
+// CHECK-NOT: ImplicitCastExpr
 return 0;
   }
 
@@ -48,3 +50,22 @@
 int x = m;
   }
 };
+
+#define M (void)1
+#define MA(a, b) (void)a, b
+// CHECK: FunctionDecl
+// CHECK-NEXT: CompoundStmt
+void macros() {
+  M;
+  MA(1, 2);
+}
+// CHECK-NEXT: NamespaceDecl
+
+#ifndef GUARD
+#define GUARD
+namespace world {
+// nodes from other files are excluded
+// CHECK-NOT {{.}}
+#include "clang-diff-ast.cpp"
+}
+#endif
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -162,24 +162,37 @@
   if (!N)
 return true;
   SourceLocation SLoc = N->getLocStart();
-  return SLoc.isValid() && SrcMgr.isInSystemHeader(SLoc);
+  if (!SLoc.isValid())
+return false;
+  // Ignore everything from other files.
+  if (!SrcMgr.isInMainFile(SLoc))
+return true;
+  // Ignore macros.
+  if (N->getLocStart() != SrcMgr.getSpellingLoc(N->getLocStart()))
+return true;
+  return false;
 }
 
+static bool isDeclExcluded(const Decl *D) { return D->isImplicit(); }
+static bool isStmtExcluded(const Stmt *S) { return false; }
+
 namespace {
 /// Counts the number of nodes that will be compared.
 struct NodeCountVisitor : public RecursiveASTVisitor {
   int Count = 0;
   const SyntaxTree::Impl &Tree;
   NodeCountVisitor(const SyntaxTree::Impl &Tree) : Tree(Tree) {}
   bool TraverseDecl(Decl *D) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), D))
+if (isNodeExcluded(Tree.AST.getSourceManager(), D) || isDeclExcluded(D))
   return true;
 ++Count;
 RecursiveASTVisitor::TraverseDecl(D);
 return true;
   }
   bool TraverseStmt(Stmt *S) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), S))
+if (S)
+  S = S->IgnoreImplicit();
+if (isNodeExcluded(Tree.AST.getSourceManager(), S) || isStmtExcluded(S))
   return true;
 ++Count;
 RecursiveASTVisitor::TraverseStmt(S);
@@ -233,15 +246,17 @@
   N.Height = std::max(N.Height, 1 + Tree.getNode(Child).Height);
   }
   bool TraverseDecl(Decl *D) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), D))
+if (isNodeExcluded(Tree.AST.getSourceManager(), D) || isDeclExcluded(D))
   return true;
 auto SavedState = PreTraverse(D);
 RecursiveASTVisitor::TraverseDecl(D);
 PostTraverse(SavedState);
 return true;
   }
   bool TraverseStmt(Stmt *S) {
-if (isNodeExcluded(Tree.AST.getSourceManager(), S))
+if (S)
+  S = S->IgnoreImplicit();
+if (isNodeExcluded(Tree.AST.getSourceManager(), S) || isStmtExcluded(S))
   return true;
 auto SavedState = PreTraverse(S);
 RecursiveASTVisitor::TraverseStmt(S);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36185: [clang-diff] Fix similarity computation

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109424.
johannes added a comment.

add test for Options.MaxSize


https://reviews.llvm.org/D36185

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-bottomup.cpp
  test/Tooling/clang-diff-opt.cpp
  test/Tooling/clang-diff-topdown.cpp

Index: test/Tooling/clang-diff-topdown.cpp
===
--- /dev/null
+++ test/Tooling/clang-diff-topdown.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -E %s > %t.src.cpp
+// RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
+// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- | FileCheck %s
+//
+// Test the top-down matching of identical subtrees only.
+
+#ifndef DEST
+
+void f1()
+{
+  // Match some subtree of height greater than 2.
+  // CHECK: Match CompoundStmt(3) to CompoundStmt(3)
+  // CHECK: Match CompoundStmt(4) to CompoundStmt(4)
+  // CHECK: Match NullStmt(5) to NullStmt(5)
+  {{;}}
+
+  // Don't match subtrees that are smaller.
+  // CHECK-NOT: Match CompoundStmt(6)
+  // CHECK-NOT: Match NullStmt(7)
+  {;}
+
+  // Greedy approach - use the first matching subtree when there are multiple
+  // identical subtrees.
+  // CHECK: Match CompoundStmt(8) to CompoundStmt(8)
+  // CHECK: Match CompoundStmt(9) to CompoundStmt(9)
+  // CHECK: Match NullStmt(10) to NullStmt(10)
+  {{;;}}
+}
+
+#else
+
+void f1() {
+
+  {{;}}
+
+  {;}
+
+  {{;;}}
+  // CHECK-NOT: Match {{.*}} to CompoundStmt(11)
+  // CHECK-NOT: Match {{.*}} to CompoundStmt(12)
+  // CHECK-NOT: Match {{.*}} to NullStmt(13)
+  {{;;}}
+
+  // CHECK-NOT: Match {{.*}} to NullStmt(14)
+  ;
+}
+
+#endif
Index: test/Tooling/clang-diff-opt.cpp
===
--- /dev/null
+++ test/Tooling/clang-diff-opt.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -E %s > %t.src.cpp
+// RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
+// RUN: clang-diff -dump-matches -s=10 %t.src.cpp %t.dst.cpp -- | FileCheck %s
+//
+// Test the behaviour of the matching according to the optimal tree edit
+// distance, implemented with Zhang and Shasha's algorithm.
+// Just for testing we use a tiny value of 10 for maxsize. Subtrees bigger than
+// this size will not be processed by the optimal algorithm.
+
+#ifndef DEST
+
+void f1() { {;} {{;}} }
+
+void f2() { {;} {{;}} }
+
+void f3() { {;} {{;;}} }
+
+#else
+
+void f1() {
+// Jaccard similarity = 3 / (5 + 4 - 3) = 3 / 6 >= 0.5
+// The optimal matching algorithm should move the ; into the outer block
+// CHECK: Match CompoundStmt(2) to CompoundStmt(2)
+// CHECK-NOT: Match CompoundStmt(3)
+// CHECK-NEXT: Match NullStmt(4) to NullStmt(3)
+  ; {{;}}
+}
+ 
+void f2() {
+  // Jaccard similarity = 7 / (10 + 10 - 7) >= 0.5
+  // As none of the subtrees is bigger than 10 nodes, the optimal algorithm
+  // will be run.
+  // CHECK: Match NullStmt(11) to NullStmt(9)
+  ;; {{;}}
+}
+
+void f3() {
+  // Jaccard similarity = 8 / (11 + 11 - 8) >= 0.5
+  // As the subtrees are bigger than 10 nodes, the optimal algorithm will not
+  // be run.
+  // CHECK: Delete NullStmt(22)
+  ;; {{;;}}
+}
+#endif
Index: test/Tooling/clang-diff-bottomup.cpp
===
--- /dev/null
+++ test/Tooling/clang-diff-bottomup.cpp
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -E %s > %t.src.cpp
+// RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
+// RUN: clang-diff -dump-matches -s=0 %t.src.cpp %t.dst.cpp -- | FileCheck %s
+//
+// Test the bottom-up matching, with maxsize set to 0, so that the optimal matching will never be applied.
+
+#ifndef DEST
+
+void f1() { ; {{;}} }
+void f2() { ;; {{;}} }
+
+#else
+
+// Jaccard similarity threshold is 0.5.
+
+void f1() {
+// CompoundStmt: 3 matched descendants, subtree sizes 4 and 5
+// Jaccard similarity = 3 / (4 + 5 - 3) = 3 / 6 >= 0.5
+// CHECK: Match FunctionDecl: f1(void (void))(1) to FunctionDecl: f1(void (void))(1)
+// CHECK: Match CompoundStmt(2) to CompoundStmt(2)
+// CHECK: Match CompoundStmt(4) to CompoundStmt(3)
+// CHECK: Match CompoundStmt(5) to CompoundStmt(4)
+// CHECK: Match NullStmt(6) to NullStmt(5)
+  {{;}} ;;
+}
+
+void f2() {
+// CompoundStmt: 3 matched descendants, subtree sizes 4 and 5
+// Jaccard similarity = 3 / (5 + 6 - 3) = 3 / 8 < 0.5
+// CHECK-NOT: Match FunctionDecl(9)
+// CHECK-NOT: Match CompoundStmt(10)
+// CHECK: Match CompoundStmt(11) to CompoundStmt(10)
+// CHECK: Match CompoundStmt(12) to CompoundStmt(11)
+// CHECK: Match NullStmt(13) to NullStmt(12)
+// CHECK-NOT: Match NullStmt(13)
+  {{;}} ;;;
+}
+ 
+#endif
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -68,7 +68,8 @@
   // Compute ChangeKind for each node based on similarity.
   void computeChangeKinds(Mapping &M);
 
-  NodeId getMapped(const std::unique_ptr &Tree, NodeId Id) const {
+  NodeId getMapped(const st

[PATCH] D36186: [clang-diff] Improve and test getNodeValue

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109425.
johannes added a comment.

NFC renames


https://reviews.llvm.org/D36186

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-bottomup.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -315,6 +315,18 @@
   const diff::Node &N = Tree.getNode(Id);
   OS << "{";
   printNodeAttributes(OS, Tree, Id);
+  auto Identifier = N.getIdentifier();
+  auto QualifiedIdentifier = N.getQualifiedIdentifier();
+  if (Identifier) {
+OS << R"(,"identifier":")";
+printJsonString(OS, *Identifier);
+OS << R"(")";
+if (QualifiedIdentifier && *Identifier != *QualifiedIdentifier) {
+  OS << R"(,"qualified_identifier":")";
+  printJsonString(OS, *QualifiedIdentifier);
+  OS << R"(")";
+}
+  }
   OS << R"(,"children":[)";
   if (N.Children.size() > 0) {
 printNodeAsJson(OS, Tree, N.Children[0]);
Index: test/Tooling/clang-diff-bottomup.cpp
===
--- test/Tooling/clang-diff-bottomup.cpp
+++ test/Tooling/clang-diff-bottomup.cpp
@@ -16,7 +16,7 @@
 void f1() {
 // CompoundStmt: 3 matched descendants, subtree sizes 4 and 5
 // Jaccard similarity = 3 / (4 + 5 - 3) = 3 / 6 >= 0.5
-// CHECK: Match FunctionDecl: f1(void (void))(1) to FunctionDecl: f1(void (void))(1)
+// CHECK: Match FunctionDecl: f1(void ())(1) to FunctionDecl: f1(void ())(1)
 // CHECK: Match CompoundStmt(2) to CompoundStmt(2)
 // CHECK: Match CompoundStmt(4) to CompoundStmt(3)
 // CHECK: Match CompoundStmt(5) to CompoundStmt(4)
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -11,18 +11,18 @@
 }
 }
 
-// CHECK: Match DeclRefExpr: foo{{.*}} to DeclRefExpr: inner::foo
+// CHECK: Match DeclRefExpr: src::foo{{.*}} to DeclRefExpr: dst::inner::foo
 void main() { inner::foo(); }
 
 // CHECK: Match StringLiteral: foo{{.*}} to StringLiteral: foo
 const char *b = "f" "o" "o";
 
 // unsigned is canonicalized to unsigned int
-// CHECK: Match TypedefDecl: nat;unsigned int;{{.*}} to TypedefDecl: nat;unsigned int;
+// CHECK: Match TypedefDecl: src::nat;unsigned int;{{.*}} to TypedefDecl: dst::nat;unsigned int;
 typedef unsigned nat;
 
-// CHECK: Match VarDecl: p(int){{.*}} to VarDecl: prod(double)
-// CHECK: Update VarDecl: p(int){{.*}} to prod(double)
+// CHECK: Match VarDecl: src::p(int){{.*}} to VarDecl: dst::prod(double)
+// CHECK: Update VarDecl: src::p(int){{.*}} to dst::prod(double)
 // CHECK: Match BinaryOperator: *{{.*}} to BinaryOperator: *
 double prod = 1 * 2 * 10;
 // CHECK: Update DeclRefExpr
@@ -44,7 +44,7 @@
 
 namespace {
 // match with parents of different type
-// CHECK: Match FunctionDecl: f1{{.*}} to FunctionDecl: f1
+// CHECK: Match FunctionDecl: f1{{.*}} to FunctionDecl: (anonymous namespace)::f1
 void f1() {{ (void) __func__;;; }}
 }
 
Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -5,34 +5,43 @@
 // CHECK: {{^}} NamespaceDecl: test;(
 namespace test {
 
-// CHECK: {{^}}  FunctionDecl: f(
+// CHECK: {{^}} FunctionDecl: test::f(
 // CHECK: CompoundStmt(
 void f() {
   // CHECK: VarDecl: i(int)(
   // CHECK: IntegerLiteral: 1
   auto i = 1;
+  // CHECK: FloatingLiteral: 1.5(
+  auto r = 1.5;
+  // CHECK: CXXBoolLiteralExpr: true(
+  auto b = true;
   // CHECK: CallExpr(
   // CHECK-NOT: ImplicitCastExpr
-  // CHECK-NEXT: DeclRefExpr: f(
+  // CHECK: DeclRefExpr: test::f(
   f();
+  // CHECK: UnaryOperator: ++(
+  ++i;
   // CHECK: BinaryOperator: =(
   i = i;
 }
 
 } // end namespace test
 
+// CHECK: UsingDirectiveDecl: test(
+using namespace test;
+
 // CHECK: TypedefDecl: nat;unsigned int;(
 typedef unsigned nat;
 // CHECK: TypeAliasDecl: real;double;(
 using real = double;
 
 class Base {
 };
 
-// CHECK: CXXRecordDecl: X;class X;(
+// CHECK: CXXRecordDecl: X;X;(
 class X : Base {
   int m;
-  // CHECK: CXXMethodDecl: foo(const char *(int))(
+  // CHECK: CXXMethodDecl: X::foo(const char *(int))(
   // CHECK: ParmVarDecl: i(int)(
   const char *foo(int i) {
 if (i == 0)
@@ -44,9 +53,9 @@
 
   // CHECK: AccessSpecDecl: public(
 public:
-  // CHECK: CXXConstructorDecl: X(void (char, int))(
+  // CHECK: CXXConstructorDecl: X::X(void (char, int))Base,X::m,(
   X(char, int) : Base(), m(0) {
-// CHECK: MemberExpr(
+// CHECK: MemberExpr: X::m(
 int x = m;
   }
 };
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -149,6 +149,8 @

[PATCH] D36186: [clang-diff] Improve and test getNodeValue

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 109426.
johannes added a comment.

renamse, NFC


https://reviews.llvm.org/D36186

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-bottomup.cpp
  test/Tooling/clang-diff-opt.cpp
  test/Tooling/clang-diff-topdown.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -315,6 +315,18 @@
   const diff::Node &N = Tree.getNode(Id);
   OS << "{";
   printNodeAttributes(OS, Tree, Id);
+  auto Identifier = N.getIdentifier();
+  auto QualifiedIdentifier = N.getQualifiedIdentifier();
+  if (Identifier) {
+OS << R"(,"identifier":")";
+printJsonString(OS, *Identifier);
+OS << R"(")";
+if (QualifiedIdentifier && *Identifier != *QualifiedIdentifier) {
+  OS << R"(,"qualified_identifier":")";
+  printJsonString(OS, *QualifiedIdentifier);
+  OS << R"(")";
+}
+  }
   OS << R"(,"children":[)";
   if (N.Children.size() > 0) {
 printNodeAsJson(OS, Tree, N.Children[0]);
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 
@@ -27,8 +27,19 @@
   {{;;}}
 }
 
+int x;
+
+namespace src {
+  int x;
+  int x1 = x + 1;
+  int x2 = ::x + 1;
+}
+
+class A { int x = 1 + 1; void f() { int x1 = x; } };
+
 #else
 
+
 void f1() {
 
   {{;}}
@@ -45,4 +56,28 @@
   ;
 }
 
+int x;
+
+namespace dst {
+  int x;
+  // CHECK: Match DeclRefExpr: :x(17) to DeclRefExpr: :x(22)
+  int x1 = x + 1;
+  // CHECK: Match DeclRefExpr: x(21) to DeclRefExpr: x(26)
+  int x2 = ::x + 1;
+}
+
+class B {
+  // Only the class name changed; it is not included in the field value,
+  // therefore there is no update.
+  // CHECK: Match FieldDecl: :x(int)(24) to FieldDecl: :x(int)(29)
+  // CHECK-NOT: Update FieldDecl: :x(int)(24)
+  int x = 1+1;
+  void f() {
+// CHECK: Match MemberExpr: :x(32) to MemberExpr: :x(37)
+// CHECK-NOT: Update MemberExpr: :x(32)
+int x1 = B::x;
+  }
+
+};
+
 #endif
Index: test/Tooling/clang-diff-opt.cpp
===
--- test/Tooling/clang-diff-opt.cpp
+++ test/Tooling/clang-diff-opt.cpp
@@ -25,7 +25,7 @@
 // CHECK-NEXT: Match NullStmt(4) to NullStmt(3)
   ; {{;}}
 }
- 
+
 void f2() {
   // Jaccard similarity = 7 / (10 + 10 - 7) >= 0.5
   // As none of the subtrees is bigger than 10 nodes, the optimal algorithm
@@ -41,4 +41,5 @@
   // CHECK: Delete NullStmt(22)
   ;; {{;;}}
 }
+ 
 #endif
Index: test/Tooling/clang-diff-bottomup.cpp
===
--- test/Tooling/clang-diff-bottomup.cpp
+++ test/Tooling/clang-diff-bottomup.cpp
@@ -16,7 +16,7 @@
 void f1() {
 // CompoundStmt: 3 matched descendants, subtree sizes 4 and 5
 // Jaccard similarity = 3 / (4 + 5 - 3) = 3 / 6 >= 0.5
-// CHECK: Match FunctionDecl: f1(void (void))(1) to FunctionDecl: f1(void (void))(1)
+// CHECK: Match FunctionDecl: f1(void ())(1) to FunctionDecl: f1(void ())(1)
 // CHECK: Match CompoundStmt(2) to CompoundStmt(2)
 // CHECK: Match CompoundStmt(4) to CompoundStmt(3)
 // CHECK: Match CompoundStmt(5) to CompoundStmt(4)
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -11,18 +11,18 @@
 }
 }
 
-// CHECK: Match DeclRefExpr: foo{{.*}} to DeclRefExpr: inner::foo
+// CHECK: Match DeclRefExpr: :foo{{.*}} to DeclRefExpr: :inner::foo
 void main() { inner::foo(); }
 
 // CHECK: Match StringLiteral: foo{{.*}} to StringLiteral: foo
 const char *b = "f" "o" "o";
 
 // unsigned is canonicalized to unsigned int
-// CHECK: Match TypedefDecl: nat;unsigned int;{{.*}} to TypedefDecl: nat;unsigned int;
+// CHECK: Match TypedefDecl: :nat;unsigned int;{{.*}} to TypedefDecl: :nat;unsigned int;
 typedef unsigned nat;
 
-// CHECK: Match VarDecl: p(int){{.*}} to VarDecl: prod(double)
-// CHECK: Update VarDecl: p(int){{.*}} to prod(double)
+// CHECK: Match VarDecl: :p(int){{.*}} to VarDecl: :prod(double)
+// CHECK: Update VarDecl: :p(int){{.*}} to :prod(double)
 // CHECK: Match BinaryOperator: *{{.*}} to BinaryOperator: *
 double prod = 1 * 2 * 10;
 // CHECK: Update DeclRefExpr
@@ -44,7 +44,7 @@
 
 namespace {
 // match with parents of different type
-// CHECK: Match FunctionDecl: f1{{.*}} to 

[PATCH] D36176: [clang-diff] Fix some errors and inconsistencies

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:96
   : TreeImpl(llvm::make_unique(this, Node, AST)) {}
+  SyntaxTree(const SyntaxTree &Tree) = delete;
   ~SyntaxTree();

arphaman wrote:
> johannes wrote:
> > arphaman wrote:
> > > It might be better to add a move constructor which would disable copy 
> > > constructors.
> > ASTDiff::getMapped internally uses the address of the SyntaxTree that is 
> > passed as parameter.
> > So the tree must be exactly the same as the one that is passed to the 
> > constructor of ASTDiff. 
> > 
> > This is quite bad. I will change ASTDiff::getMapped to accept a boolean or 
> > split it into two methods
> > Should I still add a move constructor?
> Can't you just use the `TreeImpl` pointer value? That should be the same even 
> after you've moved `SyntaxTree` with the move constructor.
ok perfect


https://reviews.llvm.org/D36176



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


[PATCH] D36177: [clang-diff] Add commandline arguments.

2017-08-02 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment.

In https://reviews.llvm.org/D36177#828909, @arphaman wrote:

> There should be a test that ensures that `stop-after` works and `extra-arg`s 
> are correctly passed to the compiler.


`stop-after` is not tested yet. It is also broken in this patch, I will move it 
to a later patch, then it is easier to test.


https://reviews.llvm.org/D36177



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


  1   2   >