[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 166070.
erik.pilkington added a comment.

Sure, the new patch moves the test to double-quotes.m and gives it a more 
meaningful name.

Thanks!


https://reviews.llvm.org/D52253

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
  clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
  clang/test/Modules/double-quotes.m


Index: clang/test/Modules/double-quotes.m
===
--- clang/test/Modules/double-quotes.m
+++ clang/test/Modules/double-quotes.m
@@ -32,6 +32,9 @@
 #import "A.h"
 #import 
 
+// rdar://43692300
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
 int bar() { return foo(); }
 
 // 
expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted 
include "A0.h" in framework header, expected angle-bracketed instead}}
Index: 
clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
@@ -0,0 +1 @@
+// Empty file!
Index: 
clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
@@ -0,0 +1 @@
+#include "Thing2.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void


Index: clang/test/Modules/double-quotes.m
===
--- clang/test/Modules/double-quotes.m
+++ clang/test/Modules/double-quotes.m
@@ -32,6 +32,9 @@
 #import "A.h"
 #import 
 
+// rdar://43692300
+#import "NotAFramework/Headers/Headers/Thing1.h"
+
 int bar() { return foo(); }
 
 // expected-warning@Inputs/double-quotes/A.framework/Headers/A.h:1{{double-quoted include "A0.h" in framework header, expected angle-bracketed instead}}
Index: clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing2.h
@@ -0,0 +1 @@
+// Empty file!
Index: clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/double-quotes/NotAFramework/Headers/Headers/Thing1.h
@@ -0,0 +1 @@
+#include "Thing2.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52259: [CUDA] Rearrange search path ordering to fix two test case failures

[Jiading Gai via Phabricator via cfe-commits]

gaijiading created this revision.
gaijiading added reviewers: jlebar, Hahnfeld, dlj, tra.
gaijiading added a project: clang.
Herald added a subscriber: cfe-commits.

Before this patch, when a system has the CUDA toolkit already installed to its 
default locations in /usr/local/cuda, there will be two unexpected test case 
failures:

1. /test/Driver/cuda-detect.cu
2. /test/Driver/cuda-macosx.cu

The reason being that the overall search order was set up to always pick up the 
path with well-known tools such as ptxas in it; However, the above two test 
cases require the CUDA path to be the one that is passed to --sysroot.

Moving the sysroot path on top fixes the issue.


Repository:
  rC Clang

https://reviews.llvm.org/D52259

Files:
  unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -29,6 +29,18 @@
 using ExprMatcher = internal::Matcher;
 using StmtMatcher = internal::Matcher;
 
+std::unique_ptr
+buildASTFromCodeWithArgs(const Twine ,
+ const std::vector ) {
+  auto AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+  EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  return AST;
+}
+
+std::unique_ptr buildASTFromCode(const Twine ) {
+  return buildASTFromCodeWithArgs(Code, {});
+}
+
 ExprMatcher declRefTo(StringRef Name) {
   return declRefExpr(to(namedDecl(hasName(Name;
 }
@@ -83,12 +95,12 @@
 "template T&& "
 "forward(typename remove_reference::type& t) noexcept { return t; }"
 "template T&& "
-"forward(typename remove_reference::type&&) noexcept { return t; } }";
+"forward(typename remove_reference::type&& t) noexcept { return t; } }";
 
 } // namespace
 
 TEST(ExprMutationAnalyzerTest, Trivial) {
-  const auto AST = tooling::buildASTFromCode("void f() { int x; x; }");
+  const auto AST = buildASTFromCode("void f() { int x; x; }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
@@ -98,8 +110,7 @@
 
 TEST_P(AssignmentTest, AssignmentModifies) {
   const std::string ModExpr = "x " + GetParam() + " 10";
-  const auto AST =
-  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
@@ -113,8 +124,7 @@
 
 TEST_P(IncDecTest, IncDecModifies) {
   const std::string ModExpr = GetParam();
-  const auto AST =
-  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
@@ -124,476 +134,447 @@
 Values("++x", "--x", "x++", "x--"), );
 
 TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
-  const auto AST = tooling::buildASTFromCode(
+  const auto AST = buildASTFromCode(
   "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
 TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
-  auto AST = tooling::buildASTFromCodeWithArgs(
+  auto AST = buildASTFromCodeWithArgs(
   "struct X { template  void mf(); };"
   "template  void f() { X x; x.mf(); }",
   {"-fno-delayed-template-parsing"});
   auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 
-  AST = tooling::buildASTFromCodeWithArgs(
-  "template  void f() { T x; x.mf(); }",
-  {"-fno-delayed-template-parsing"});
+  AST = buildASTFromCodeWithArgs("template  void f() { T x; x.mf(); }",
+ {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 
-  AST = tooling::buildASTFromCodeWithArgs(
+  AST = buildASTFromCodeWithArgs(
   "template  struct X;"
   "template  void f() { X x; x.mf(); }",
   {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 
 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
-  const auto AST = tooling::buildASTFromCode(
+  const auto AST = buildASTFromCode(
   "void f() { struct Foo { void mf() const; }; 

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang updated this revision to Diff 166068.
shuaiwang marked 7 inline comments as done.
shuaiwang added a comment.

[WIP] Addressed some of review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D52219

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

Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -52,16 +52,34 @@
 bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   const auto *const E = selectFirst("expr", Results);
+  assert(S && E);
   return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
+bool isPointeeMutated(const SmallVectorImpl ,
+  ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  const auto *const E = selectFirst("expr", Results);
+  assert(S && E);
+  return ExprMutationAnalyzer(*S, AST->getASTContext()).isPointeeMutated(E);
+}
+
 SmallVector
 mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
+  const Stmt *(ExprMutationAnalyzer::*MutationFinder)(const Expr *) =
+  ::findMutation;
+  const Stmt *(ExprMutationAnalyzer::*PointeeMutationFinder)(const Expr *) =
+  ::findPointeeMutation;
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
-const Stmt *By = Analyzer.findMutation(E);
+auto Finder = MutationFinder;
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;
+}
+const Stmt *By = (Analyzer.*Finder)(E);
 std::string buffer;
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
@@ -100,10 +118,14 @@
 } // namespace
 
 TEST(ExprMutationAnalyzerTest, Trivial) {
-  const auto AST = buildASTFromCode("void f() { int x; x; }");
-  const auto Results =
+  auto AST = buildASTFromCode("void f() { int x; x; }");
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = buildASTFromCode("void f() { const int x = 0; x; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 class AssignmentTest : public ::testing::TestWithParam {};
@@ -134,41 +156,104 @@
 Values("++x", "--x", "x++", "x--"), );
 
 TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
-  const auto AST = buildASTFromCode(
+  auto AST = buildASTFromCode(
   "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
-  const auto Results =
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_TRUE(isMutated(Results, AST.get()));
+  EXPECT_FALSE(isPointeeMutated(Results, AST.get()));
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = buildASTFromCode(
+  "void f() { struct Foo { void mf(); }; Foo *p; p->mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 }
 
 TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
   auto AST = buildASTFromCodeWithArgs(
   "struct X { template  void mf(); };"
-  "template  void f() { X x; x.mf(); }",
+  "template  void f() { X x; x.mf(); }"
+  "template  void g() { X *p; p->mf(); }",
   {"-fno-delayed-template-parsing"});
   auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 
-  AST = buildASTFromCodeWithArgs("template  void f() { T x; x.mf(); }",
- {"-fno-delayed-template-parsing"});
+  AST =
+  buildASTFromCodeWithArgs("template  void f() { T x; x.mf(); }"
+   "template  void g() { T *p; p->mf(); }",
+   {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 
   AST = buildASTFromCodeWithArgs(
   "template  

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added a comment.

In https://reviews.llvm.org/D52219#1238423, @JonasToth wrote:

> Do you think it would be possible to the analysis for `>const?< int 
> ***`-cases? (recursively checking through the pointer levels)


I think that should be possible, will do after single-layer pointee analysis is 
done. I believe we can build multi-layer analysis based on much of the 
single-layer analysis.




Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:198
 const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // `Exp` can be *directly* mutated if the type of `Exp` is not const.
+  // Bail out early otherwise.

JonasToth wrote:
> Just to be sure that i understand:
> the changes here are more performance optimizations then directly related to 
> detect pointee mutations?
Yes :)



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:481
+  const auto AsArg =
+  anyOf(callExpr(hasAnyArgument(equalsNode(Exp))),
+cxxConstructExpr(hasAnyArgument(equalsNode(Exp))),

JonasToth wrote:
> shouldn't be the constness of the argument considered here?
We need that for non-pointee version, but not for pointee version, for example:
```
void g1(int * const);

void f1() {
  int *x;
  g1(x); // <-- x is passed to `g1`, we consider that as a mutation, the 
argument type do have a top-level const
}

void g2(const int *);

void f2() {
  int *x;
  g2(x); // <-- declRefExp(to(x)) is NOT directly passed to `g2`, there's a 
layer a ImplicitCastExpr in between, and after the implicit cast, the 
type of the expression becomes "const int *" instead of just "int*", so it'll 
fail the `isPointeeMutable` check at the beginning of 
`findPointeeDirectMutation`
}
```

In summary, we rely on:
- Checking whether pointee is actually mutable at the beginning
- Carefully handling casts by not trivially ignoring them unless absolutely safe


Repository:
  rC Clang

https://reviews.llvm.org/D52219



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

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang updated this revision to Diff 166065.
shuaiwang added a comment.

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D52219

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

Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -52,16 +52,34 @@
 bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
   const auto *const S = selectFirst("stmt", Results);
   const auto *const E = selectFirst("expr", Results);
+  assert(E);
   return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
+bool isPointeeMutated(const SmallVectorImpl ,
+  ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  const auto *const E = selectFirst("expr", Results);
+  assert(E);
+  return ExprMutationAnalyzer(*S, AST->getASTContext()).isPointeeMutated(E);
+}
+
 SmallVector
 mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
+  const Stmt *(ExprMutationAnalyzer::*MutationFinder)(const Expr *) =
+  ::findMutation;
+  const Stmt *(ExprMutationAnalyzer::*PointeeMutationFinder)(const Expr *) =
+  ::findPointeeMutation;
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
-const Stmt *By = Analyzer.findMutation(E);
+auto Finder = MutationFinder;
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;
+}
+const Stmt *By = (Analyzer.*Finder)(E);
 std::string buffer;
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
@@ -100,10 +118,14 @@
 } // namespace
 
 TEST(ExprMutationAnalyzerTest, Trivial) {
-  const auto AST = buildASTFromCode("void f() { int x; x; }");
-  const auto Results =
+  auto AST = buildASTFromCode("void f() { int x; x; }");
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = buildASTFromCode("void f() { const int x = 0; x; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 class AssignmentTest : public ::testing::TestWithParam {};
@@ -134,41 +156,104 @@
 Values("++x", "--x", "x++", "x--"), );
 
 TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
-  const auto AST = buildASTFromCode(
+  auto AST = buildASTFromCode(
   "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
-  const auto Results =
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_TRUE(isMutated(Results, AST.get()));
+  EXPECT_FALSE(isPointeeMutated(Results, AST.get()));
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = buildASTFromCode(
+  "void f() { struct Foo { void mf(); }; Foo *p; p->mf(); }");
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_TRUE(isPointeeMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 }
 
 TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
   auto AST = buildASTFromCodeWithArgs(
   "struct X { template  void mf(); };"
-  "template  void f() { X x; x.mf(); }",
+  "template  void f() { X x; x.mf(); }"
+  "template  void g() { X *p; p->mf(); }",
   {"-fno-delayed-template-parsing"});
   auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 
-  AST = buildASTFromCodeWithArgs("template  void f() { T x; x.mf(); }",
- {"-fno-delayed-template-parsing"});
+  AST =
+  buildASTFromCodeWithArgs("template  void f() { T x; x.mf(); }"
+   "template  void g() { T *p; p->mf(); }",
+   {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 
   AST = buildASTFromCodeWithArgs(
   "template  struct X;"
-  "template  void f() { X x; x.mf(); }",
+  "template  void f() { X 

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added inline comments.



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

compnerd wrote:
> I think that the condition here is inverted, and should be enabled for 
> MinGW32.
What error prompted this? It hasn't caused problems when running libc++'s tests 
against MSVC's STL (with both C1XX and Clang).


Repository:
  rCXX libc++

https://reviews.llvm.org/D51868



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

r342525 - [NFC] Fix uncompilable test cases of ExprMutationAnalyzer.

[Shuai Wang via cfe-commits]

Author: shuaiwang
Date: Tue Sep 18 20:50:03 2018
New Revision: 342525

URL: http://llvm.org/viewvc/llvm-project?rev=342525=rev
Log:
[NFC] Fix uncompilable test cases of ExprMutationAnalyzer.

And ensure future test cases doesn't have compile errors.

Modified:
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Modified: cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp?rev=342525=342524=342525=diff
==
--- cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp (original)
+++ cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp Tue Sep 18 
20:50:03 2018
@@ -29,6 +29,18 @@ namespace {
 using ExprMatcher = internal::Matcher;
 using StmtMatcher = internal::Matcher;
 
+std::unique_ptr
+buildASTFromCodeWithArgs(const Twine ,
+ const std::vector ) {
+  auto AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+  EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+  return AST;
+}
+
+std::unique_ptr buildASTFromCode(const Twine ) {
+  return buildASTFromCodeWithArgs(Code, {});
+}
+
 ExprMatcher declRefTo(StringRef Name) {
   return declRefExpr(to(namedDecl(hasName(Name;
 }
@@ -83,12 +95,12 @@ const std::string StdForward =
 "template T&& "
 "forward(typename remove_reference::type& t) noexcept { return t; }"
 "template T&& "
-"forward(typename remove_reference::type&&) noexcept { return t; } }";
+"forward(typename remove_reference::type&& t) noexcept { return t; } }";
 
 } // namespace
 
 TEST(ExprMutationAnalyzerTest, Trivial) {
-  const auto AST = tooling::buildASTFromCode("void f() { int x; x; }");
+  const auto AST = buildASTFromCode("void f() { int x; x; }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
@@ -98,8 +110,7 @@ class AssignmentTest : public ::testing:
 
 TEST_P(AssignmentTest, AssignmentModifies) {
   const std::string ModExpr = "x " + GetParam() + " 10";
-  const auto AST =
-  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
@@ -113,8 +124,7 @@ class IncDecTest : public ::testing::Tes
 
 TEST_P(IncDecTest, IncDecModifies) {
   const std::string ModExpr = GetParam();
-  const auto AST =
-  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
@@ -124,7 +134,7 @@ INSTANTIATE_TEST_CASE_P(AllIncDecOperato
 Values("++x", "--x", "x++", "x--"), );
 
 TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
-  const auto AST = tooling::buildASTFromCode(
+  const auto AST = buildASTFromCode(
   "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -132,7 +142,7 @@ TEST(ExprMutationAnalyzerTest, NonConstM
 }
 
 TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
-  auto AST = tooling::buildASTFromCodeWithArgs(
+  auto AST = buildASTFromCodeWithArgs(
   "struct X { template  void mf(); };"
   "template  void f() { X x; x.mf(); }",
   {"-fno-delayed-template-parsing"});
@@ -140,13 +150,12 @@ TEST(ExprMutationAnalyzerTest, AssumedNo
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 
-  AST = tooling::buildASTFromCodeWithArgs(
-  "template  void f() { T x; x.mf(); }",
-  {"-fno-delayed-template-parsing"});
+  AST = buildASTFromCodeWithArgs("template  void f() { T x; x.mf(); 
}",
+ {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 
-  AST = tooling::buildASTFromCodeWithArgs(
+  AST = buildASTFromCodeWithArgs(
   "template  struct X;"
   "template  void f() { X x; x.mf(); }",
   {"-fno-delayed-template-parsing"});
@@ -155,7 +164,7 @@ TEST(ExprMutationAnalyzerTest, AssumedNo
 }
 
 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
-  const auto AST = tooling::buildASTFromCode(
+  const auto AST = buildASTFromCode(
   "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -163,7 +172,7 @@ 

[PATCH] D52074: [PowerPC] [Clang] Add vector int128 pack/unpack builtins

[Zixuan Wu via Phabricator via cfe-commits]

wuzish added a comment.
Herald added a subscriber: jsji.

Please help me to commit this patch. Thanks a lot.


Repository:
  rC Clang

https://reviews.llvm.org/D52074



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

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: test/support/verbose_assert.h:24
 : (IsStreamable::value ? 2 : -1))>
-struct SelectStream {
+struct SelectErrStream {
   static_assert(ST == -1, "specialization required for ST != -1");

Why the renaming here? 

What's the deal with sometimes using `clog` and sometimes `cerr`?
(I know, you didn't do this, but ???)



Repository:
  rCXX libc++

https://reviews.llvm.org/D51868



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

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D52200#1239007, @aaronpuchert wrote:

> I think it should be possible to get rid of `self->` in the warning message 
> if we want to, after all `this->` is omitted in C++ as well.


Hmm.  It would be consistent to apply the same rule to both cases, and I don't 
have any serious concerns about dropping it.  If anything, Objective-C pushes 
the ivar-decorating convention harder than C++ does, since it's officially 
blessed in the language.


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Erik,

Thanks for catching this.

Can you rename the test and the input file path to something more meaningful? 
Better yet if you remove the radar bits. I think you can actually squeeze this 
test in test/Modules/double-quotes.m.


Repository:
  rC Clang

https://reviews.llvm.org/D52253



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

[PATCH] D51120: clang-format Additional Indent for class blocks

[Darby Payne via Phabricator via cfe-commits]

dpayne updated this revision to Diff 166057.
dpayne added a comment.

Adding the missing Style member variable.


Repository:
  rC Clang

https://reviews.llvm.org/D51120

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/CMakeLists.txt
  unittests/Format/FormatTestClassIndent.cpp

Index: unittests/Format/FormatTestClassIndent.cpp
===
--- /dev/null
+++ unittests/Format/FormatTestClassIndent.cpp
@@ -0,0 +1,144 @@
+//===- unittest/Format/FormatTestAccess.cpp - Formatting unit tests
+//-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "../Tooling/RewriterTestContext.h"
+#include "FormatTestUtils.h"
+
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class FormatTestClassIndent : public ::testing::Test {
+protected:
+  std::string format(llvm::StringRef Code, const FormatStyle ) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(0, Code.size()));
+bool IncompleteFormat = false;
+tooling::Replacements Replaces =
+reformat(Style, Code, Ranges, "", );
+EXPECT_FALSE(IncompleteFormat) << Code << "\n\n";
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  FormatStyle getStyleWithAdditionalIndent() {
+FormatStyle Style = getLLVMStyle();
+Style.AdditionalIndentClassBlock = true;
+return Style;
+  }
+
+  void verifyFormat(llvm::StringRef Code, const FormatStyle & Style = getLLVMStyle()) {
+EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestClassIndent, NoChangeWithoutAccess) {
+  verifyFormat("class A {\n"
+   "  int a1;\n"
+   "};");
+  verifyFormat("class B {\n"
+   "int b1;\n"
+   "};",
+   getStyleWithAdditionalIndent());
+}
+
+TEST_F(FormatTestClassIndent, ChangeWithAccess) {
+  verifyFormat("class C {\n"
+   "  int c1;\n"
+   "public:\n"
+   "  int c2;\n"
+   "};");
+  verifyFormat("class D {\n"
+   "int d1;\n"
+   "  public:\n"
+   "int d2;\n"
+   "};",
+   getStyleWithAdditionalIndent());
+}
+
+TEST_F(FormatTestClassIndent, MultipleAccessLevels) {
+  verifyFormat("class E {\n"
+   "  int e1;\n"
+   "public:\n"
+   "  int e2;\n"
+   "private:\n"
+   "  int e3;\n"
+   "protected:\n"
+   "  int e4;\n"
+   "};");
+  verifyFormat("class F {\n"
+   "int f1;\n"
+   "  public:\n"
+   "int f2;\n"
+   "  private:\n"
+   "int f3;\n"
+   "  protected:\n"
+   "int f4;\n"
+   "};",
+   getStyleWithAdditionalIndent());
+}
+
+TEST_F(FormatTestClassIndent, NestedClass) {
+  verifyFormat("class G {\n"
+   "  int g1;\n"
+   "  class GGA {\n"
+   "int gg1;\n"
+   "  public:\n"
+   "int gg2;\n"
+   "  };\n"
+   "public:\n"
+   "  class GGB {\n"
+   "int gg1;\n"
+   "  public:\n"
+   "int gg2;\n"
+   "  };\n"
+   "  int g2;\n"
+   "private:\n"
+   "  int g3;\n"
+   "protected:\n"
+   "  int g4;\n"
+   "};");
+  verifyFormat("class H {\n"
+   "int h1;\n"
+   "class HHA {\n"
+   "int hh1;\n"
+   "  public:\n"
+   "int hh2;\n"
+   "};\n"
+   "  public:\n"
+   "class HHB {\n"
+   "int hh1;\n"
+   "  public:\n"
+   "int hh2;\n"
+   "};\n"
+   "int h2;\n"
+   "  private:\n"
+   "int h3;\n"
+   "  protected:\n"
+   "int h4;\n"
+   "};",
+   getStyleWithAdditionalIndent());
+}
+
+} // end namespace
+} // end namespace format
+} // end namespace clang
Index: 

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:670
 if (IsIncludeeInFramework) {
   NewInclude += StringRef(ToFramework).drop_back(10); // drop .framework
   NewInclude += "/";

Crash was here, if ToFramework is empty because no "Foo.framework" entry found 
in the path.


Repository:
  rC Clang

https://reviews.llvm.org/D52253



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

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added a reviewer: bruno.
Herald added a subscriber: dexonsmith.

rdar://43692300


Repository:
  rC Clang

https://reviews.llvm.org/D52253

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File.h
  clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File0.h
  clang/test/Frontend/rdar43692300.m


Index: clang/test/Frontend/rdar43692300.m
===
--- /dev/null
+++ clang/test/Frontend/rdar43692300.m
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -I %S/Inputs %s
+
+// expected-no-diagnostics
+
+#include "Radar43692300/Headers/Headers/File.h"
+
+int main()
+{
+}
Index: clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File0.h
===
--- /dev/null
+++ clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File0.h
@@ -0,0 +1 @@
+// This file is empty!
Index: clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File.h
===
--- /dev/null
+++ clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File.h
@@ -0,0 +1 @@
+#include "File0.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void


Index: clang/test/Frontend/rdar43692300.m
===
--- /dev/null
+++ clang/test/Frontend/rdar43692300.m
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -I %S/Inputs %s
+
+// expected-no-diagnostics
+
+#include "Radar43692300/Headers/Headers/File.h"
+
+int main()
+{
+}
Index: clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File0.h
===
--- /dev/null
+++ clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File0.h
@@ -0,0 +1 @@
+// This file is empty!
Index: clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File.h
===
--- /dev/null
+++ clang/test/Frontend/Inputs/Radar43692300/Headers/Headers/File.h
@@ -0,0 +1 @@
+#include "File0.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -648,7 +648,7 @@
 ++I;
   }
 
-  return FoundComp >= 2;
+  return !FrameworkName.empty() && FoundComp >= 2;
 }
 
 static void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342519 - Thread safety analysis: Fix crash for function pointers

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Tue Sep 18 17:19:38 2018
New Revision: 342519

URL: http://llvm.org/viewvc/llvm-project?rev=342519=rev
Log:
Thread safety analysis: Fix crash for function pointers

For function pointers, the FunctionDecl of the callee is unknown, so
getDirectCallee will return nullptr. We have to catch that case to avoid
crashing. We assume there is no attribute then.

Modified:
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=342519=342518=342519=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Tue Sep 18 17:19:38 2018
@@ -354,15 +354,17 @@ til::SExpr *SExprBuilder::translateCallE
 const Expr *SelfE) {
   if (CapabilityExprMode) {
 // Handle LOCK_RETURNED
-const FunctionDecl *FD = CE->getDirectCallee()->getMostRecentDecl();
-if (LockReturnedAttr* At = FD->getAttr()) {
-  CallingContext LRCallCtx(Ctx);
-  LRCallCtx.AttrDecl = CE->getDirectCallee();
-  LRCallCtx.SelfArg  = SelfE;
-  LRCallCtx.NumArgs  = CE->getNumArgs();
-  LRCallCtx.FunArgs  = CE->getArgs();
-  return const_cast(
-  translateAttrExpr(At->getArg(), ).sexpr());
+if (const FunctionDecl *FD = CE->getDirectCallee()) {
+  FD = FD->getMostRecentDecl();
+  if (LockReturnedAttr *At = FD->getAttr()) {
+CallingContext LRCallCtx(Ctx);
+LRCallCtx.AttrDecl = CE->getDirectCallee();
+LRCallCtx.SelfArg = SelfE;
+LRCallCtx.NumArgs = CE->getNumArgs();
+LRCallCtx.FunArgs = CE->getArgs();
+return const_cast(
+translateAttrExpr(At->getArg(), ).sexpr());
+  }
 }
   }
 

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=342519=342518=342519=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Sep 18 17:19:38 
2018
@@ -2323,6 +2323,7 @@ Foo& getBarFoo(Bar , int c) { return
 void test() {
   Foo foo;
   Foo *fooArray;
+  Foo &(*fooFuncPtr)();
   Bar bar;
   int a;
   int b;
@@ -2359,6 +2360,10 @@ void test() {
   (a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock();
   (a > 0 ? fooArray[1] : fooArray[b]).a = 0;
   (a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock();
+
+  fooFuncPtr().mu_.Lock();
+  fooFuncPtr().a = 0;
+  fooFuncPtr().mu_.Unlock();
 }
 
 


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

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

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 166053.
tra added a comment.

Renamed last instance of 'Matches' -> 'PreventedBy'.


https://reviews.llvm.org/D51808

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCUDA/usual-deallocators.cu
  clang/test/SemaCUDA/call-host-fn-from-device.cu
  clang/test/SemaCUDA/usual-deallocators.cu

Index: clang/test/SemaCUDA/usual-deallocators.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/usual-deallocators.cu
@@ -0,0 +1,95 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o /dev/null -verify=device
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o /dev/null -verify=host
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o /dev/null -verify=device
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o /dev/null -verify=host
+
+#include "Inputs/cuda.h"
+extern __host__ void host_fn();
+extern __device__ void dev_fn();
+extern __host__ __device__ void hd_fn();
+
+struct H1D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct h1D1 {
+  __host__ void operator delete(void *) = delete;
+  // host-note@-1 {{'operator delete' has been explicitly marked deleted here}}
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1d1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) = delete;
+  // device-note@-1 {{'operator delete' has been explicitly marked deleted here}}
+};
+
+struct H1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H2D1 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H2D2 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1H2D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+
+template 
+__host__ __device__ void test_hd(void *p) {
+  T *t = (T *)p;
+  delete t;
+  // host-error@-1 {{attempt to use a deleted function}}
+  // device-error@-2 {{attempt to use a deleted function}}
+}
+
+__host__ __device__ void tests_hd(void *t) {
+  test_hd(t);
+  test_hd(t);
+  // host-note@-1 {{in instantiation of function template specialization 'test_hd' requested here}}
+  test_hd(t);
+  // device-note@-1 {{in instantiation of function template specialization 'test_hd' requested here}}
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+}
Index: clang/test/SemaCUDA/call-host-fn-from-device.cu
===
--- clang/test/SemaCUDA/call-host-fn-from-device.cu
+++ clang/test/SemaCUDA/call-host-fn-from-device.cu
@@ -41,12 +41,12 @@
   operator Dummy() { return Dummy(); }
   // expected-note@-1 {{'operator Dummy' declared here}}
 
-  __host__ void operator delete(void*);
-  __device__ void operator delete(void*, size_t);
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void*, __SIZE_TYPE__);
 };
 
 struct U {
-  __device__ void operator delete(void*, size_t) = delete;
+  __device__ void operator delete(void*, __SIZE_TYPE__) = delete;
   __host__ __device__ void operator delete(void*);
 };
 
Index: clang/test/CodeGenCUDA/usual-deallocators.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/usual-deallocators.cu
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o - | FileCheck %s 

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This is pretty cool. The process launching APIs in LLVM were pretty basic, left 
a lot to be desired, returned ints, etc etc. This addresses a lot of that.




Comment at: clang/trunk/lib/Driver/Driver.cpp:3030
+if (Arg *A = Args.getLastArg(options::OPT__SLASH_MP)) {
+C.CoresToUse = llvm::hardware_concurrency();
+StringRef(A->getValue()).getAsInteger(10, C.CoresToUse);

Seems nice to save a syscall and not ask how many cores we have if we were 
given an explicit value first.



Comment at: llvm/trunk/lib/Support/Windows/Program.inc:424
 
-ProcessInfo sys::Wait(const ProcessInfo , unsigned SecondsToWait,
-  bool WaitUntilChildTerminates, std::string *ErrMsg) {
-  assert(PI.Pid && "invalid pid to wait on, process not started?");
-  assert((PI.Process && PI.Process != INVALID_HANDLE_VALUE) &&
- "invalid process handle to wait on, process not started?");
+bool sys::WaitMany(MutableArrayRef PIArray, bool WaitOnAll,
+   unsigned SecondsToWait, bool WaitUntilProcessTerminates) {

I guess no Posix implementation? It's kind of hard to know if we made the right 
abstractions without doing it for Windows and *nix.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.

I think it should be possible to get rid of `self->` in the warning message if 
we want to, after all `this->` is omitted in C++ as well.




Comment at: test/SemaObjCXX/warn-thread-safety-analysis.mm:42
+  value_ -= 1;
+} // expected-warning{{mutex 'self->lock_' is still held at the end of 
function}}
+

@rjmccall Would you rather write `self->lock_` or `lock_` in the warning 
message?


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 166051.
aaronpuchert added a comment.

Detect ObjC pointer types as well as ordinary pointers.


Repository:
  rC Clang

https://reviews.llvm.org/D52200

Files:
  include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  lib/Analysis/ThreadSafetyCommon.cpp
  test/SemaObjCXX/warn-thread-safety-analysis.mm

Index: test/SemaObjCXX/warn-thread-safety-analysis.mm
===
--- /dev/null
+++ test/SemaObjCXX/warn-thread-safety-analysis.mm
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -Wno-objc-root-class %s
+
+class __attribute__((lockable)) Lock {
+public:
+  void Acquire() __attribute__((exclusive_lock_function())) {}
+  void Release() __attribute__((unlock_function())) {}
+};
+
+class __attribute__((scoped_lockable)) AutoLock {
+public:
+  AutoLock(Lock ) __attribute__((exclusive_lock_function(lock)))
+  : lock_(lock) {
+lock.Acquire();
+  }
+  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
+
+private:
+  Lock _;
+};
+
+@interface MyInterface {
+@private
+  Lock lock_;
+  int value_;
+}
+
+- (void)incrementValue;
+- (void)decrementValue;
+
+@end
+
+@implementation MyInterface
+
+- (void)incrementValue {
+  AutoLock lock(lock_);
+  value_ += 1;
+}
+
+- (void)decrementValue {
+  lock_.Acquire(); // expected-note{{mutex acquired here}}
+  value_ -= 1;
+} // expected-warning{{mutex 'self->lock_' is still held at the end of function}}
+
+@end
Index: lib/Analysis/ThreadSafetyCommon.cpp
===
--- lib/Analysis/ThreadSafetyCommon.cpp
+++ lib/Analysis/ThreadSafetyCommon.cpp
@@ -211,6 +211,8 @@
 return translateCXXThisExpr(cast(S), Ctx);
   case Stmt::MemberExprClass:
 return translateMemberExpr(cast(S), Ctx);
+  case Stmt::ObjCIvarRefExprClass:
+return translateObjCIVarRefExpr(cast(S), Ctx);
   case Stmt::CallExprClass:
 return translateCallExpr(cast(S), Ctx);
   case Stmt::CXXMemberCallExprClass:
@@ -311,9 +313,9 @@
   return nullptr;
 }
 
-static bool hasCppPointerType(const til::SExpr *E) {
+static bool hasAnyPointerType(const til::SExpr *E) {
   auto *VD = getValueDeclFromSExpr(E);
-  if (VD && VD->getType()->isPointerType())
+  if (VD && VD->getType()->isAnyPointerType())
 return true;
   if (const auto *C = dyn_cast(E))
 return C->castOpcode() == til::CAST_objToPtr;
@@ -344,7 +346,20 @@
 D = getFirstVirtualDecl(VD);
 
   til::Project *P = new (Arena) til::Project(E, D);
-  if (hasCppPointerType(BE))
+  if (hasAnyPointerType(BE))
+P->setArrow(true);
+  return P;
+}
+
+til::SExpr *SExprBuilder::translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+   CallingContext *Ctx) {
+  til::SExpr *BE = translate(IVRE->getBase(), Ctx);
+  til::SExpr *E = new (Arena) til::SApply(BE);
+
+  const auto *D = cast(IVRE->getDecl()->getCanonicalDecl());
+
+  til::Project *P = new (Arena) til::Project(E, D);
+  if (hasAnyPointerType(BE))
 P->setArrow(true);
   return P;
 }
Index: include/clang/Analysis/Analyses/ThreadSafetyCommon.h
===
--- include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -397,6 +397,8 @@
CallingContext *Ctx) ;
   til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext *Ctx);
   til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx);
+  til::SExpr *translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+   CallingContext *Ctx);
   til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx,
 const Expr *SelfE = nullptr);
   til::SExpr *translateCXXMemberCallExpr(const CXXMemberCallExpr *ME,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 166050.
tra marked an inline comment as done.
tra added a comment.

Updated assertion message.


https://reviews.llvm.org/D51808

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCUDA/usual-deallocators.cu
  clang/test/SemaCUDA/call-host-fn-from-device.cu
  clang/test/SemaCUDA/usual-deallocators.cu

Index: clang/test/SemaCUDA/usual-deallocators.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/usual-deallocators.cu
@@ -0,0 +1,95 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o /dev/null -verify=device
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o /dev/null -verify=host
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o /dev/null -verify=device
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown \
+// RUN:   -emit-llvm -o /dev/null -verify=host
+
+#include "Inputs/cuda.h"
+extern __host__ void host_fn();
+extern __device__ void dev_fn();
+extern __host__ __device__ void hd_fn();
+
+struct H1D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct h1D1 {
+  __host__ void operator delete(void *) = delete;
+  // host-note@-1 {{'operator delete' has been explicitly marked deleted here}}
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1d1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) = delete;
+  // device-note@-1 {{'operator delete' has been explicitly marked deleted here}}
+};
+
+struct H1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H2D1 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H2D2 {
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1H2D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1D2 {
+  __host__ void operator delete(void *) { host_fn(); };
+  __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+  __device__ void operator delete(void *) { dev_fn(); };
+  __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+
+template 
+__host__ __device__ void test_hd(void *p) {
+  T *t = (T *)p;
+  delete t;
+  // host-error@-1 {{attempt to use a deleted function}}
+  // device-error@-2 {{attempt to use a deleted function}}
+}
+
+__host__ __device__ void tests_hd(void *t) {
+  test_hd(t);
+  test_hd(t);
+  // host-note@-1 {{in instantiation of function template specialization 'test_hd' requested here}}
+  test_hd(t);
+  // device-note@-1 {{in instantiation of function template specialization 'test_hd' requested here}}
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+  test_hd(t);
+}
Index: clang/test/SemaCUDA/call-host-fn-from-device.cu
===
--- clang/test/SemaCUDA/call-host-fn-from-device.cu
+++ clang/test/SemaCUDA/call-host-fn-from-device.cu
@@ -41,12 +41,12 @@
   operator Dummy() { return Dummy(); }
   // expected-note@-1 {{'operator Dummy' declared here}}
 
-  __host__ void operator delete(void*);
-  __device__ void operator delete(void*, size_t);
+  __host__ void operator delete(void *) { host_fn(); };
+  __device__ void operator delete(void*, __SIZE_TYPE__);
 };
 
 struct U {
-  __device__ void operator delete(void*, size_t) = delete;
+  __device__ void operator delete(void*, __SIZE_TYPE__) = delete;
   __host__ __device__ void operator delete(void*);
 };
 
Index: clang/test/CodeGenCUDA/usual-deallocators.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/usual-deallocators.cu
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN:   -emit-llvm -o - | FileCheck %s 

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D52193#1238978, @aganea wrote:

> In https://reviews.llvm.org/D52193#1238944, @zturner wrote:
>
> > In https://reviews.llvm.org/D52193#1238923, @aganea wrote:
> >
> > > The goal of this change is frictionless compilation into VS2017 when 
> > > using `clang-cl` as a compiler. We've realized that compiling Clang+LLD 
> > > with Clang generates a faster executable that with MSVC (even latest one).
> > >  I currently can't see a good way of generating the Visual Studio 
> > > solution with CMake, while using Ninja+clang-cl for compilation. They are 
> > > two orthogonal configurations. Any suggestions?
> >
> >
> > I don't think this is necessary.  I think your updated Matrix is pretty 
> > good.
> >
> > I'm surprised to see that Ninja + Clang is slower than Ninja + MSVC.  Are 
> > you using lld here?
>
>
> Yes, all the ‘Clang’ rows use both clang-cl and lld-link.
>  Like stated above, I think this is caused by a lot more processes 
> (clang-cl.exe) being invoked. In contrast, cl.exe does not invoke a child 
> process. There are about 3200 files to compiles, which makes 6400 calls to 
> clang-cl. At about ~60ms lead time per process, that adds up to an extra 
> ~3min wall clock time. It is the simplest explanation I could find.


Would you mind syncing to r342002 and trying again?  I don't doubt your 
results, but I'm interested to see how much of an improvement this patch 
provides.

  commit 840717872a0e0f03b19040ef143bf45aa1a7f0a0
  Author: Reid Kleckner 
  Date:   Tue Sep 11 22:25:13 2018 +
  
  [cmake] Speed up check-llvm 5x by delay loading shell32 and ole32
  
  Summary:
  Previously, check-llvm on my Windows 10 workstation took about 300s to
  run, and it would lock up my mouse. Now, it takes just under 60s.
  Previously running the tests only used about 15% of the available CPU
  time, now it uses up to 60%.
  
  Shell32.dll and ole32.dll have direct dependencies on user32.dll and
  gdi32.dll. These DLLs are mostly used to for Windows GUI functionality,
  which most LLVM tools don't need. It seems that these DLLs acquire and
  release some system resources on startup and exit, and that requires
  acquiring the same highly contended lock discussed in this post:
  
https://randomascii.wordpress.com/2017/07/09/24-core-cpu-and-i-cant-move-my-mouse/
  
  Most LLVM tools still have a transitive dependency on
  SHGetKnownFolderPathW, which is used to look up the user home directory
  and local AppData directory. We also use SHFileOperationW to recursively
  delete a directory, but only LLDB and clang-doc use this today. At some
  point, we might consider removing these last shell32.dll deps, but for
  now, I would like to take this free performance.
  
  Many thanks to Bruce Dawson for suggesting this fix. He looked at an ETW
  trace of an LLVM test run, noticed these DLLs, and suggested avoiding
  them.
  
  Reviewers: zturner, pcc, thakis
  
  Subscribers: mgorny, llvm-commits, hiraditya
  
  Differential Revision: https://reviews.llvm.org/D51952
  
  Notes:
  git-svn-rev: 342002


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

I found something that would theoretically work:

  P->setArrow((isa(ME->getBase()) && Ctx && Ctx->SelfArg) ? 
Ctx->SelfArrow : ME->isArrow());

So if we have `this` and a context that tells us we have to replace `this` by 
something else, then we check `Ctx->SelfArrow`, otherwise we take 
`ME->isArrow()`.

But that doesn't work. When translating into the TIL (typed intermediate 
language), referencing and dereferencing operators are completely ignored.

  struct Foo {
Mutex mu_;
void foo1(Foo *f_declared) EXCLUSIVE_LOCKS_REQUIRED(f_declared->mu_);
  };
  
  void test() {
Foo myfoo;
myfoo.foo1();  // \
  // expected-warning {{calling function 'foo1' requires holding mutex 
'myfoo.mu_' exclusively}}
  }

With the above change we warn that `calling function 'foo1' requires holding 
mutex 'myfoo->mu_' exclusively`. It should be `()->mu_`, but the `&` is 
lost. So we can't derive the information that we want from `isArrow` alone.

Now there is a reason why these operators are ignored — the TIL tries to 
"canonicalize" expressions, so that it detects that `()->mu_` and 
`myfoo.mu_` are the same thing. Changing that is probably possible, but beyond 
the scope of this change.

Short of that, we must be able to detect pointers. I think we could use 
`Type::isAnyPointerType` instead of `Type::isPointerType` in 
`hasCppPointerType` (and probably rename that).

For later I think we should consider a different canonicalization that doesn't 
just ignore `&` and `*`.


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In https://reviews.llvm.org/D52193#1238944, @zturner wrote:

> In https://reviews.llvm.org/D52193#1238923, @aganea wrote:
>
> > The goal of this change is frictionless compilation into VS2017 when using 
> > `clang-cl` as a compiler. We've realized that compiling Clang+LLD with 
> > Clang generates a faster executable that with MSVC (even latest one).
> >  I currently can't see a good way of generating the Visual Studio solution 
> > with CMake, while using Ninja+clang-cl for compilation. They are two 
> > orthogonal configurations. Any suggestions?
>
>
> I don't think this is necessary.  I think your updated Matrix is pretty good.
>
> I'm surprised to see that Ninja + Clang is slower than Ninja + MSVC.  Are you 
> using lld here?


Yes, all the ‘Clang’ rows use both clang-cl and lld-link.
Like stated above, I think this is caused by a lot more processes 
(clang-cl.exe) being invoked. In contrast, cl.exe does not invoke a child 
process. There are about 3200 files to compiles, which makes 6400 calls to 
clang-cl. At about ~60ms lead time per process, that adds up to an extra ~3min 
wall clock time. It is the simplest explanation I could find.


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

r342517 - Add a callback for `__has_include` and use it for dependency scanning.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Tue Sep 18 16:27:02 2018
New Revision: 342517

URL: http://llvm.org/viewvc/llvm-project?rev=342517=rev
Log:
Add a callback for `__has_include` and use it for dependency scanning.

This adds a preprocessor callback for the `__has_include` and
`__has_include_next` directives.

Successful checking for the presence of a header should add it to the list of
header dependencies so this overrides the callback in the dependency scanner.

Patch by Pete Cooper with some additions by me.

rdar://problem/39545636

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

Added:
cfe/trunk/test/Frontend/dependency-gen-has-include.c
Modified:
cfe/trunk/include/clang/Lex/PPCallbacks.h
cfe/trunk/lib/Frontend/DependencyFile.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp

Modified: cfe/trunk/include/clang/Lex/PPCallbacks.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PPCallbacks.h?rev=342517=342516=342517=diff
==
--- cfe/trunk/include/clang/Lex/PPCallbacks.h (original)
+++ cfe/trunk/include/clang/Lex/PPCallbacks.h Tue Sep 18 16:27:02 2018
@@ -276,6 +276,12 @@ public:
SourceRange Range) {
   }
 
+  /// Hook called when a '__has_include' or '__has_include_next' directive is
+  /// read.
+  virtual void HasInclude(SourceLocation Loc, StringRef FileName, bool 
IsAngled,
+  const FileEntry *File,
+  SrcMgr::CharacteristicKind FileType) {}
+
   /// Hook called when a source range is skipped.
   /// \param Range The SourceRange that was skipped. The range begins at the
   /// \#if/\#else directive and ends after the \#endif/\#else directive.
@@ -443,6 +449,13 @@ public:
 Second->PragmaDiagnostic(Loc, Namespace, mapping, Str);
   }
 
+  void HasInclude(SourceLocation Loc, StringRef FileName, bool IsAngled,
+  const FileEntry *File,
+  SrcMgr::CharacteristicKind FileType) override {
+First->HasInclude(Loc, FileName, IsAngled, File, FileType);
+Second->HasInclude(Loc, FileName, IsAngled, File, FileType);
+  }
+
   void PragmaOpenCLExtension(SourceLocation NameLoc, const IdentifierInfo 
*Name,
  SourceLocation StateLoc, unsigned State) override 
{
 First->PragmaOpenCLExtension(NameLoc, Name, StateLoc, State);

Modified: cfe/trunk/lib/Frontend/DependencyFile.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DependencyFile.cpp?rev=342517=342516=342517=diff
==
--- cfe/trunk/lib/Frontend/DependencyFile.cpp (original)
+++ cfe/trunk/lib/Frontend/DependencyFile.cpp Tue Sep 18 16:27:02 2018
@@ -200,6 +200,10 @@ public:
   const Module *Imported,
   SrcMgr::CharacteristicKind FileType) override;
 
+  void HasInclude(SourceLocation Loc, StringRef SpelledFilename, bool IsAngled,
+  const FileEntry *File,
+  SrcMgr::CharacteristicKind FileType) override;
+
   void EndOfMainFile() override {
 OutputDependencyFile();
   }
@@ -328,6 +332,17 @@ void DFGImpl::InclusionDirective(SourceL
   }
 }
 
+void DFGImpl::HasInclude(SourceLocation Loc, StringRef SpelledFilename,
+ bool IsAngled, const FileEntry *File,
+ SrcMgr::CharacteristicKind FileType) {
+  if (!File)
+return;
+  StringRef Filename = File->getName();
+  if (!FileMatchesDepCriteria(Filename.data(), FileType))
+return;
+  AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
+}
+
 bool DFGImpl::AddFilename(StringRef Filename) {
   if (FilesSet.insert(Filename).second) {
 Files.push_back(Filename);

Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=342517=342516=342517=diff
==
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Tue Sep 18 16:27:02 2018
@@ -23,6 +23,7 @@
 #include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Lex/DirectoryLookup.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
@@ -1242,6 +1243,13 @@ static bool EvaluateHasIncludeCommon(Tok
   PP.LookupFile(FilenameLoc, Filename, isAngled, LookupFrom, 
LookupFromFile,
 CurDir, nullptr, nullptr, nullptr, nullptr);
 
+  if (PPCallbacks *Callbacks = PP.getPPCallbacks()) {
+SrcMgr::CharacteristicKind FileType = SrcMgr::C_User;
+if (File)
+  FileType = PP.getHeaderSearchInfo().getFileDirFlavor(File);
+Callbacks->HasInclude(FilenameLoc, Filename, isAngled, File, FileType);
+  }
+
   // Get the result value.  A result of true 

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342517: Add a callback for `__has_include` and use it for 
dependency scanning. (authored by vsapsai, committed by ).
Herald added subscribers: llvm-commits, jsji.

Changed prior to commit:
  https://reviews.llvm.org/D30882?vs=165610=166047#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30882

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

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

r342516 - [MS] Defer dllexport inline friend functions like other inline methods

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Sep 18 16:16:30 2018
New Revision: 342516

URL: http://llvm.org/viewvc/llvm-project?rev=342516=rev
Log:
[MS] Defer dllexport inline friend functions like other inline methods

This special case was added in r264841, but the code breaks our
invariants by calling EmitTopLevelDecl without first creating a
HandlingTopLevelDeclRAII scope.

This fixes the PCH crash in https://crbug.com/884427. I was never able
to make a satisfactory reduction, unfortunately. I'm not very worried
about this regressing since this change makes the code simpler while
passing the existing test that shows we do emit dllexported friend
function definitions. Now we just defer their emission until the tag is
fully complete, which is generally good.

Modified:
cfe/trunk/lib/CodeGen/ModuleBuilder.cpp

Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=342516=342515=342516=diff
==
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Tue Sep 18 16:16:30 2018
@@ -64,7 +64,7 @@ namespace {
 std::unique_ptr Builder;
 
   private:
-SmallVector DeferredInlineMethodDefinitions;
+SmallVector DeferredInlineMemberFuncDefs;
 
   public:
 CodeGeneratorImpl(DiagnosticsEngine , llvm::StringRef ModuleName,
@@ -80,7 +80,7 @@ namespace {
 
 ~CodeGeneratorImpl() override {
   // There should normally not be any leftover inline method definitions.
-  assert(DeferredInlineMethodDefinitions.empty() ||
+  assert(DeferredInlineMemberFuncDefs.empty() ||
  Diags.hasErrorOccurred());
 }
 
@@ -163,16 +163,16 @@ namespace {
 }
 
 void EmitDeferredDecls() {
-  if (DeferredInlineMethodDefinitions.empty())
+  if (DeferredInlineMemberFuncDefs.empty())
 return;
 
   // Emit any deferred inline method definitions. Note that more deferred
   // methods may be added during this loop, since ASTConsumer callbacks
   // can be invoked if AST inspection results in declarations being added.
   HandlingTopLevelDeclRAII HandlingDecl(*this);
-  for (unsigned I = 0; I != DeferredInlineMethodDefinitions.size(); ++I)
-Builder->EmitTopLevelDecl(DeferredInlineMethodDefinitions[I]);
-  DeferredInlineMethodDefinitions.clear();
+  for (unsigned I = 0; I != DeferredInlineMemberFuncDefs.size(); ++I)
+Builder->EmitTopLevelDecl(DeferredInlineMemberFuncDefs[I]);
+  DeferredInlineMemberFuncDefs.clear();
 }
 
 void HandleInlineFunctionDefinition(FunctionDecl *D) override {
@@ -181,17 +181,6 @@ namespace {
 
   assert(D->doesThisDeclarationHaveABody());
 
-  // Handle friend functions.
-  if (D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)) {
-if (Ctx->getTargetInfo().getCXXABI().isMicrosoft()
-&& !D->getLexicalDeclContext()->isDependentContext())
-  Builder->EmitTopLevelDecl(D);
-return;
-  }
-
-  // Otherwise, must be a method.
-  auto MD = cast(D);
-
   // We may want to emit this definition. However, that decision might be
   // based on computing the linkage, and we have to defer that in case we
   // are inside of something that will change the method's final linkage,
@@ -200,13 +189,13 @@ namespace {
   // void bar();
   // void foo() { bar(); }
   //   } A;
-  DeferredInlineMethodDefinitions.push_back(MD);
+  DeferredInlineMemberFuncDefs.push_back(D);
 
   // Provide some coverage mapping even for methods that aren't emitted.
   // Don't do this for templated classes though, as they may not be
   // instantiable.
-  if (!MD->getParent()->isDependentContext())
-Builder->AddDeferredUnusedCoverageMapping(MD);
+  if (!D->getLexicalDeclContext()->isDependentContext())
+Builder->AddDeferredUnusedCoverageMapping(D);
 }
 
 /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl


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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Zachary Turner via Phabricator via cfe-commits]

zturner added a reviewer: thakis.
zturner added a comment.

That said, the numbers are pretty convincing

These two rows alone:
MSBuild, Clang + LLD(29min 12sec)   32 parallel msbuild
MSBuild, Clang /MP + LLD(9min 22sec)32 parallel msbuild

Are enough to make this patch worthy of serious consideration.  (I haven't 
looked at the content / complexity yet, was waiting on the numbers first).


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D52193#1238923, @aganea wrote:

> The goal of this change is frictionless compilation into VS2017 when using 
> `clang-cl` as a compiler. We've realized that compiling Clang+LLD with Clang 
> generates a faster executable that with MSVC (even latest one).
>  I currently can't see a good way of generating the Visual Studio solution 
> with CMake, while using Ninja+clang-cl for compilation. They are two 
> orthogonal configurations. Any suggestions?


I don't think this is necessary.  I think your updated Matrix is pretty good.

I'm surprised to see that Ninja + Clang is slower than Ninja + MSVC.  Are you 
using lld here?


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D52252: Driver: render arguments for the embedded bitcode correctly

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd created this revision.
compnerd added a reviewer: steven_wu.

When embedding bitcode, only a subset of the arguments should be recorded into
the bitcode compilation command line.  The frontend job is split into two jobs,
one which will generate the bitcode.  Ensure that the arguments for the
compilation to bitcode is properly stripped so that the embedded arguments are
the permitted subset.


Repository:
  rC Clang

https://reviews.llvm.org/D52252

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Clang.cpp

Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1297,21 +1297,28 @@
   }
 }

-void Clang::AddARMTargetArgs(const llvm::Triple , const ArgList ,
- ArgStringList , bool KernelOrKext) const {
+namespace {
+void RenderARMABI(const llvm::Triple , const ArgList ,
+  ArgStringList ) {
   // Select the ABI to use.
   // FIXME: Support -meabi.
   // FIXME: Parts of this are duplicated in the backend, unify this somehow.
   const char *ABIName = nullptr;
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ)) {
 ABIName = A->getValue();
-  else {
+  } else {
 std::string CPU = getCPUName(Args, Triple, /*FromAs*/ false);
 ABIName = llvm::ARM::computeDefaultTargetABI(Triple, CPU).data();
   }

   CmdArgs.push_back("-target-abi");
   CmdArgs.push_back(ABIName);
+}
+}
+
+void Clang::AddARMTargetArgs(const llvm::Triple , const ArgList ,
+ ArgStringList , bool KernelOrKext) const {
+  RenderARMABI(Triple, Args, CmdArgs);

   // Determine floating point ABI from the options & target defaults.
   arm::FloatABI ABI = arm::getARMFloatABI(getToolChain(), Args);
@@ -1423,6 +1430,22 @@
   }
 }

+namespace {
+void RenderAArch64ABI(const llvm::Triple , const ArgList ,
+  ArgStringList ) {
+  const char *ABIName = nullptr;
+  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+ABIName = A->getValue();
+  else if (Triple.isOSDarwin())
+ABIName = "darwinpcs";
+  else
+ABIName = "aapcs";
+
+  CmdArgs.push_back("-target-abi");
+  CmdArgs.push_back(ABIName);
+}
+}
+
 void Clang::AddAArch64TargetArgs(const ArgList ,
  ArgStringList ) const {
   const llvm::Triple  = getToolChain().getEffectiveTriple();
@@ -1436,16 +1459,7 @@
 options::OPT_mno_implicit_float, true))
 CmdArgs.push_back("-no-implicit-float");

-  const char *ABIName = nullptr;
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
-ABIName = A->getValue();
-  else if (Triple.isOSDarwin())
-ABIName = "darwinpcs";
-  else
-ABIName = "aapcs";
-
-  CmdArgs.push_back("-target-abi");
-  CmdArgs.push_back(ABIName);
+  RenderAArch64ABI(Triple, Args, CmdArgs);

   if (Arg *A = Args.getLastArg(options::OPT_mfix_cortex_a53_835769,
options::OPT_mno_fix_cortex_a53_835769)) {
@@ -3378,13 +3392,116 @@
 Args.AddLastArg(CmdArgs, options::OPT_save_temps_EQ);

   // Embed-bitcode option.
+  // Only white-listed flags below are allowed to be embedded.
   if (C.getDriver().embedBitcodeInObject() && !C.getDriver().isUsingLTO() &&
   (isa(JA) || isa(JA))) {
 // Add flags implied by -fembed-bitcode.
 Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
 // Disable all llvm IR level optimizations.
 CmdArgs.push_back("-disable-llvm-passes");
+
+// reject options that shouldn't be supported in bitcode
+// also reject kernel/kext
+static const constexpr unsigned kBitcodeOptionBlacklist[] = {
+options::OPT_mkernel,
+options::OPT_fapple_kext,
+options::OPT_ffunction_sections,
+options::OPT_fno_function_sections,
+options::OPT_fdata_sections,
+options::OPT_fno_data_sections,
+options::OPT_funique_section_names,
+options::OPT_fno_unique_section_names,
+options::OPT_mrestrict_it,
+options::OPT_mno_restrict_it,
+options::OPT_mstackrealign,
+options::OPT_mno_stackrealign,
+options::OPT_mstack_alignment,
+options::OPT_mcmodel_EQ,
+options::OPT_mlong_calls,
+options::OPT_mno_long_calls,
+options::OPT_ggnu_pubnames,
+options::OPT_gdwarf_aranges,
+options::OPT_fdebug_types_section,
+options::OPT_fno_debug_types_section,
+options::OPT_fdwarf_directory_asm,
+options::OPT_fno_dwarf_directory_asm,
+options::OPT_mrelax_all,
+options::OPT_mno_relax_all,
+options::OPT_ftrap_function_EQ,
+options::OPT_ffixed_r9,
+options::OPT_mfix_cortex_a53_835769,
+options::OPT_mno_fix_cortex_a53_835769,
+options::OPT_ffixed_x18,
+options::OPT_mglobal_merge,
+options::OPT_mno_global_merge,
+

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

The goal of this change is frictionless compilation into VS2017 when using 
`clang-cl` as a compiler. We've realized that compiling Clang+LLD with Clang 
generates a faster executable that with MSVC (even latest one).
I currently can't see a good way of generating the Visual Studio solution with 
CMake, while using Ninja+clang-cl for compilation. They are two orthogonal 
configurations. Any suggestions?


Repository:
  rC Clang

https://reviews.llvm.org/D52193



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

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande added inline comments.



Comment at: lib/Serialization/ASTReaderDecl.cpp:1771
 auto *Def = DD.Definition;
 DD = std::move(MergeDD);
 DD.Definition = Def;

Hi @rsmith, thanks again for looking!  This is the part that I was concerned 
about: that `DD` could be either a `DefinitionData` or `LambdaDefinitionData` 
reference; similar `MergeDD`.  It didn't seem that move-assign was well-defined 
in the cases where they were mixed.

If their being different is completely unexpected, though, this whole thing 
could perhaps be an assertion that they are both lambda, or both not lambda.  
And an `IsLambda` field on the CXXRecordDecl` (or `TTK_Lambda`).

Alternatively, I could make the lambda data a pointer or 
`std::unique_ptr` inside `DefinitionData` (and maybe make 
the latter `final`), so as not to allocate much extra space in the non-lambda 
case.

Depends on whether a new tag type is not too much of a breaking change.  Also, 
whether it's known at this point that this record is a lambda (and `DD` is 
already of the correct type).


Repository:
  rC Clang

https://reviews.llvm.org/D50948



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

[PATCH] D51868: [libcxx] Build and test fixes for Windows

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.
Herald added a subscriber: libcxx-commits.



Comment at: include/filesystem:1396
 
-  _LIBCPP_FUNC_VIS
   void __create_what(int __num_paths);

This possibly changes the meaning on other targets.  What was the error that 
this triggered?



Comment at: test/support/test_macros.h:147
-#  elif defined(_WIN32)
-#if defined(_MSC_VER) && !defined(__MINGW32__)
-#  define TEST_HAS_C11_FEATURES // Using Microsoft's C Runtime library

I think that the condition here is inverted, and should be enabled for MinGW32.


Repository:
  rCXX libc++

https://reviews.llvm.org/D51868



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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 166044.
JonasToth added a comment.

- minor adjustments, add fixmes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,354 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+  //
+  TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // TODO Fix this
+  // CHECK FIXES: TemplatedType *TT4(nullptr);
+  // CHECK FIXES: {{^  }}TemplatedType TT5;
+  // CHECK FIXES: {{^  }}TemplatedType **TT6 = nullptr;
+  // CHECK FIXES: {{^  }}TemplatedType *const *const TT7{nullptr};
+
+  TemplatedType **TT8(nullptr), TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // TODO Fix this
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so there will be no transformation
+  MY_NICE_TYPE p3, v1, v2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: 

r342515 - Driver: extract a local variable for the Toolchain (NFC)

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Tue Sep 18 15:14:50 2018
New Revision: 342515

URL: http://llvm.org/viewvc/llvm-project?rev=342515=rev
Log:
Driver: extract a local variable for the Toolchain (NFC)

Create and store a reference to the current toolchain rather than calling
`getToolChain` throughout the function.  NFC.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342515=342514=342515=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 15:14:50 2018
@@ -3138,13 +3138,14 @@ static void RenderDebugOptions(const Too
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const 
{
-  const llvm::Triple  = getToolChain().getTriple();
-  const llvm::Triple  = getToolChain().getEffectiveTriple();
+  const auto  = getToolChain();
+  const llvm::Triple  = TC.getTriple();
+  const llvm::Triple  = TC.getEffectiveTriple();
   const std::string  = Triple.getTriple();
 
   bool KernelOrKext =
   Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
-  const Driver  = getToolChain().getDriver();
+  const Driver  = TC.getDriver();
   ArgStringList CmdArgs;
 
   // Check number of inputs for sanity. We need at least one input.
@@ -3198,9 +3199,7 @@ void Clang::ConstructJob(Compilation ,
 }
   }
 
-  const llvm::Triple *AuxTriple =
-  IsCuda ? getToolChain().getAuxTriple() : nullptr;
-
+  const llvm::Triple *AuxTriple = IsCuda ? TC.getAuxTriple() : nullptr;
   bool IsWindowsGNU = RawTriple.isWindowsGNUEnvironment();
   bool IsWindowsCygnus = RawTriple.isWindowsCygwinEnvironment();
   bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
@@ -3276,7 +3275,7 @@ void Clang::ConstructJob(Compilation ,
   // Push all default warning arguments that are specific to
   // the given target.  These come before user provided warning options
   // are provided.
-  getToolChain().addClangWarningOptions(CmdArgs);
+  TC.addClangWarningOptions(CmdArgs);
 
   // Select the appropriate action.
   RewriteKind rewriteKind = RK_None;
@@ -3428,7 +3427,7 @@ void Clang::ConstructJob(Compilation ,
 
   CheckCodeGenerationOptions(D, Args);
 
-  unsigned FunctionAlignment = ParseFunctionAlignment(getToolChain(), Args);
+  unsigned FunctionAlignment = ParseFunctionAlignment(TC, Args);
   assert(FunctionAlignment <= 31 && "function alignment will be truncated!");
   if (FunctionAlignment) {
 CmdArgs.push_back("-function-alignment");
@@ -3438,8 +3437,7 @@ void Clang::ConstructJob(Compilation ,
   llvm::Reloc::Model RelocationModel;
   unsigned PICLevel;
   bool IsPIE;
-  std::tie(RelocationModel, PICLevel, IsPIE) =
-  ParsePICArgs(getToolChain(), Args);
+  std::tie(RelocationModel, PICLevel, IsPIE) = ParsePICArgs(TC, Args);
 
   const char *RMName = RelocationModelName(RelocationModel);
 
@@ -3467,13 +3465,13 @@ void Clang::ConstructJob(Compilation ,
 
   CmdArgs.push_back("-mthread-model");
   if (Arg *A = Args.getLastArg(options::OPT_mthread_model)) {
-if (!getToolChain().isThreadModelSupported(A->getValue()))
+if (!TC.isThreadModelSupported(A->getValue()))
   D.Diag(diag::err_drv_invalid_thread_model_for_target)
   << A->getValue() << A->getAsString(Args);
 CmdArgs.push_back(A->getValue());
   }
   else
-CmdArgs.push_back(Args.MakeArgString(getToolChain().getThreadModel()));
+CmdArgs.push_back(Args.MakeArgString(TC.getThreadModel()));
 
   Args.AddLastArg(CmdArgs, options::OPT_fveclib);
 
@@ -3528,7 +3526,7 @@ void Clang::ConstructJob(Compilation ,
 
   if (Arg *A = Args.getLastArg(options::OPT_fpcc_struct_return,
options::OPT_freg_struct_return)) {
-if (getToolChain().getArch() != llvm::Triple::x86) {
+if (TC.getArch() != llvm::Triple::x86) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getSpelling() << RawTriple.str();
 } else if (A->getOption().matches(options::OPT_fpcc_struct_return)) {
@@ -3593,18 +3591,17 @@ void Clang::ConstructJob(Compilation ,
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("-split-stacks");
 
-  RenderFloatingPointOptions(getToolChain(), D, OFastEnabled, Args, CmdArgs);
+  RenderFloatingPointOptions(TC, D, OFastEnabled, Args, CmdArgs);
 
   // Decide whether to use verbose asm. Verbose assembly is the default on
   // toolchains which have the integrated assembler on by default.
-  bool IsIntegratedAssemblerDefault =
-  getToolChain().IsIntegratedAssemblerDefault();
+  bool IsIntegratedAssemblerDefault = TC.IsIntegratedAssemblerDefault();
   if (Args.hasFlag(options::OPT_fverbose_asm, options::OPT_fno_verbose_asm,

[PATCH] D52179: [clang-tidy] Replace redundant checks with an assert().

[Artem Belevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE342514: [clang-tidy] Replace redundant checks with an 
assert(). (authored by tra, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52179?vs=165841=166042#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52179

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp


Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -385,6 +385,9 @@
 const NamedDecl *D,
 const std::vector>
 ) {
+  assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() 
&&
+ "Decl must be an explicit identifier with a name.");
+
   if (isa(D) && NamingStyles[SK_ObjcIvar])
 return SK_ObjcIvar;
   
@@ -548,8 +551,6 @@
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->isUsualDeallocationFunction() ||
-Decl->isCopyAssignmentOperator() || Decl->isMoveAssignmentOperator() ||
 Decl->size_overridden_methods() > 0)
   return SK_Invalid;
 


Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -385,6 +385,9 @@
 const NamedDecl *D,
 const std::vector>
 ) {
+  assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() &&
+ "Decl must be an explicit identifier with a name.");
+
   if (isa(D) && NamingStyles[SK_ObjcIvar])
 return SK_ObjcIvar;
   
@@ -548,8 +551,6 @@
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->isUsualDeallocationFunction() ||
-Decl->isCopyAssignmentOperator() || Decl->isMoveAssignmentOperator() ||
 Decl->size_overridden_methods() > 0)
   return SK_Invalid;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r342514 - [clang-tidy] Replace redundant checks with an assert().

[Artem Belevich via cfe-commits]

Author: tra
Date: Tue Sep 18 14:51:02 2018
New Revision: 342514

URL: http://llvm.org/viewvc/llvm-project?rev=342514=rev
Log:
[clang-tidy] Replace redundant checks with an assert().

findStyleKind is only called if D is an explicit identifier with a name,
so the checks for operators will never return true. The explicit assert()
enforces this invariant.

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

Modified:
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=342514=342513=342514=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp 
Tue Sep 18 14:51:02 2018
@@ -385,6 +385,9 @@ static StyleKind findStyleKind(
 const NamedDecl *D,
 const std::vector>
 ) {
+  assert(D && D->getIdentifier() && !D->getName().empty() && !D->isImplicit() 
&&
+ "Decl must be an explicit identifier with a name.");
+
   if (isa(D) && NamingStyles[SK_ObjcIvar])
 return SK_ObjcIvar;
   
@@ -548,8 +551,6 @@ static StyleKind findStyleKind(
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->isUsualDeallocationFunction() ||
-Decl->isCopyAssignmentOperator() || Decl->isMoveAssignmentOperator() ||
 Decl->size_overridden_methods() > 0)
   return SK_Invalid;
 


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

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 166041.
JonasToth added a comment.

- Merge branch 'master' into experiment_isolate_decl
- further work on isolate decl, fix tokenizing bug with templates
- include big test-suite and make them run


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,365 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = 
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = 
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template 
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType TT3;
+  //
+  TemplatedType *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK FIXES: TemplatedType *TT4(nullptr);
+  // CHECK FIXES: {{^  }}TemplatedType TT5;
+  // CHECK FIXES: {{^  }}TemplatedType **TT6 = nullptr;
+  // CHECK FIXES: {{^  }}TemplatedType *const *const TT7{nullptr};
+  //
+  TemplatedType **TT8(nullptr), TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so 

[PATCH] D52250: [clangd] expose MergedIndex class

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.

This allows inheriting from it, so index() can ga away and allowing
TestTU::index) to be fixed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52250

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Merge.cpp
  clangd/index/Merge.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -48,11 +48,12 @@
   return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
-// FIXME: This should return a FileIndex with both preamble and main index.
 std::unique_ptr TestTU::index() const {
   auto AST = build();
-  auto Content = indexMainDecls(AST);
-  return MemIndex::build(std::move(Content.first), std::move(Content.second));
+  auto Idx = llvm::make_unique();
+  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
+  Idx->updateMain(Filename, AST);
+  return Idx;
 }
 
 const Symbol (const SymbolSlab , llvm::StringRef QName) {
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -161,24 +161,24 @@
 TEST(MergeIndexTest, Lookup) {
   auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"}), RefSlab()),
J = MemIndex::build(generateSymbols({"ns::B", "ns::C"}), RefSlab());
-  auto M = mergeIndex(I.get(), J.get());
-  EXPECT_THAT(lookup(*M, SymbolID("ns::A")), UnorderedElementsAre("ns::A"));
-  EXPECT_THAT(lookup(*M, SymbolID("ns::B")), UnorderedElementsAre("ns::B"));
-  EXPECT_THAT(lookup(*M, SymbolID("ns::C")), UnorderedElementsAre("ns::C"));
-  EXPECT_THAT(lookup(*M, {SymbolID("ns::A"), SymbolID("ns::B")}),
+  MergedIndex M(I.get(), J.get());
+  EXPECT_THAT(lookup(M, SymbolID("ns::A")), UnorderedElementsAre("ns::A"));
+  EXPECT_THAT(lookup(M, SymbolID("ns::B")), UnorderedElementsAre("ns::B"));
+  EXPECT_THAT(lookup(M, SymbolID("ns::C")), UnorderedElementsAre("ns::C"));
+  EXPECT_THAT(lookup(M, {SymbolID("ns::A"), SymbolID("ns::B")}),
   UnorderedElementsAre("ns::A", "ns::B"));
-  EXPECT_THAT(lookup(*M, {SymbolID("ns::A"), SymbolID("ns::C")}),
+  EXPECT_THAT(lookup(M, {SymbolID("ns::A"), SymbolID("ns::C")}),
   UnorderedElementsAre("ns::A", "ns::C"));
-  EXPECT_THAT(lookup(*M, SymbolID("ns::D")), UnorderedElementsAre());
-  EXPECT_THAT(lookup(*M, {}), UnorderedElementsAre());
+  EXPECT_THAT(lookup(M, SymbolID("ns::D")), UnorderedElementsAre());
+  EXPECT_THAT(lookup(M, {}), UnorderedElementsAre());
 }
 
 TEST(MergeIndexTest, FuzzyFind) {
   auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"}), RefSlab()),
J = MemIndex::build(generateSymbols({"ns::B", "ns::C"}), RefSlab());
   FuzzyFindRequest Req;
   Req.Scopes = {"ns::"};
-  EXPECT_THAT(match(*mergeIndex(I.get(), J.get()), Req),
+  EXPECT_THAT(match(MergedIndex(I.get(), J.get()), Req),
   UnorderedElementsAre("ns::A", "ns::B", "ns::C"));
 }
 
@@ -231,7 +231,7 @@
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn({"unittest"});
   FileIndex StaticIndex({"unittest"});
-  auto MergedIndex = mergeIndex((), ());
+  MergedIndex Merge(, );
 
   const char *HeaderCode = "class Foo;";
   auto HeaderSymbols = TestTU::withHeaderCode("class Foo;").headerSymbols();
@@ -266,7 +266,7 @@
   RefsRequest Request;
   Request.IDs = {Foo.ID};
   RefSlab::Builder Results;
-  MergedIndex->refs(Request, [&](const Ref ) { Results.insert(Foo.ID, O); });
+  Merge.refs(Request, [&](const Ref ) { Results.insert(Foo.ID, O); });
 
   EXPECT_THAT(
   std::move(Results).build(),
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -119,10 +119,10 @@
   EXPECT_THAT(getRefs(*Symbols, ID), RefsAre({FileURI("f1.cc")}));
 }
 
-std::vector match(const FileIndex ,
+std::vector match(const SymbolIndex ,
const FuzzyFindRequest ) {
   std::vector Matches;
-  I.index().fuzzyFind(Req, [&](const Symbol ) {
+  I.fuzzyFind(Req, [&](const Symbol ) {
 Matches.push_back((Sym.Scope + Sym.Name).str());
   });
   return Matches;
@@ -146,7 +146,7 @@
   FuzzyFindRequest Req;
   Req.Query = "";
   bool SeenSymbol = false;
-  M.index().fuzzyFind(Req, [&](const Symbol ) {
+  M.fuzzyFind(Req, [&](const Symbol ) {
 EXPECT_EQ(Sym.CanonicalDeclaration.FileURI, "unittest:///f.h");
 SeenSymbol = true;
   });
@@ -200,7 +200,7 @@
   FuzzyFindRequest Req;
   Req.Query = "";
   bool SeenSymbol = false;
-  M.index().fuzzyFind(Req, [&](const Symbol ) 

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

r342512 - Driver: hoist `-mlimit-float-precision` (NFC)

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Tue Sep 18 14:12:39 2018
New Revision: 342512

URL: http://llvm.org/viewvc/llvm-project?rev=342512=rev
Log:
Driver: hoist `-mlimit-float-precision` (NFC)

Move the floating point argument handling into the RenderFloatingPointOptions
helper.  This relocation just puts the floating point related options into a
single location.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342512=342511=342512=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 14:12:39 2018
@@ -2085,6 +2085,11 @@ static void RenderFloatingPointOptions(c
   StringRef DenormalFPMath = "";
   StringRef FPContract = "";
 
+  if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {
+CmdArgs.push_back("-mlimit-float-precision");
+CmdArgs.push_back(A->getValue());
+  }
+
   for (const Arg *A : Args) {
 switch (A->getOption().getID()) {
 // If this isn't an FP option skip the claim below
@@ -3662,11 +3667,6 @@ void Clang::ConstructJob(Compilation ,
   getToolChain().addClangTargetOptions(Args, CmdArgs,
JA.getOffloadingDeviceKind());
 
-  if (Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) {
-CmdArgs.push_back("-mlimit-float-precision");
-CmdArgs.push_back(A->getValue());
-  }
-
   // FIXME: Handle -mtune=.
   (void)Args.hasArg(options::OPT_mtune_EQ);
 


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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166035.
nickdesaulniers added a comment.

git-clang-format HEAD~


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,9 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() && !S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,9 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() && !S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, gbiv.
Herald added a reviewer: george.burgess.iv.
Herald added a subscriber: cfe-commits.
nickdesaulniers removed a reviewer: gbiv.

Fixes PR32985.


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1680,7 +1680,9 @@
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
 if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+&& TypeQuals & Result.getCVRQualifiers() &&
+!S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1680,7 +1680,9 @@
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
 if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+&& TypeQuals & Result.getCVRQualifiers() &&
+!S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Yvan Roux via cfe-commits]

Seems that it was fixed earlier today.

Thanks,
Yvan

On Tue, 18 Sep 2018 at 19:31, Yvan Roux  wrote:
>
> Hi Sam,
>
> It took a very long time to identify it, but this commit broke ARMv7
> bots, where this test hangs.  Logs are available here (initial ones
> are too old):
>
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/3685/steps/ninja%20check%201/logs/stdiohttp://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/3685/steps/ninja%20check%201/logs/stdio
>
> Thanks,
> YvanOn Thu, 30 Aug 2018 at 17:15, Sam McCall via Phabricator via
> cfe-commits  wrote:
> >
> > sammccall added inline comments.
> >
> >
> > 
> > Comment at: clangd/TUScheduler.cpp:474
> > +llvm::unique_function)> 
> > Callback) {
> > +  if (RunSync)
> > +return Callback(getPossiblyStalePreamble());
> > 
> > ilya-biryukov wrote:
> > > It seems we could remove the special-casing of `RunSync` and still get 
> > > correct results (the Requests queue is always empty).
> > > But feel free to keep it for clarity.
> > Right, of course :-)
> > Replaced this with an assert before we write to the queue.
> >
> >
> > 
> > Comment at: clangd/TUScheduler.h:123
> > +  /// Controls whether preamble reads wait for the preamble to be 
> > up-to-date.
> > +  enum PreambleConsistency {
> > +/// The preamble is generated from the current version of the file.
> > 
> > ilya-biryukov wrote:
> > > Maybe use a strongly-typed enum outside the class?
> > > `TUScheduler::Stale` will turn into `PreambleConsistency::Stale` at the 
> > > call-site. The main upside is that it does not pollute completions inside 
> > > the class with enumerators.
> > >
> > > Just a suggestion, feel free to ignore.
> > Yeah, the downside to that is that it *does* pollute the clangd:: 
> > namespace. So both are a bit sad.
> >
> > This header is fairly widely visible (since it's included by clangdserver) 
> > and this API is fairly narrowly interesting, so as far as completion goes I 
> > think I prefer it being hidden in TUScheduler.
> >
> >
> > Repository:
> >   rL LLVM
> >
> > https://reviews.llvm.org/D51438
> >
> >
> >
> > ___
> > 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] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D50901#1238795, @lebedev.ri wrote:

> @vsk thanks for taking a look!


/me can't read :)
That was supposed to be @vitalybuka, of course (:


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

@vsk thanks for taking a look!




Comment at: lib/CodeGen/CGExprScalar.cpp:305
   enum ImplicitConversionCheckKind : unsigned char {
-ICCK_IntegerTruncation = 0,
+ICCK_IntegerTruncation = 0, // Legacy, no longer used.
+ICCK_UnsignedIntegerTruncation = 1,

vitalybuka wrote:
> why do you need to keep it?
*Here* - for consistency with the compiler-rt part.

There - what about mismatch in the used clang version
(which still only produced the `(ImplicitConversionCheckKind)0`), and 
compiler-rt version
(which has `(ImplicitConversionCheckKind)1` and 
`(ImplicitConversionCheckKind)2`)?
Is it 100.00% guaranteed not to happen? I'm not so sure.


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:305
   enum ImplicitConversionCheckKind : unsigned char {
-ICCK_IntegerTruncation = 0,
+ICCK_IntegerTruncation = 0, // Legacy, no longer used.
+ICCK_UnsignedIntegerTruncation = 1,

why do you need to keep it?


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

Re: r342501 - Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via cfe-commits]

r342510 by dblaikie fixed this.

Thanks,
Steve

On Tue, Sep 18, 2018 at 1:18 PM Stephen Hines  wrote:

> Sure, I'm looking now.
>
> Thanks,
> Steve
>
> On Tue, Sep 18, 2018 at 1:02 PM  wrote:
>
>> Hi Stephen,
>>
>> Your change is causing a test failure on the PS4 linux bot, can you
>> please take a look?
>>
>>
>> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/36712/steps/test/logs/stdio
>>
>> FAIL: Clang :: Driver/clang_f_opts.c (8141 of 44013)
>>  TEST 'Clang :: Driver/clang_f_opts.c' FAILED
>> 
>> ...
>> Command Output (stderr):
>> --
>> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/clang_f_opts.c:537:29:
>> error: CHECK-NO-MIX-OMIT-FP-PG: expected string not found in input
>> // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
>> ^
>> :1:1: note: scanning from here
>> clang version 8.0.0 (trunk 342502)
>> ^
>> :5:934: note: possible intended match here
>>  
>> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang-8"
>> "-cc1" "-triple" "x86_64-scei-ps4" "-S" "-disable-free" "-main-file-name"
>> "clang_f_opts.c" "-mrelocation-model" "pic" "-pic-level" "2"
>> "-mthread-model" "posix" "-masm-verbose" "-mconstructor-aliases"
>> "-munwind-tables" "-target-cpu" "btver2" "-debugger-tuning=sce" "-mllvm"
>> "-generate-arange-section" "-debug-forward-template-params"
>> "-dwarf-explicit-import" "-coverage-notes-file"
>> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver/clang_f_opts.gcno"
>> "-resource-dir"
>> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0"
>> "-fdebug-compilation-dir"
>> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver"
>> "-fstack-size-section" "-ferror-limit" "19" "-fmessage-length" "0" "-pg"
>> "-stack-protector" "2" "-fdeclspec" "-fobjc-runtime=gnustep"
>> "-fdiagnostics-show-option" "-o" "clang_f_opts.s" "-x" "c"
>> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/clang_f_opts.c"
>> "-faddrsig"
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>^
>>
>> Douglas Yung
>>
>> > -Original Message-
>> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
>> > Of Stephen Hines via cfe-commits
>> > Sent: Tuesday, September 18, 2018 11:35
>> > To: cfe-commits@lists.llvm.org
>> > Subject: r342501 - Fix logic around determining use of frame pointer
>> > with -pg.
>> >
>> > Author: srhines
>> > Date: Tue Sep 18 11:34:33 2018
>> > New Revision: 342501
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=342501=rev
>> > Log:
>> > Fix logic around determining use of frame pointer with -pg.
>> >
>> > Summary:
>> > As part of r342165, I rewrote the logic to check whether
>> > -fno-omit-frame-pointer was passed after a -fomit-frame-pointer
>> > argument. This CL switches that logic to use the consolidated
>> > shouldUseFramePointer() function. This fixes a potential issue where -
>> > pg
>> > gets used with -fomit-frame-pointer on a platform that must always
>> > retain
>> > frame pointers.
>> >
>> > Reviewers: dblaikie
>> >
>> > Reviewed By: dblaikie
>> >
>> > Subscribers: cfe-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D52191
>> >
>> > Modified:
>> > cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> >
>> > Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342501=342500&
>> > r2=342501=diff
>> > ===
>> > ===
>> > --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
>> > +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 11:34:33 2018
>> > @@ -4956,8 +4956,7 @@ void Clang::ConstructJob(Compilation ,
>> >}
>> >
>> >if (Arg *A = Args.getLastArg(options::OPT_pg))
>> > -if (Args.hasFlag(options::OPT_fomit_frame_pointer,
>> > - options::OPT_fno_omit_frame_pointer,
>> > /*default=*/false))
>> > +if (shouldUseFramePointer(Args, Triple))
>> >D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-
>> > frame-pointer"
>> ><< A-
>> > >getAsString(Args);
>> >
>> >
>> >
>> > ___
>> > 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] D52191: Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

Thanks @dblaikie for the quick fixup. I must have accidentally dropped the '!', 
because I did run check-all to test the change.


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

Re: r342501 - Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via cfe-commits]

Sure, I'm looking now.

Thanks,
Steve

On Tue, Sep 18, 2018 at 1:02 PM  wrote:

> Hi Stephen,
>
> Your change is causing a test failure on the PS4 linux bot, can you please
> take a look?
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/36712/steps/test/logs/stdio
>
> FAIL: Clang :: Driver/clang_f_opts.c (8141 of 44013)
>  TEST 'Clang :: Driver/clang_f_opts.c' FAILED
> 
> ...
> Command Output (stderr):
> --
> /home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/clang_f_opts.c:537:29:
> error: CHECK-NO-MIX-OMIT-FP-PG: expected string not found in input
> // CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
> ^
> :1:1: note: scanning from here
> clang version 8.0.0 (trunk 342502)
> ^
> :5:934: note: possible intended match here
>  
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang-8"
> "-cc1" "-triple" "x86_64-scei-ps4" "-S" "-disable-free" "-main-file-name"
> "clang_f_opts.c" "-mrelocation-model" "pic" "-pic-level" "2"
> "-mthread-model" "posix" "-masm-verbose" "-mconstructor-aliases"
> "-munwind-tables" "-target-cpu" "btver2" "-debugger-tuning=sce" "-mllvm"
> "-generate-arange-section" "-debug-forward-template-params"
> "-dwarf-explicit-import" "-coverage-notes-file"
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver/clang_f_opts.gcno"
> "-resource-dir"
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0"
> "-fdebug-compilation-dir"
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver"
> "-fstack-size-section" "-ferror-limit" "19" "-fmessage-length" "0" "-pg"
> "-stack-protector" "2" "-fdeclspec" "-fobjc-runtime=gnustep"
> "-fdiagnostics-show-option" "-o" "clang_f_opts.s" "-x" "c"
> "/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/clang_f_opts.c"
> "-faddrsig"
>
>
>
>
>
>
>
>
>
>
>
>
>^
>
> Douglas Yung
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> > Of Stephen Hines via cfe-commits
> > Sent: Tuesday, September 18, 2018 11:35
> > To: cfe-commits@lists.llvm.org
> > Subject: r342501 - Fix logic around determining use of frame pointer
> > with -pg.
> >
> > Author: srhines
> > Date: Tue Sep 18 11:34:33 2018
> > New Revision: 342501
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=342501=rev
> > Log:
> > Fix logic around determining use of frame pointer with -pg.
> >
> > Summary:
> > As part of r342165, I rewrote the logic to check whether
> > -fno-omit-frame-pointer was passed after a -fomit-frame-pointer
> > argument. This CL switches that logic to use the consolidated
> > shouldUseFramePointer() function. This fixes a potential issue where -
> > pg
> > gets used with -fomit-frame-pointer on a platform that must always
> > retain
> > frame pointers.
> >
> > Reviewers: dblaikie
> >
> > Reviewed By: dblaikie
> >
> > Subscribers: cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D52191
> >
> > Modified:
> > cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> >
> > Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> > URL: http://llvm.org/viewvc/llvm-
> > project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342501=342500&
> > r2=342501=diff
> > ===
> > ===
> > --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
> > +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 11:34:33 2018
> > @@ -4956,8 +4956,7 @@ void Clang::ConstructJob(Compilation ,
> >}
> >
> >if (Arg *A = Args.getLastArg(options::OPT_pg))
> > -if (Args.hasFlag(options::OPT_fomit_frame_pointer,
> > - options::OPT_fno_omit_frame_pointer,
> > /*default=*/false))
> > +if (shouldUseFramePointer(Args, Triple))
> >D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-
> > frame-pointer"
> ><< A-
> > >getAsString(Args);
> >
> >
> >
> > ___
> > 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] D52191: Fix logic around determining use of frame pointer with -pg.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: srhines.
dblaikie added a comment.

Fixed in r342510 with the solution I mentioned up-thread.


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

Re: [PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[David Blaikie via cfe-commits]

Fixed in r342510 with the solution I mentioned up-thread.

On Tue, Sep 18, 2018 at 1:10 PM Volodymyr Sapsai via Phabricator <
revi...@reviews.llvm.org> wrote:

> vsapsai added a comment.
>
> Confirm that reverting the change locally fixes the tests. If nobody beats
> me to it, I plan to revert the change in 30-60 minutes. @srhines, if you
> want to fix it in another way and need more time, please let me know.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D52191
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342510 - Fix fomit-frame-pointe+pg error

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Tue Sep 18 13:11:45 2018
New Revision: 342510

URL: http://llvm.org/viewvc/llvm-project?rev=342510=rev
Log:
Fix fomit-frame-pointe+pg error

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342510=342509=342510=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 13:11:45 2018
@@ -4956,7 +4956,7 @@ void Clang::ConstructJob(Compilation ,
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (shouldUseFramePointer(Args, Triple))
+if (!shouldUseFramePointer(Args, Triple))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


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

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Confirm that reverting the change locally fixes the tests. If nobody beats me 
to it, I plan to revert the change in 30-60 minutes. @srhines, if you want to 
fix it in another way and need more time, please let me know.


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

RE: r342501 - Fix logic around determining use of frame pointer with -pg.

[via cfe-commits]

Hi Stephen,

Your change is causing a test failure on the PS4 linux bot, can you please take 
a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/36712/steps/test/logs/stdio

FAIL: Clang :: Driver/clang_f_opts.c (8141 of 44013)
 TEST 'Clang :: Driver/clang_f_opts.c' FAILED 

...
Command Output (stderr):
--
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/clang_f_opts.c:537:29:
 error: CHECK-NO-MIX-OMIT-FP-PG: expected string not found in input
// CHECK-NO-MIX-OMIT-FP-PG: '-fomit-frame-pointer' not allowed with '-pg'
^
:1:1: note: scanning from here
clang version 8.0.0 (trunk 342502)
^
:5:934: note: possible intended match here
 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang-8"
 "-cc1" "-triple" "x86_64-scei-ps4" "-S" "-disable-free" "-main-file-name" 
"clang_f_opts.c" "-mrelocation-model" "pic" "-pic-level" "2" "-mthread-model" 
"posix" "-masm-verbose" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" 
"btver2" "-debugger-tuning=sce" "-mllvm" "-generate-arange-section" 
"-debug-forward-template-params" "-dwarf-explicit-import" 
"-coverage-notes-file" 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver/clang_f_opts.gcno"
 "-resource-dir" 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/lib/clang/8.0.0"
 "-fdebug-compilation-dir" 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver"
 "-fstack-size-section" "-ferror-limit" "19" "-fmessage-length" "0" "-pg" 
"-stack-protector" "2" "-fdeclspec" "-fobjc-runtime=gnustep" 
"-fdiagnostics-show-option" "-o" "clang_f_opts.s" "-x" "c" 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/clang_f_opts.c"
 "-faddrsig"











 ^

Douglas Yung

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of Stephen Hines via cfe-commits
> Sent: Tuesday, September 18, 2018 11:35
> To: cfe-commits@lists.llvm.org
> Subject: r342501 - Fix logic around determining use of frame pointer
> with -pg.
> 
> Author: srhines
> Date: Tue Sep 18 11:34:33 2018
> New Revision: 342501
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=342501=rev
> Log:
> Fix logic around determining use of frame pointer with -pg.
> 
> Summary:
> As part of r342165, I rewrote the logic to check whether
> -fno-omit-frame-pointer was passed after a -fomit-frame-pointer
> argument. This CL switches that logic to use the consolidated
> shouldUseFramePointer() function. This fixes a potential issue where -
> pg
> gets used with -fomit-frame-pointer on a platform that must always
> retain
> frame pointers.
> 
> Reviewers: dblaikie
> 
> Reviewed By: dblaikie
> 
> Subscribers: cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D52191
> 
> Modified:
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> 
> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342501=342500&
> r2=342501=diff
> ===
> ===
> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 11:34:33 2018
> @@ -4956,8 +4956,7 @@ void Clang::ConstructJob(Compilation ,
>}
> 
>if (Arg *A = Args.getLastArg(options::OPT_pg))
> -if (Args.hasFlag(options::OPT_fomit_frame_pointer,
> - options::OPT_fno_omit_frame_pointer,
> /*default=*/false))
> +if (shouldUseFramePointer(Args, Triple))
>D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-
> frame-pointer"
><< A-
> >getAsString(Args);
> 
> 
> 
> 

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Rumeet Dhindsa via Phabricator via cfe-commits]

rdhindsa added a comment.

It seems that following tests are broken with this change:

clang/test/Driver/clang_f_opts.c
clang/test/Frontend/gnu-mcount.c


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

@rsmith - ping.
This one should be rather uncontroversial i think? Is this moving in the 
direction you suggested? :)


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Seems like this change causes 2 test failures:

  Clang :: Driver/clang_f_opts.c
  Clang :: Frontend/gnu-mcount.c

Some of the failing bots are

http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/15363/
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/53328/


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[Aaron Puchert via Phabricator via cfe-commits]

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

Thanks to both of you for the reviews. I'll see what I can do about the arrows. 
My gut feeling is that using `{Member,ObjCIVarRef}Expr::isArrow` is the right 
way, but it's not yet obvious to me how.




Comment at: include/clang/Analysis/Analyses/ThreadSafetyCommon.h:400
   til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx);
+  til::SExpr *translateObjCIVarRefExpr(const ObjCIvarRefExpr *ME,
+   CallingContext *Ctx);

aaron.ballman wrote:
> `ME` is kind of a poor name; can you switch to `IVRE` like you used in the 
> implementation?
Of course, that's a copy-and-paste error.



Comment at: lib/Analysis/ThreadSafetyCommon.cpp:362
+  til::Project *P = new (Arena) til::Project(E, D);
+  if (hasCppPointerType(BE))
+P->setArrow(true);

rjmccall wrote:
> aaron.ballman wrote:
> > I feel like this will always return false. However, I wonder if 
> > `IVRE->getBase()->isArrow()` is more appropriate here?
> The base of an `ObjCIvarRefExpr` will never directly be a C pointer type.  I 
> don't know whether this data structure looks through casts, but it's 
> certainly *possible* to cast arbitrarily weird C stuff to ObjC pointer type.  
> On the other hand, by and large nobody actually ever does that, so I wouldn't 
> make a special case for it here.
> 
> `ObjCIvarRefExpr` just has an `isArrow()` method directly if this is just 
> supposed to be capturing the difference between `.` and `->`.  But then, so 
> does `MemberExpr`, and in that case you're doing this odd analysis of the 
> base instead of trusting `isArrow()`, so I don't know what to tell you to do.
> 
> Overall it is appropriate to treat an ObjC ivar reference as a perfectly 
> ordinary projection.  The only thing that's special about it is that the 
> actual offset isn't necessarily statically known, but ivars still behave as 
> if they're all independently declared, so I wouldn't worry about it.
I had wondered about this as well, but when I changed `hasCppPointerType(BE)` 
to `ME->isArrow()` in the `MemberExpr` case, I got some test failures. For 
example:

```
struct Foo {
  int a __attribute__((guarded_by(mu)));
  Mutex mu;
};

void f() {
  Foo foo;
  foo.a = 5; // \
// expected-warning{{writing variable 'a' requires holding mutex 'foo.mu' 
exclusively}}
}
```

Instead we warn `writing variable 'a' requires holding mutex 'foo->mu' 
exclusively`. That's not right.

The expression (`ME`) we are processing is `mu` from the attribute on `a`, 
which the compiler sees as `this->mu`. So we get `ME->isArrow() == true` and 
`ME->getBase()` is a `CXXThisExpr`.

Basically the `translate*` functions do a substitution. They try to derive 
`foo.mu` from `this->mu` on the `a` member and the expression `foo.a`. The 
annotation `this->mu` is the expression we get as parameter, and `foo.a` is 
encoded in the `CallingContext`.

The information whether `foo.a` is an arrow (it isn't) seems to be in the 
`CallingContext`'s `SelfArrow` member.


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

Re: [clang-tools-extra] r342227 - [clangd] NFC: Fix IndexBenchmark CLI arguments handling

[Kirill Bobyrev via cfe-commits]

Thanks for the explanation! I didn’t know that benchmark’s Initialize does 
that, that was probably the source of my confusion. The suggestion looks 
reasonable, I should try this approach, it looks to be cleaner.

-Kirill

> On 18 Sep 2018, at 21:16, Roman Lebedev  wrote:
> 
> On Tue, Sep 18, 2018 at 10:09 PM, Kirill Bobyrev
>  wrote:
>> Hi Roman,
>> 
>> Is there any benefit of doing so? Also, I’m not sure whether I understood 
>> you correctly. Consuming benchmark options *before* trimming would probably 
>> not be the desired behaviour since the first two arguments arguments are 
>> passed directly to the tool driver.
> Currently, you have to call it like
> $ IndexBenchmark IndexFilename RequestsFilename --benchmark_something
> 
> If you do
> $ IndexBenchmark --benchmark_something IndexFilename RequestsFilename
> your code will still consume argv[1] (i.e. "--benchmark_something") as
> IndexFilename
> 
> But if you do call  ::benchmark::Initialize(, argv);  first it
> will work as you'd expect,
> i.e. you would still consume argv[1], but it then would have been
> adjusted to IndexFilename
> 
>> I might have misunderstood you, could you please elaborate on the proposed 
>> idea?
>> 
>> Kind regards,
>> Kirill
> Roman.
> 
>>> On 14 Sep 2018, at 14:46, Roman Lebedev  wrote:
>>> 
>>> On Fri, Sep 14, 2018 at 3:21 PM, Kirill Bobyrev via cfe-commits
>>>  wrote:
 Author: omtcyfz
 Date: Fri Sep 14 05:21:09 2018
 New Revision: 342227
 
 URL: http://llvm.org/viewvc/llvm-project?rev=342227=rev
 Log:
 [clangd] NFC: Fix IndexBenchmark CLI arguments handling
 
 Modified:
   clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
 
 Modified: clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp?rev=342227=342226=342227=diff
 ==
 --- clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp (original)
 +++ clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp Fri Sep 
 14 05:21:09 2018
 @@ -101,9 +101,11 @@ int main(int argc, char *argv[]) {
  }
>>> 
  IndexFilename = argv[1];
  RequestsFilename = argv[2];
 -  // Trim first two arguments of the benchmark invocation.
 -  argv += 3;
 -  argc -= 3;
 +  // Trim first two arguments of the benchmark invocation and pretend no
 +  // arguments were passed in the first place.
 +  argv[2] = argv[0];
 +  argv += 2;
 +  argc -= 2;
  ::benchmark::Initialize(, argv);
>>> Passing-by thought: why is this being done in *this* order?
>>> Why not first let the ::benchmark::Initialize() consume it's flags first?
>>> 
  ::benchmark::RunSpecifiedBenchmarks();
 }
 
>>> Roman.
>>> 
 ___
 cfe-commits mailing list
 cfe-commits@lists.llvm.org
 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> 

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

Re: [clang-tools-extra] r342227 - [clangd] NFC: Fix IndexBenchmark CLI arguments handling

[Roman Lebedev via cfe-commits]

On Tue, Sep 18, 2018 at 10:09 PM, Kirill Bobyrev
 wrote:
> Hi Roman,
>
> Is there any benefit of doing so? Also, I’m not sure whether I understood you 
> correctly. Consuming benchmark options *before* trimming would probably not 
> be the desired behaviour since the first two arguments arguments are passed 
> directly to the tool driver.
Currently, you have to call it like
$ IndexBenchmark IndexFilename RequestsFilename --benchmark_something

If you do
$ IndexBenchmark --benchmark_something IndexFilename RequestsFilename
your code will still consume argv[1] (i.e. "--benchmark_something") as
IndexFilename

But if you do call  ::benchmark::Initialize(, argv);  first it
will work as you'd expect,
i.e. you would still consume argv[1], but it then would have been
adjusted to IndexFilename

> I might have misunderstood you, could you please elaborate on the proposed 
> idea?
>
> Kind regards,
> Kirill
Roman.

>> On 14 Sep 2018, at 14:46, Roman Lebedev  wrote:
>>
>> On Fri, Sep 14, 2018 at 3:21 PM, Kirill Bobyrev via cfe-commits
>>  wrote:
>>> Author: omtcyfz
>>> Date: Fri Sep 14 05:21:09 2018
>>> New Revision: 342227
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=342227=rev
>>> Log:
>>> [clangd] NFC: Fix IndexBenchmark CLI arguments handling
>>>
>>> Modified:
>>>clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
>>>
>>> Modified: clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp?rev=342227=342226=342227=diff
>>> ==
>>> --- clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp (original)
>>> +++ clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp Fri Sep 14 
>>> 05:21:09 2018
>>> @@ -101,9 +101,11 @@ int main(int argc, char *argv[]) {
>>>   }
>>
>>>   IndexFilename = argv[1];
>>>   RequestsFilename = argv[2];
>>> -  // Trim first two arguments of the benchmark invocation.
>>> -  argv += 3;
>>> -  argc -= 3;
>>> +  // Trim first two arguments of the benchmark invocation and pretend no
>>> +  // arguments were passed in the first place.
>>> +  argv[2] = argv[0];
>>> +  argv += 2;
>>> +  argc -= 2;
>>>   ::benchmark::Initialize(, argv);
>> Passing-by thought: why is this being done in *this* order?
>> Why not first let the ::benchmark::Initialize() consume it's flags first?
>>
>>>   ::benchmark::RunSpecifiedBenchmarks();
>>> }
>>>
>> Roman.
>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r342227 - [clangd] NFC: Fix IndexBenchmark CLI arguments handling

[Kirill Bobyrev via cfe-commits]

Hi Roman,

Is there any benefit of doing so? Also, I’m not sure whether I understood you 
correctly. Consuming benchmark options *before* trimming would probably not be 
the desired behaviour since the first two arguments arguments are passed 
directly to the tool driver.

I might have misunderstood you, could you please elaborate on the proposed idea?

Kind regards,
Kirill

> On 14 Sep 2018, at 14:46, Roman Lebedev  wrote:
> 
> On Fri, Sep 14, 2018 at 3:21 PM, Kirill Bobyrev via cfe-commits
>  wrote:
>> Author: omtcyfz
>> Date: Fri Sep 14 05:21:09 2018
>> New Revision: 342227
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=342227=rev
>> Log:
>> [clangd] NFC: Fix IndexBenchmark CLI arguments handling
>> 
>> Modified:
>>clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
>> 
>> Modified: clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp?rev=342227=342226=342227=diff
>> ==
>> --- clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp (original)
>> +++ clang-tools-extra/trunk/clangd/benchmarks/IndexBenchmark.cpp Fri Sep 14 
>> 05:21:09 2018
>> @@ -101,9 +101,11 @@ int main(int argc, char *argv[]) {
>>   }
> 
>>   IndexFilename = argv[1];
>>   RequestsFilename = argv[2];
>> -  // Trim first two arguments of the benchmark invocation.
>> -  argv += 3;
>> -  argc -= 3;
>> +  // Trim first two arguments of the benchmark invocation and pretend no
>> +  // arguments were passed in the first place.
>> +  argv[2] = argv[0];
>> +  argv += 2;
>> +  argc -= 2;
>>   ::benchmark::Initialize(, argv);
> Passing-by thought: why is this being done in *this* order?
> Why not first let the ::benchmark::Initialize() consume it's flags first?
> 
>>   ::benchmark::RunSpecifiedBenchmarks();
>> }
>> 
> Roman.
> 
>> ___
>> 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] D52191: Fix logic around determining use of frame pointer with -pg.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D52191#1238648, @srhines wrote:

> In https://reviews.llvm.org/D52191#1238628, @dblaikie wrote:
>
> > Sure, looks good. Though my other/vague concern is why does this case error 
> > about fomit-frame-pointer having no effect, but other things (like using 
> > -fomit-frame-pointer on a target that requires frame pointers) that ignore 
> > -fomit-frame-pointer don't? Weird. But it probably makes sense somehow.
>
>
> I think the original issue is that one should not **explicitly** say "omit 
> frame pointers" and "instrument for profiling with mcount". It's possible 
> that there should be other errors for using "omit frame pointers" on targets 
> that require them, but that should be checked independently elsewhere. Do we 
> have many (any) of these kinds of targets? I'm going to submit this patch, 
> but am happy to add a diagnostic later on for the other case, if you think 
> that is worthwhile.


Yeah, looks like the other cases (targets, specifically - and yeah, I didn't 
look at the implementation of shouldUseFramePointer far enough to see which 
targets, etc - OK, I just looked now, and it's Darwin ARM & Thumb - so arguably 
on those targets, since -fomit-frame-pointer has no effect, maybe it's not 
important to error on -fomit-frame-pointer -pg)

Also I just realized you probably want/need "!shouldUseFramePointer" on your 
error. (missed the inversion) - sorry I didn't catch that in review. Should 
catch that with your test from the previous change?


Repository:
  rL LLVM

https://reviews.llvm.org/D52191



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

[clang-tools-extra] r342505 - [clangd] Fix error handling for SymbolID parsing (notably YAML and dexp)

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Sep 18 12:00:59 2018
New Revision: 342505

URL: http://llvm.org/viewvc/llvm-project?rev=342505=rev
Log:
[clangd] Fix error handling for SymbolID parsing (notably YAML and dexp)

Modified:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp

Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=342505=342504=342505=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Sep 18 12:00:59 2018
@@ -41,8 +41,13 @@ SymbolID SymbolID::fromRaw(llvm::StringR
 
 std::string SymbolID::str() const { return toHex(raw()); }
 
-void operator>>(StringRef Str, SymbolID ) {
-  ID = SymbolID::fromRaw(fromHex(Str));
+llvm::Expected SymbolID::fromStr(llvm::StringRef Str) {
+  if (Str.size() != RawSize * 2)
+return createStringError(llvm::inconvertibleErrorCode(), "Bad ID length");
+  for (char C : Str)
+if (!isHexDigit(C))
+  return createStringError(llvm::inconvertibleErrorCode(), "Bad hex ID");
+  return fromRaw(fromHex(Str));
 }
 
 raw_ostream <<(raw_ostream , SymbolOrigin O) {

Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=342505=342504=342505=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Sep 18 12:00:59 2018
@@ -91,12 +91,12 @@ public:
 return StringRef(reinterpret_cast(HashValue.data()), 
RawSize);
   }
   static SymbolID fromRaw(llvm::StringRef);
+
   // Returns a 40-bytes hex encoded string.
   std::string str() const;
+  static llvm::Expected fromStr(llvm::StringRef);
 
 private:
-  friend void operator>>(llvm::StringRef Str, SymbolID );
-
   std::array HashValue;
 };
 
@@ -108,15 +108,9 @@ inline llvm::hash_code hash_value(const
   return llvm::hash_code(Result);
 }
 
-// Write SymbolID into the given stream. SymbolID is encoded as a 40-bytes
-// hex string.
+// Write SymbolID into the given stream. SymbolID is encoded as ID.str().
 llvm::raw_ostream <<(llvm::raw_ostream , const SymbolID );
 
-// Construct SymbolID from a hex string.
-// The HexStr is required to be a 40-bytes hex string, which is encoded from 
the
-// "<<" operator.
-void operator>>(llvm::StringRef HexStr, SymbolID );
-
 } // namespace clangd
 } // namespace clang
 namespace llvm {

Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=342505=342504=342505=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Tue Sep 18 12:00:59 2018
@@ -40,10 +40,13 @@ struct NormalizedSymbolID {
 OS << ID;
   }
 
-  SymbolID denormalize(IO &) {
-SymbolID ID;
-HexString >> ID;
-return ID;
+  SymbolID denormalize(IO ) {
+auto ID = SymbolID::fromStr(HexString);
+if (!ID) {
+  I.setError(llvm::toString(ID.takeError()));
+  return SymbolID();
+}
+return *ID;
   }
 
   std::string HexString;

Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp?rev=342505=342504=342505=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp Tue Sep 18 12:00:59 
2018
@@ -144,14 +144,14 @@ class Lookup : public Command {
   };
 
   void run() override {
-auto Raw = fromHex(ID);
-if (Raw.size() != clang::clangd::SymbolID::RawSize) {
-  llvm::outs() << "invalid SymbolID\n";
+auto SID = clang::clangd::SymbolID::fromStr(ID);
+if (!SID) {
+  llvm::outs() << llvm::toString(SID.takeError()) << "\n";
   return;
 }
 
 clang::clangd::LookupRequest Request;
-Request.IDs = {clang::clangd::SymbolID::fromRaw(Raw)};
+Request.IDs = {*SID};
 bool FoundSymbol = false;
 Index->lookup(Request, [&](const Symbol ) {
   FoundSymbol = true;

Modified: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp?rev=342505=342504=342505=diff
==
--- 

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342501: Fix logic around determining use of frame pointer 
with -pg. (authored by srhines, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52191?vs=165826=166007#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52191

Files:
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4956,8 +4956,7 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (Args.hasFlag(options::OPT_fomit_frame_pointer,
- options::OPT_fno_omit_frame_pointer, /*default=*/false))
+if (shouldUseFramePointer(Args, Triple))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4956,8 +4956,7 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (Args.hasFlag(options::OPT_fomit_frame_pointer,
- options::OPT_fno_omit_frame_pointer, /*default=*/false))
+if (shouldUseFramePointer(Args, Triple))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342501: Fix logic around determining use of frame pointer 
with -pg. (authored by srhines, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52191

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -4956,8 +4956,7 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (Args.hasFlag(options::OPT_fomit_frame_pointer,
- options::OPT_fno_omit_frame_pointer, /*default=*/false))
+if (shouldUseFramePointer(Args, Triple))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -4956,8 +4956,7 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (Args.hasFlag(options::OPT_fomit_frame_pointer,
- options::OPT_fno_omit_frame_pointer, /*default=*/false))
+if (shouldUseFramePointer(Args, Triple))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342501 - Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via cfe-commits]

Author: srhines
Date: Tue Sep 18 11:34:33 2018
New Revision: 342501

URL: http://llvm.org/viewvc/llvm-project?rev=342501=rev
Log:
Fix logic around determining use of frame pointer with -pg.

Summary:
As part of r342165, I rewrote the logic to check whether
-fno-omit-frame-pointer was passed after a -fomit-frame-pointer
argument. This CL switches that logic to use the consolidated
shouldUseFramePointer() function. This fixes a potential issue where -pg
gets used with -fomit-frame-pointer on a platform that must always retain
frame pointers.

Reviewers: dblaikie

Reviewed By: dblaikie

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=342501=342500=342501=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Sep 18 11:34:33 2018
@@ -4956,8 +4956,7 @@ void Clang::ConstructJob(Compilation ,
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (Args.hasFlag(options::OPT_fomit_frame_pointer,
- options::OPT_fno_omit_frame_pointer, /*default=*/false))
+if (shouldUseFramePointer(Args, Triple))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


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

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

In https://reviews.llvm.org/D52191#1238628, @dblaikie wrote:

> Sure, looks good. Though my other/vague concern is why does this case error 
> about fomit-frame-pointer having no effect, but other things (like using 
> -fomit-frame-pointer on a target that requires frame pointers) that ignore 
> -fomit-frame-pointer don't? Weird. But it probably makes sense somehow.


I think the original issue is that one should not **explicitly** say "omit 
frame pointers" and "instrument for profiling with mcount". It's possible that 
there should be other errors for using "omit frame pointers" on targets that 
require them, but that should be checked independently elsewhere. Do we have 
many (any) of these kinds of targets? I'm going to submit this patch, but am 
happy to add a diagnostic later on for the other case, if you think that is 
worthwhile.


Repository:
  rC Clang

https://reviews.llvm.org/D52191



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

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

[David Blaikie via Phabricator via cfe-commits]

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

Sure, looks good. Though my other/vague concern is why does this case error 
about fomit-frame-pointer having no effect, but other things (like using 
-fomit-frame-pointer on a target that requires frame pointers) that ignore 
-fomit-frame-pointer don't? Weird. But it probably makes sense somehow.


Repository:
  rC Clang

https://reviews.llvm.org/D52191



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

[PATCH] D52084: [clangd] Improve PostingList iterator string representation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In https://reviews.llvm.org/D52084#1238537, @sammccall wrote:

> This change seems fine but...
>
> This representation with the raw DocIDs and position dumped seems mostly 
> useful for debugging buggy iterator implementations, but not really useful 
> for understanding query structure and behavior.


I agree; There is another issue that I was looking into: I think that it might 
be useful to understand the structure of fuzzy find query when using `dexp` 
tool and I thought that it would be great if we could dump the iterator tree 
structure along with the results (which is an extension of 
https://reviews.llvm.org/D52233). For `dexp` usecase, it would be great to dump 
the size and the origin of each piece of iterator tree (e.g. labels), but I 
also think that it might be useful to have "debugging" format so I couldn't 
figure out what's the best approach here.

> I thought we might be replacing this soon. Of course there's no fundamental 
> reason we can't keep both but I'm curious why this is an area of focus :-)

Yes, we are. Initially, this wasn't an area of focus: I just forgot to update 
the comment in `Iterator.h` when moving `PostingList` to a separate file and 
slightly changing the format, but then Eric had a good proposal and I thought 
that it's a good improvement. Also, even though the implementation will be 
different, the dumping format could be the same, so it's not really 
implementation-specific.


https://reviews.llvm.org/D52084



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

Re: [PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Yvan Roux via cfe-commits]

Hi Sam,

It took a very long time to identify it, but this commit broke ARMv7
bots, where this test hangs.  Logs are available here (initial ones
are too old):

http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/3685/steps/ninja%20check%201/logs/stdiohttp://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/3685/steps/ninja%20check%201/logs/stdio

Thanks,
YvanOn Thu, 30 Aug 2018 at 17:15, Sam McCall via Phabricator via
cfe-commits  wrote:
>
> sammccall added inline comments.
>
>
> 
> Comment at: clangd/TUScheduler.cpp:474
> +llvm::unique_function)> 
> Callback) {
> +  if (RunSync)
> +return Callback(getPossiblyStalePreamble());
> 
> ilya-biryukov wrote:
> > It seems we could remove the special-casing of `RunSync` and still get 
> > correct results (the Requests queue is always empty).
> > But feel free to keep it for clarity.
> Right, of course :-)
> Replaced this with an assert before we write to the queue.
>
>
> 
> Comment at: clangd/TUScheduler.h:123
> +  /// Controls whether preamble reads wait for the preamble to be up-to-date.
> +  enum PreambleConsistency {
> +/// The preamble is generated from the current version of the file.
> 
> ilya-biryukov wrote:
> > Maybe use a strongly-typed enum outside the class?
> > `TUScheduler::Stale` will turn into `PreambleConsistency::Stale` at the 
> > call-site. The main upside is that it does not pollute completions inside 
> > the class with enumerators.
> >
> > Just a suggestion, feel free to ignore.
> Yeah, the downside to that is that it *does* pollute the clangd:: namespace. 
> So both are a bit sad.
>
> This header is fairly widely visible (since it's included by clangdserver) 
> and this API is fairly narrowly interesting, so as far as completion goes I 
> think I prefer it being hidden in TUScheduler.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D51438
>
>
>
> ___
> 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] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

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

> In https://reviews.llvm.org/D51789#1238396, @rjmccall wrote:
>
> > That may work for libc++'s purposes, but it's clearly inappropriate as a 
> > compiler rule.  There are good reasons why something with hidden visibility 
> > would need to be explicitly instantiated.
>
>
> I take your word for it, but I can't think of any example. For my education, 
> do you have a specific example in mind?


I mean, most code doesn't use explicit instantiations to begin with, but — the 
general idea would be someone using an explicit instantiation, either for 
compile-time or seperate-compilation reasons, for some type that's entirely 
private to their library.

Here's an example of it from Swift that happened to be easy to find:

  
https://github.com/apple/swift/blob/master/lib/SILOptimizer/ARC/RCStateTransitionVisitors.h

The entire template being instantiated there is private to the SILOptimizer 
library.  Swift doesn't use explicit visibility attributes much, preferring to 
globally assume `-fvisibility=hidden`, but if we used them, there would 
definitely be an attribute on that template.

>> For many programmers, hidden visibility means "this is private to my 
>> library", not "this is actually public to my library, but I'm walking an ABI 
>> tightrope".
> 
> In libc++'s case, the functions we will annotate with 
> `exclude_from_explicit_instantiation` are private to libc++ too (in the sense 
> that we don't want them part of the ABI and they are not exported from the 
> dylib). Those functions were previously marked with `__always_inline__` to 
> make sure they were not part of the ABI.

Yeah, I understand the use case.  That's what I was calling an ABI tightrope.  
The functions you're annotating are still part of libc++'s *logical* interface, 
they're just not exported by the dylib.

> Note that I'm quite happy with `exclude_from_explicit_instantiation` as it 
> solves libc++'s problem -- I'm trying to see whether another solution would 
> serve people better while still solving libc++'s problem. (Appart from 
> explicitly exporting functions, typeinfos and vtables like we've talked about 
> on cfe-dev, which is a superior solution to everything else but is left as a 
> future improvement for the time being).

Understood.

John.


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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

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

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342499: [Modules] Add platform and environment features to 
requires clause (authored by bruno, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51910?vs=165861=165999#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51910

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

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

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

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342499: [Modules] Add platform and environment features to 
requires clause (authored by bruno, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51910?vs=165861=165998#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51910

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

Index: lib/Basic/Module.cpp
===
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -71,6 +71,37 @@
   }
 }
 
+static bool isPlatformEnvironment(const TargetInfo , StringRef Feature) {
+  StringRef Platform = Target.getPlatformName();
+  StringRef Env = Target.getTriple().getEnvironmentName();
+
+  // Attempt to match platform and environment.
+  if (Platform == Feature || Target.getTriple().getOSName() == Feature ||
+  Env == Feature)
+return true;
+
+  auto CmpPlatformEnv = [](StringRef LHS, StringRef RHS) {
+auto Pos = LHS.find("-");
+if (Pos == StringRef::npos)
+  return false;
+SmallString<128> NewLHS = LHS.slice(0, Pos);
+NewLHS += LHS.slice(Pos+1, LHS.size());
+return NewLHS == RHS;
+  };
+
+  SmallString<128> PlatformEnv = Target.getTriple().getOSAndEnvironmentName();
+  // Darwin has different but equivalent variants for simulators, example:
+  //   1. x86_64-apple-ios-simulator
+  //   2. x86_64-apple-iossimulator
+  // where both are valid examples of the same platform+environment but in the
+  // variant (2) the simulator is hardcoded as part of the platform name. Both
+  // forms above should match for "iossimulator" requirement.
+  if (Target.getTriple().isOSDarwin() && PlatformEnv.endswith("simulator"))
+return PlatformEnv == Feature || CmpPlatformEnv(PlatformEnv, Feature);
+
+  return PlatformEnv == Feature;
+}
+
 /// Determine whether a translation unit built using the current
 /// language options has the given feature.
 static bool hasFeature(StringRef Feature, const LangOptions ,
@@ -93,7 +124,8 @@
 .Case("opencl", LangOpts.OpenCL)
 .Case("tls", Target.isTLSSupported())
 .Case("zvector", LangOpts.ZVector)
-.Default(Target.hasFeature(Feature));
+.Default(Target.hasFeature(Feature) ||
+ isPlatformEnvironment(Target, Feature));
   if (!HasFeature)
 HasFeature = std::find(LangOpts.ModuleFeatures.begin(),
LangOpts.ModuleFeatures.end(),
Index: docs/Modules.rst
===
--- docs/Modules.rst
+++ docs/Modules.rst
@@ -411,7 +411,7 @@
   *feature*:
 ``!``:sub:`opt` *identifier*
 
-The requirements clause allows specific modules or submodules to specify that they are only accessible with certain language dialects or on certain platforms. The feature list is a set of identifiers, defined below. If any of the features is not available in a given translation unit, that translation unit shall not import the module. When building a module for use by a compilation, submodules requiring unavailable features are ignored. The optional ``!`` indicates that a feature is incompatible with the module.
+The requirements clause allows specific modules or submodules to specify that they are only accessible with certain language dialects, platforms, environments and target specific features. The feature list is a set of identifiers, defined below. If any of the features is not available in a given translation unit, that translation unit shall not import the module. When building a module for use by a compilation, submodules requiring unavailable features are ignored. The optional ``!`` indicates that a feature is incompatible with the module.
 
 The following features are defined:
 
@@ -466,6 +466,11 @@
 *target feature*
   A specific target feature (e.g., ``sse4``, ``avx``, ``neon``) is available.
 
+*platform/os*
+  A os/platform variant (e.g. ``freebsd``, ``win32``, ``windows``, ``linux``, ``ios``, ``macos``, ``iossimulator``) is available.
+
+*environment*
+  A environment variant (e.g. ``gnu``, ``gnueabi``, ``android``, ``msvc``) is available.
 
 **Example:** The ``std`` module can be extended to also include C++ and C++11 headers using a *requires-declaration*:
 
Index: test/Modules/target-platform-features.m
===
--- test/Modules/target-platform-features.m
+++ test/Modules/target-platform-features.m
@@ -0,0 +1,79 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/InputsA
+
+// RUN: echo "module RequiresMacOS {"   >> %t/InputsA/module.map
+// RUN: echo "  requires macos" >> %t/InputsA/module.map
+// RUN: echo "}">> %t/InputsA/module.map
+// RUN: echo "module RequiresNotiOS 

r342499 - [Modules] Add platform and environment features to requires clause

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Tue Sep 18 10:11:13 2018
New Revision: 342499

URL: http://llvm.org/viewvc/llvm-project?rev=342499=rev
Log:
[Modules] Add platform and environment features to requires clause

Allows module map writers to add build requirements based on
platform/os. This helps when target features and language dialects
aren't enough to conditionalize building a module, among other things,
it allow module maps for different platforms to live in the same file.

rdar://problem/43909745

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

Added:
cfe/trunk/test/Modules/target-platform-features.m
Modified:
cfe/trunk/docs/Modules.rst
cfe/trunk/lib/Basic/Module.cpp

Modified: cfe/trunk/docs/Modules.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Modules.rst?rev=342499=342498=342499=diff
==
--- cfe/trunk/docs/Modules.rst (original)
+++ cfe/trunk/docs/Modules.rst Tue Sep 18 10:11:13 2018
@@ -411,7 +411,7 @@ A *requires-declaration* specifies the r
   *feature*:
 ``!``:sub:`opt` *identifier*
 
-The requirements clause allows specific modules or submodules to specify that 
they are only accessible with certain language dialects or on certain 
platforms. The feature list is a set of identifiers, defined below. If any of 
the features is not available in a given translation unit, that translation 
unit shall not import the module. When building a module for use by a 
compilation, submodules requiring unavailable features are ignored. The 
optional ``!`` indicates that a feature is incompatible with the module.
+The requirements clause allows specific modules or submodules to specify that 
they are only accessible with certain language dialects, platforms, 
environments and target specific features. The feature list is a set of 
identifiers, defined below. If any of the features is not available in a given 
translation unit, that translation unit shall not import the module. When 
building a module for use by a compilation, submodules requiring unavailable 
features are ignored. The optional ``!`` indicates that a feature is 
incompatible with the module.
 
 The following features are defined:
 
@@ -466,6 +466,11 @@ tls
 *target feature*
   A specific target feature (e.g., ``sse4``, ``avx``, ``neon``) is available.
 
+*platform/os*
+  A os/platform variant (e.g. ``freebsd``, ``win32``, ``windows``, ``linux``, 
``ios``, ``macos``, ``iossimulator``) is available.
+
+*environment*
+  A environment variant (e.g. ``gnu``, ``gnueabi``, ``android``, ``msvc``) is 
available.
 
 **Example:** The ``std`` module can be extended to also include C++ and C++11 
headers using a *requires-declaration*:
 

Modified: cfe/trunk/lib/Basic/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=342499=342498=342499=diff
==
--- cfe/trunk/lib/Basic/Module.cpp (original)
+++ cfe/trunk/lib/Basic/Module.cpp Tue Sep 18 10:11:13 2018
@@ -71,6 +71,37 @@ Module::~Module() {
   }
 }
 
+static bool isPlatformEnvironment(const TargetInfo , StringRef Feature) 
{
+  StringRef Platform = Target.getPlatformName();
+  StringRef Env = Target.getTriple().getEnvironmentName();
+
+  // Attempt to match platform and environment.
+  if (Platform == Feature || Target.getTriple().getOSName() == Feature ||
+  Env == Feature)
+return true;
+
+  auto CmpPlatformEnv = [](StringRef LHS, StringRef RHS) {
+auto Pos = LHS.find("-");
+if (Pos == StringRef::npos)
+  return false;
+SmallString<128> NewLHS = LHS.slice(0, Pos);
+NewLHS += LHS.slice(Pos+1, LHS.size());
+return NewLHS == RHS;
+  };
+
+  SmallString<128> PlatformEnv = Target.getTriple().getOSAndEnvironmentName();
+  // Darwin has different but equivalent variants for simulators, example:
+  //   1. x86_64-apple-ios-simulator
+  //   2. x86_64-apple-iossimulator
+  // where both are valid examples of the same platform+environment but in the
+  // variant (2) the simulator is hardcoded as part of the platform name. Both
+  // forms above should match for "iossimulator" requirement.
+  if (Target.getTriple().isOSDarwin() && PlatformEnv.endswith("simulator"))
+return PlatformEnv == Feature || CmpPlatformEnv(PlatformEnv, Feature);
+
+  return PlatformEnv == Feature;
+}
+
 /// Determine whether a translation unit built using the current
 /// language options has the given feature.
 static bool hasFeature(StringRef Feature, const LangOptions ,
@@ -93,7 +124,8 @@ static bool hasFeature(StringRef Feature
 .Case("opencl", LangOpts.OpenCL)
 .Case("tls", Target.isTLSSupported())
 .Case("zvector", LangOpts.ZVector)
-.Default(Target.hasFeature(Feature));
+.Default(Target.hasFeature(Feature) ||
+ isPlatformEnvironment(Target, 

[PATCH] D52084: [clangd] Improve PostingList iterator string representation

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

This change seems fine but...

This representation with the raw DocIDs and position dumped seems mostly useful 
for debugging buggy iterator implementations, but not really useful for 
understanding query structure and behavior.

I thought we might be replacing this soon. Of course there's no fundamental 
reason we can't keep both but I'm curious why this is an area of focus :-)


https://reviews.llvm.org/D52084



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

[PATCH] D50766: Fix false positive unsequenced access and modification warning in array subscript expression.

[Mateusz Janek via Phabricator via cfe-commits]

stryku added a comment.

Friendly ping (:


https://reviews.llvm.org/D50766



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

[PATCH] D51372: FENV_ACCESS support for libm-style constrained intrinsics

[Kevin P. Neal via Phabricator via cfe-commits]

kpn updated this revision to Diff 165995.
kpn added a comment.

Rebase.

Correct obvious error with powi. Fix test and test both C and (partial) C++.

Ping.


https://reviews.llvm.org/D51372

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/Analysis/BodyFarm.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Frontend/Rewrite/RewriteModernObjC.cpp
  lib/Frontend/Rewrite/RewriteObjC.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  test/CodeGen/fenv-access-pragma.c
  test/CodeGen/fenv-access-pragma.cpp
  test/CodeGen/fenv-math-builtins.c

Index: test/CodeGen/fenv-math-builtins.c
===
--- test/CodeGen/fenv-math-builtins.c
+++ test/CodeGen/fenv-math-builtins.c
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm  %s | FileCheck %s 
+
+// Test codegen of math builtins when using FENV_ACCESS ON.
+// Derived from math-builtins.c and keeps calls in the same order.
+#pragma STDC FENV_ACCESS ON
+
+void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
+  __builtin_pow(f,f);__builtin_powf(f,f);   __builtin_powl(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.pow.f64(double, double, metadata, metadata) [[MATH_INTRINSIC:#[0-9]+]]
+// CHECK: declare float @llvm.experimental.constrained.pow.f32(float, float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.pow.f80(x86_fp80, x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_powi(f,f);__builtin_powif(f,f);   __builtin_powil(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.powi.f64(double, i32, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.powi.f32(float, i32, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.powi.f80(x86_fp80, i32, metadata, metadata) [[MATH_INTRINSIC]]
+
+  /* math */
+  __builtin_cos(f);__builtin_cosf(f);   __builtin_cosl(f); 
+// CHECK: declare double @llvm.experimental.constrained.cos.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.cos.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.cos.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_exp(f);__builtin_expf(f);   __builtin_expl(f);
+// CHECK: declare double @llvm.experimental.constrained.exp.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.exp.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.exp.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_exp2(f);   __builtin_exp2f(f);  __builtin_exp2l(f); 
+
+// CHECK: declare double @llvm.experimental.constrained.exp2.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.exp2.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.exp2.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_fma(f,f,f);__builtin_fmaf(f,f,f);   __builtin_fmal(f,f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.fma.f64(double, double, double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.fma.f32(float, float, float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.fma.f80(x86_fp80, x86_fp80, x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_log(f);__builtin_logf(f);   __builtin_logl(f);
+
+// CHECK: declare double @llvm.experimental.constrained.log.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.log.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.log.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_log10(f);  __builtin_log10f(f); __builtin_log10l(f);
+
+// CHECK: declare double @llvm.experimental.constrained.log10.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.log10.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.log10.f80(x86_fp80, metadata, metadata) [[MATH_INTRINSIC]]
+
+  __builtin_nearbyint(f);  __builtin_nearbyintf(f); __builtin_nearbyintl(f);
+
+// CHECK: declare double @llvm.experimental.constrained.nearbyint.f64(double, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: declare float @llvm.experimental.constrained.nearbyint.f32(float, metadata, metadata) [[MATH_INTRINSIC]]
+// CHECK: 

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

[Jano Simas via Phabricator via cfe-commits]

janosimas added a comment.

To use in a git pre-commit I wanted to use the flags:
-warnings-as-errors=*
-header-filter=.*

I wanted that the script returned an error value for my selected checks, even 
if they are just warnings.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49864



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

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

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

In https://reviews.llvm.org/D52058#1237868, @JDevlieghere wrote:

> Generally this looks good, but I'd like for the other to have a look first 
> (at this and the other patch) before accepting.


Sounds good. Thanks for your comments!




Comment at: lib/CodeGen/CGDebugInfo.cpp:1783
+  if (auto *TS = dyn_cast(VL)) {
+if (TS->getSpecializedTemplateOrPartial()
+.is()) {

JDevlieghere wrote:
> JDevlieghere wrote:
> > Might be nice to add a comment here saying what you're doing in this block 
> > and below. Looks like the top one is for partial specialization and the 
> > bottom one for the general case?
> I also suggest to extract `TS->getSpecializedTemplateOrPartial()` into a 
> variable to make this a little less dense.
Hmm... Yeah. I'll take a look at clarifying this section.


Repository:
  rC Clang

https://reviews.llvm.org/D52058



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

[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Looks like a good start! I think getting the pointers right will be most 
difficult, because of the multiple levels of indirection they allow. Do you 
think it would be possible to the analysis for `>const?< int ***`-cases? 
(recursively checking through the pointer levels)




Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:198
 const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // `Exp` can be *directly* mutated if the type of `Exp` is not const.
+  // Bail out early otherwise.

Just to be sure that i understand:
the changes here are more performance optimizations then directly related to 
detect pointee mutations?



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:440
+  return nullptr;
+  } else if (const auto *RT = Exp->getType()->getAs()) {
+if (const CXXRecordDecl *RecordDecl = RT->getAsCXXRecordDecl()) {

no `else` after return. this makes the short circuit logic clearer



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:442
+if (const CXXRecordDecl *RecordDecl = RT->getAsCXXRecordDecl()) {
+  const bool ExpIsConst = Exp->getType().isConstant(Context);
+  if (llvm::any_of(

values are not marked as const by the llvm code guideline



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:459
+  }
+  const bool ExpIsPointer = Exp->getType()->getAs();
+

implicit conversion from pointer to bool? Please make that better visible and 
please dont use const on values. 
Not sure for the matchers, I have seen both, but they should be treated as 
values as well i think.



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:463
+  // A member function is assumed to be non-const when it is unresolved.
+  // We don't handle pointer-like type here as non-const member function of
+  // pointee can't be directly invoked without dereferencing the pointer-like

Please make that sentence clearer, i could not understand it (only interpret :D)



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:481
+  const auto AsArg =
+  anyOf(callExpr(hasAnyArgument(equalsNode(Exp))),
+cxxConstructExpr(hasAnyArgument(equalsNode(Exp))),

shouldn't be the constness of the argument considered here?



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:488
+
+  // Returned.
+  const auto AsReturnValue = returnStmt(hasReturnValue(equalsNode(Exp)));

Either remove the comment or make it a full sentence please. I think the 
variable naming is clear enough to elide



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:500
+const Stmt *ExprMutationAnalyzer::findPointeeCastMutation(const Expr *Exp) {
+  // Only handling LValueToRValue for now for easier unit testing during
+  // implementation.

Please add a todo to ensure the missing casts are not forgotten



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:43
   const auto *const E = selectFirst("expr", Results);
+  assert(E);
   return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);

maybe even `assert(E && S)`?



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:67
+if (const auto *DRE = dyn_cast(E)) {
+  if (DRE->getNameInfo().getAsString()[0] == 'p')
+Finder = PointeeMutationFinder;

That doesn't look like a super idea. It is super hidden that variable 'p*' will 
be analyzed for pointee mutation and others aren't. Especially in 12 months and 
when someone else wants to change something, this will be the source of 
headaches.

Don't you think there could be a better way of doing this switch?



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = tooling::buildASTFromCode(

I feel that there a multiple tests missing:

- multiple levels of pointers `int ***`, `int * const *`
- pointers to references `int &*`
- references to pointers `int *&`
- ensure that having a const pointer does no influence the pointee analysis 
`int * const p =  *p = 42;`
- a class with `operator*` + `operator->` const/non-const and the analysis for 
pointers to that class
- pointer returned from a function
- non-const reference returned 
```
int& foo(int *p) {
  return *p;
}
```




Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:175
+  Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()"));
 

Could you please add the `EXPECT_FALSE(isMutated()) && 
EXPECT_TRUE(isPointeeMutated()`?
Maybe a short helper for that like `isOnlyPointeeMutated()` would be nice.

Here and the other cases as well

[PATCH] D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Analysis/ThreadSafetyCommon.cpp:362
+  til::Project *P = new (Arena) til::Project(E, D);
+  if (hasCppPointerType(BE))
+P->setArrow(true);

aaron.ballman wrote:
> I feel like this will always return false. However, I wonder if 
> `IVRE->getBase()->isArrow()` is more appropriate here?
The base of an `ObjCIvarRefExpr` will never directly be a C pointer type.  I 
don't know whether this data structure looks through casts, but it's certainly 
*possible* to cast arbitrarily weird C stuff to ObjC pointer type.  On the 
other hand, by and large nobody actually ever does that, so I wouldn't make a 
special case for it here.

`ObjCIvarRefExpr` just has an `isArrow()` method directly if this is just 
supposed to be capturing the difference between `.` and `->`.  But then, so 
does `MemberExpr`, and in that case you're doing this odd analysis of the base 
instead of trusting `isArrow()`, so I don't know what to tell you to do.

Overall it is appropriate to treat an ObjC ivar reference as a perfectly 
ordinary projection.  The only thing that's special about it is that the actual 
offset isn't necessarily statically known, but ivars still behave as if they're 
all independently declared, so I wouldn't worry about it.


Repository:
  rC Clang

https://reviews.llvm.org/D52200



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

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

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D51789#1238396, @rjmccall wrote:

> That may work for libc++'s purposes, but it's clearly inappropriate as a 
> compiler rule.  There are good reasons why something with hidden visibility 
> would need to be explicitly instantiated.


I take your word for it, but I can't think of any example. For my education, do 
you have a specific example in mind?

> For many programmers, hidden visibility means "this is private to my 
> library", not "this is actually public to my library, but I'm walking an ABI 
> tightrope".

In libc++'s case, the functions we will annotate with 
`exclude_from_explicit_instantiation` are private to libc++ too (in the sense 
that we don't want them part of the ABI and they are not exported from the 
dylib). Those functions were previously marked with `__always_inline__` to make 
sure they were not part of the ABI.

Note that I'm quite happy with `exclude_from_explicit_instantiation` as it 
solves libc++'s problem -- I'm trying to see whether another solution would 
serve people better while still solving libc++'s problem. (Appart from 
explicitly exporting functions, typeinfos and vtables like we've talked about 
on cfe-dev, which is a superior solution to everything else but is left as a 
future improvement for the time being).


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

This is intended, IIUC. The syntax of the clang-tidy-diff.py mirrors the syntax 
of clang-tidy itself, and the `--` option is used in the same way as in 
clang-tidy - to denote the start of compiler arguments (and switch to the fixed 
compilation database). Do you have a use case that would require passing 
clang-tidy options beyond the list already supported by clang-tidy-diff.py?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49864



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

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

[Mailing List "cfe-commits" via Phabricator via cfe-commits]

cfe-commits added a comment.



- F7238787: msg-7222-125.txt 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078



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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

That may work for libc++'s purposes, but it's clearly inappropriate as a 
compiler rule.  There are good reasons why something with hidden visibility 
would need to be explicitly instantiated.  For many programmers, hidden 
visibility means "this is private to my library", not "this is actually public 
to my library, but I'm walking an ABI tightrope".


Repository:
  rC Clang

https://reviews.llvm.org/D51789



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

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

[Sam McCall via cfe-commits]

On Tue, Sep 18, 2018, 16:28 Eric Liu via Phabricator <
revi...@reviews.llvm.org> wrote:

> ioeric added a comment.
>
> In https://reviews.llvm.org/D52078#1238301, @sammccall wrote:
>
> > Something not listed in cons: because macros aren't namespaced and we
> don't have lots of signals, they can be really spammy.
> >  Potentially, offering macros that aren't in the TU could be a loss even
> if it's a win for other types of signals.
>
>
> Aren't they already spammy from Sema? Sema can provide thousands of macros
> in the TU.
>
Indeed, but Sema will at least implicitly restrict to the transitive
headers!

Agree with everything you say, looking forward to this change!

We penalize quality of macro symbols in the global index. Maybe we can do
> the same thing for dynamic index?
>
> > We could always e.g. postfilter index macro results using the include
> structure of the preamble, so no concern for this patch, just something to
> think about for the followup.
>
> Sounds good.
>
> > We also need to make sure that we're not indexing/serving header guards
> as code completions (e.g. if SemaCodeComplete is currently taking care of
> this)
>
> These symbols are already filtered out in `SymbolCollector`.
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D52078
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

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

rsmith added a comment.

We don't want to allocate storage for the lambda fields for non-lambda classes, 
which is why we use distinct base classes. Is the problem you're trying to 
solve here that we fake a definition in AST deserialization before we know 
whether the class is a lambda? If so, that seems solvable by moving the 
IsLambda flag out of DefinitionData into CXXRecordData (perhaps as a distinct 
TagTypeKind?).


Repository:
  rC Clang

https://reviews.llvm.org/D50948



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

r342484 - [index] Enhance indexing for module references

[Argyrios Kyrtzidis via cfe-commits]

Author: akirtzidis
Date: Tue Sep 18 08:02:56 2018
New Revision: 342484

URL: http://llvm.org/viewvc/llvm-project?rev=342484=rev
Log:
[index] Enhance indexing for module references

* Create a USR for the occurrences of the 'module' symbol kind
* Record module references for each identifier in an import declaration

Added:
cfe/trunk/test/Index/Core/Inputs/module/SubModA.h
cfe/trunk/test/Index/Core/Inputs/module/SubSubModA.h
Modified:
cfe/trunk/include/clang/Index/IndexDataConsumer.h
cfe/trunk/include/clang/Index/USRGeneration.h
cfe/trunk/lib/Index/IndexingAction.cpp
cfe/trunk/lib/Index/IndexingContext.cpp
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/test/Index/Core/Inputs/module/module.modulemap
cfe/trunk/test/Index/Core/index-with-module.m
cfe/trunk/tools/c-index-test/core_main.cpp
cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp
cfe/trunk/tools/libclang/CXIndexDataConsumer.h

Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexDataConsumer.h?rev=342484=342483=342484=diff
==
--- cfe/trunk/include/clang/Index/IndexDataConsumer.h (original)
+++ cfe/trunk/include/clang/Index/IndexDataConsumer.h Tue Sep 18 08:02:56 2018
@@ -50,7 +50,12 @@ public:
 SourceLocation Loc);
 
   /// \returns true to continue indexing, or false to abort.
+  ///
+  /// This will be called for each module reference in an import decl.
+  /// For "@import MyMod.SubMod", there will be a call for 'MyMod' with the
+  /// 'reference' role, and a call for 'SubMod' with the 'declaration' role.
   virtual bool handleModuleOccurence(const ImportDecl *ImportD,
+ const Module *Mod,
  SymbolRoleSet Roles, SourceLocation Loc);
 
   virtual void finish() {}

Modified: cfe/trunk/include/clang/Index/USRGeneration.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/USRGeneration.h?rev=342484=342483=342484=diff
==
--- cfe/trunk/include/clang/Index/USRGeneration.h (original)
+++ cfe/trunk/include/clang/Index/USRGeneration.h Tue Sep 18 08:02:56 2018
@@ -16,6 +16,7 @@
 namespace clang {
 class Decl;
 class MacroDefinitionRecord;
+class Module;
 class SourceLocation;
 class SourceManager;
 
@@ -70,6 +71,22 @@ bool generateUSRForMacro(const MacroDefi
 bool generateUSRForMacro(StringRef MacroName, SourceLocation Loc,
  const SourceManager , SmallVectorImpl );
 
+/// Generate a USR for a module, including the USR prefix.
+/// \returns true on error, false on success.
+bool generateFullUSRForModule(const Module *Mod, raw_ostream );
+
+/// Generate a USR for a top-level module name, including the USR prefix.
+/// \returns true on error, false on success.
+bool generateFullUSRForTopLevelModuleName(StringRef ModName, raw_ostream );
+
+/// Generate a USR fragment for a module.
+/// \returns true on error, false on success.
+bool generateUSRFragmentForModule(const Module *Mod, raw_ostream );
+
+/// Generate a USR fragment for a module name.
+/// \returns true on error, false on success.
+bool generateUSRFragmentForModuleName(StringRef ModName, raw_ostream );
+
 } // namespace index
 } // namespace clang
 

Modified: cfe/trunk/lib/Index/IndexingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingAction.cpp?rev=342484=342483=342484=diff
==
--- cfe/trunk/lib/Index/IndexingAction.cpp (original)
+++ cfe/trunk/lib/Index/IndexingAction.cpp Tue Sep 18 08:02:56 2018
@@ -37,6 +37,7 @@ bool IndexDataConsumer::handleMacroOccur
 }
 
 bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
+  const Module *Mod,
   SymbolRoleSet Roles,
   SourceLocation Loc) {
   return true;

Modified: cfe/trunk/lib/Index/IndexingContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=342484=342483=342484=diff
==
--- cfe/trunk/lib/Index/IndexingContext.cpp (original)
+++ cfe/trunk/lib/Index/IndexingContext.cpp Tue Sep 18 08:02:56 2018
@@ -80,11 +80,27 @@ bool IndexingContext::handleReference(co
   RefE, RefD, DC);
 }
 
+static void reportModuleReferences(const Module *Mod,
+   ArrayRef IdLocs,
+   const ImportDecl *ImportD,
+   IndexDataConsumer ) {
+  if (!Mod)
+return;
+  reportModuleReferences(Mod->Parent, IdLocs.drop_back(), ImportD,
+ 

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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D52078#1238301, @sammccall wrote:

> Something not listed in cons: because macros aren't namespaced and we don't 
> have lots of signals, they can be really spammy. 
>  Potentially, offering macros that aren't in the TU could be a loss even if 
> it's a win for other types of signals.


Aren't they already spammy from Sema? Sema can provide thousands of macros in 
the TU.

We penalize quality of macro symbols in the global index. Maybe we can do the 
same thing for dynamic index?

> We could always e.g. postfilter index macro results using the include 
> structure of the preamble, so no concern for this patch, just something to 
> think about for the followup.

Sounds good.

> We also need to make sure that we're not indexing/serving header guards as 
> code completions (e.g. if SemaCodeComplete is currently taking care of this)

These symbols are already filtered out in `SymbolCollector`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078



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

[PATCH] D52084: [clangd] Improve PostingList iterator string representation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165973.
kbobyrev marked an inline comment as done.

https://reviews.llvm.org/D52084

Files:
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp


Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -262,13 +262,15 @@
   const PostingList L4({0, 1, 5});
   const PostingList L5({});
 
-  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4]");
+  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
 
   auto Nested =
   createAnd(createAnd(L1.iterator(), L2.iterator()),
 createOr(L3.iterator(), L4.iterator(), L5.iterator()));
 
-  EXPECT_EQ(llvm::to_string(*Nested), "(& (| [5] [1] [END]) (& [1] [1]))");
+  EXPECT_EQ(
+  llvm::to_string(*Nested),
+  "(& (| [... 5] [... 1 ...] [END]) (& [1 ...] [1 ...]))");
 }
 
 TEST(DexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/PostingList.cpp
===
--- clang-tools-extra/clangd/index/dex/PostingList.cpp
+++ clang-tools-extra/clangd/index/dex/PostingList.cpp
@@ -61,10 +61,14 @@
 private:
   llvm::raw_ostream (llvm::raw_ostream ) const override {
 OS << '[';
+if (Index != std::begin(Documents))
+  OS << "... ";
 if (Index != std::end(Documents))
   OS << *Index;
 else
   OS << "END";
+if (Index != std::end(Documents) && Index != std::end(Documents) - 1)
+  OS << " ...";
 OS << ']';
 return OS;
   }
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -93,9 +93,8 @@
   ///
   /// Where Type is the iterator type representation: "&" for And, "|" for Or,
   /// ChildN is N-th iterator child. Raw iterators over PostingList are
-  /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
-  /// PostingList entry and the element which is pointed to by the PostingList
-  /// iterator is enclosed in {} braces.
+  /// represented as "[... ID ...]" where ID is the element under iterator
+  /// cursor.
   friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
 return Iterator.dump(OS);


Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -262,13 +262,15 @@
   const PostingList L4({0, 1, 5});
   const PostingList L5({});
 
-  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4]");
+  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
 
   auto Nested =
   createAnd(createAnd(L1.iterator(), L2.iterator()),
 createOr(L3.iterator(), L4.iterator(), L5.iterator()));
 
-  EXPECT_EQ(llvm::to_string(*Nested), "(& (| [5] [1] [END]) (& [1] [1]))");
+  EXPECT_EQ(
+  llvm::to_string(*Nested),
+  "(& (| [... 5] [... 1 ...] [END]) (& [1 ...] [1 ...]))");
 }
 
 TEST(DexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/PostingList.cpp
===
--- clang-tools-extra/clangd/index/dex/PostingList.cpp
+++ clang-tools-extra/clangd/index/dex/PostingList.cpp
@@ -61,10 +61,14 @@
 private:
   llvm::raw_ostream (llvm::raw_ostream ) const override {
 OS << '[';
+if (Index != std::begin(Documents))
+  OS << "... ";
 if (Index != std::end(Documents))
   OS << *Index;
 else
   OS << "END";
+if (Index != std::end(Documents) && Index != std::end(Documents) - 1)
+  OS << " ...";
 OS << ']';
 return OS;
   }
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -93,9 +93,8 @@
   ///
   /// Where Type is the iterator type representation: "&" for And, "|" for Or,
   /// ChildN is N-th iterator child. Raw iterators over PostingList are
-  /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
-  /// PostingList entry and the element which is pointed to by the PostingList
-  /// iterator is enclosed in {} braces.
+  /// represented as "[... ID ...]" where ID is the element under iterator
+  /// cursor.
   friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
 return Iterator.dump(OS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52233: [dexp] Allow users to dump JSON representations of fuzzy find requests

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous.

This might be useful for benchmark construction.


https://reviews.llvm.org/D52233

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -113,6 +113,11 @@
   cl::init(10),
   cl::desc("Max results to display"),
   };
+  cl::opt ShowJSONRequest{
+  "show-json",
+  cl::desc("Print Fuzzy Find Request in JSON format"),
+  cl::init(false),
+  };
 
   void run() override {
 FuzzyFindRequest Request;
@@ -123,6 +128,8 @@
   StringRef(this->Scopes).split(Scopes, ',');
   Request.Scopes = {Scopes.begin(), Scopes.end()};
 }
+if (ShowJSONRequest)
+  llvm::outs() << llvm::formatv("Request:\n{0}\n\n", toJSON(Request));
 // FIXME(kbobyrev): Print symbol final scores to see the distribution.
 static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
 llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",


Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -113,6 +113,11 @@
   cl::init(10),
   cl::desc("Max results to display"),
   };
+  cl::opt ShowJSONRequest{
+  "show-json",
+  cl::desc("Print Fuzzy Find Request in JSON format"),
+  cl::init(false),
+  };
 
   void run() override {
 FuzzyFindRequest Request;
@@ -123,6 +128,8 @@
   StringRef(this->Scopes).split(Scopes, ',');
   Request.Scopes = {Scopes.begin(), Scopes.end()};
 }
+if (ShowJSONRequest)
+  llvm::outs() << llvm::formatv("Request:\n{0}\n\n", toJSON(Request));
 // FIXME(kbobyrev): Print symbol final scores to see the distribution.
 static const auto OutputFormat = "{0,-4} | {1,-40} | {2,-25}\n";
 llvm::outs() << llvm::formatv(OutputFormat, "Rank", "Symbol ID",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Something not listed in cons: because macros aren't namespaced and we don't 
have lots of signals, they can be really spammy. 
Potentially, offering macros that aren't in the TU could be a loss even if it's 
a win for other types of signals.

We could always e.g. postfilter index macro results using the include structure 
of the preamble, so no concern for this patch, just something to think about 
for the followup.

We also need to make sure that we're not indexing/serving header guards as code 
completions (e.g. if SemaCodeComplete is currently taking care of this)




Comment at: clangd/index/FileIndex.cpp:48
 CollectorOpts.RefFilter = RefKind::All;
+  }else {
+IndexOpts.IndexMacrosInPreprocessor = true;

nit: please clang-format


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078



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

[PATCH] D52079: [Sema] Do not load macros from preamble when LoadExternal is false.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 165969.
ioeric added a comment.

- added lit test


Repository:
  rC Clang

https://reviews.llvm.org/D52079

Files:
  include/clang/Sema/CodeCompleteOptions.h
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-pch-skip.cpp

Index: test/Index/complete-pch-skip.cpp
===
--- test/Index/complete-pch-skip.cpp
+++ test/Index/complete-pch-skip.cpp
@@ -4,19 +4,26 @@
 
 int main() { return ns:: }
 int main2() { return ns::foo(). }
+int main3() { PREAMBLE_ }
 
-// RUN: echo "namespace ns { struct foo { int baz }; }" > %t.h
+// RUN: printf "namespace ns { struct foo { int baz }; }\n#define PREAMBLE_MAC" > %t.h
 // RUN: c-index-test -write-pch %t.h.pch -x c++-header %t.h
 //
 // RUN: c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=WITH-PCH %s
 // WITH-PCH: {TypedText bar}
 // WITH-PCH: {TypedText foo}
 
+// RUN: c-index-test -code-completion-at=%s:7:24 -include %t.h %s | FileCheck -check-prefix=WITH-PCH-MACRO %s
+// WITH-PCH-MACRO: {TypedText PREAMBLE_MAC}
+
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH %s
 // SKIP-PCH-NOT: foo
 // SKIP-PCH: {TypedText bar}
 // SKIP-PCH-NOT: foo
 
+// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:7:24 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH-MACRO %s
+// SKIP-PCH-MACRO-NOT: {TypedText PREAMBLE_MAC}
+
 // Verify that with *no* preamble (no -include flag) we still get local results.
 // SkipPreamble used to break this, by making lookup *too* lazy.
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 %s | FileCheck -check-prefix=NO-PCH %s
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3304,14 +3304,14 @@
 }
 
 static void AddMacroResults(Preprocessor , ResultBuilder ,
-bool IncludeUndefined,
+bool LoadExternal, bool IncludeUndefined,
 bool TargetTypeIsPointer = false) {
   typedef CodeCompletionResult Result;
 
   Results.EnterNewScope();
 
-  for (Preprocessor::macro_iterator M = PP.macro_begin(),
- MEnd = PP.macro_end();
+  for (Preprocessor::macro_iterator M = PP.macro_begin(LoadExternal),
+MEnd = PP.macro_end(LoadExternal);
M != MEnd; ++M) {
 auto MD = PP.getMacroDefinition(M->first);
 if (IncludeUndefined || MD) {
@@ -3327,7 +3327,6 @@
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions ,
@@ -3611,7 +3610,7 @@
   }
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -3749,7 +3748,8 @@
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false,
+PreferredTypeIsPointer);
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
 }
@@ -4372,7 +4372,7 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -4688,7 +4688,7 @@
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -5722,7 +5722,7 @@
  CodeCompleter->loadExternal());
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -5951,10 +5951,9 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
  

[PATCH] D52084: [clangd] NFC: Update documentation of Iterator's dump format

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:66
+  OS << "... ";
 if (Index != std::end(Documents))
+  OS << *Index << " ...";

nit: should we drop the trailing `...` if Index is the last element?


https://reviews.llvm.org/D52084



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

[PATCH] D51340: [WIP] Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta updated this revision to Diff 165966.
takuto.ikuta edited the summary of this revision.
takuto.ikuta added a comment.

Current implementation cannot build chrome when pch is enabled.
undefined symbol error happens during linking blink_modules.dll


https://reviews.llvm.org/D51340

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/CLCompatOptions.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
  clang/test/CodeGenCXX/hidden-dllimport.cpp

Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
 
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
 
 // CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
 
Index: clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
@@ -0,0 +1,161 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+int ImportedFunctionUser() {
+  return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;
+
+  // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+  // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+  void InclassDefFunc() {}
+
+  int f();
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+  }
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+  __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+  // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+  void OutclassDefFunc();
+
+
+  // Copy assignment operator.
+  // DEFAULT-DAG: define weak_odr dso_local dllexport dereferenceable(1) %class.NoTemplateExportedClass* @"??4NoTemplateExportedClass@@QEAAAEAV0@AEBV0@@Z"
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void 

[PATCH] D52084: [clangd] NFC: Update documentation of Iterator's dump format

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165964.
kbobyrev added a comment.

@ioeric does this format look better?


https://reviews.llvm.org/D52084

Files:
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp


Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -262,13 +262,15 @@
   const PostingList L4({0, 1, 5});
   const PostingList L5({});
 
-  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4]");
+  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
 
   auto Nested =
   createAnd(createAnd(L1.iterator(), L2.iterator()),
 createOr(L3.iterator(), L4.iterator(), L5.iterator()));
 
-  EXPECT_EQ(llvm::to_string(*Nested), "(& (| [5] [1] [END]) (& [1] [1]))");
+  EXPECT_EQ(
+  llvm::to_string(*Nested),
+  "(& (| [... 5 ...] [... 1 ...] [END]) (& [1 ...] [1 ...]))");
 }
 
 TEST(DexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/PostingList.cpp
===
--- clang-tools-extra/clangd/index/dex/PostingList.cpp
+++ clang-tools-extra/clangd/index/dex/PostingList.cpp
@@ -61,8 +61,10 @@
 private:
   llvm::raw_ostream (llvm::raw_ostream ) const override {
 OS << '[';
+if (Index != std::begin(Documents))
+  OS << "... ";
 if (Index != std::end(Documents))
-  OS << *Index;
+  OS << *Index << " ...";
 else
   OS << "END";
 OS << ']';
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -93,9 +93,8 @@
   ///
   /// Where Type is the iterator type representation: "&" for And, "|" for Or,
   /// ChildN is N-th iterator child. Raw iterators over PostingList are
-  /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
-  /// PostingList entry and the element which is pointed to by the PostingList
-  /// iterator is enclosed in {} braces.
+  /// represented as "[... ID ...]" where ID is the element under iterator
+  /// cursor.
   friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
 return Iterator.dump(OS);


Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -262,13 +262,15 @@
   const PostingList L4({0, 1, 5});
   const PostingList L5({});
 
-  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4]");
+  EXPECT_EQ(llvm::to_string(*(L0.iterator())), "[4 ...]");
 
   auto Nested =
   createAnd(createAnd(L1.iterator(), L2.iterator()),
 createOr(L3.iterator(), L4.iterator(), L5.iterator()));
 
-  EXPECT_EQ(llvm::to_string(*Nested), "(& (| [5] [1] [END]) (& [1] [1]))");
+  EXPECT_EQ(
+  llvm::to_string(*Nested),
+  "(& (| [... 5 ...] [... 1 ...] [END]) (& [1 ...] [1 ...]))");
 }
 
 TEST(DexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/PostingList.cpp
===
--- clang-tools-extra/clangd/index/dex/PostingList.cpp
+++ clang-tools-extra/clangd/index/dex/PostingList.cpp
@@ -61,8 +61,10 @@
 private:
   llvm::raw_ostream (llvm::raw_ostream ) const override {
 OS << '[';
+if (Index != std::begin(Documents))
+  OS << "... ";
 if (Index != std::end(Documents))
-  OS << *Index;
+  OS << *Index << " ...";
 else
   OS << "END";
 OS << ']';
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -93,9 +93,8 @@
   ///
   /// Where Type is the iterator type representation: "&" for And, "|" for Or,
   /// ChildN is N-th iterator child. Raw iterators over PostingList are
-  /// represented as "[ID1, ID2, ..., {IDN}, ... END]" where IDN is N-th
-  /// PostingList entry and the element which is pointed to by the PostingList
-  /// iterator is enclosed in {} braces.
+  /// represented as "[... ID ...]" where ID is the element under iterator
+  /// cursor.
   friend llvm::raw_ostream <<(llvm::raw_ostream ,
const Iterator ) {
 return Iterator.dump(OS);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52047: [clangd] Add a "benchmark" for tracking memory

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165962.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Rebase on top of master, move logging to symbol index `build()` caller side.


https://reviews.llvm.org/D52047

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/SymbolYAML.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp

Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
   for (const auto  : TempInvertedIndex)
 InvertedIndex.insert({TokenToPostingList.first,
   PostingList(move(TokenToPostingList.second))});
-
-  vlog("Built Dex with estimated memory usage {0} bytes.",
-   estimateMemoryUsage());
 }
 
 /// Constructs iterators over tokens extracted from the query and exhausts it
@@ -239,7 +236,7 @@
   Bytes += LookupTable.getMemorySize();
   Bytes += InvertedIndex.getMemorySize();
   for (const auto  : InvertedIndex)
-Bytes += P.second.bytes();
+Bytes += P.first.Data.size() + P.second.bytes() * sizeof(DocID);
   return Bytes + BackingDataSize;
 }
 
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
 
 #include "SymbolYAML.h"
 #include "Index.h"
+#include "Logger.h"
 #include "Serialization.h"
 #include "Trace.h"
 #include "dex/Dex.h"
@@ -225,8 +226,12 @@
   if (!Slab)
 return nullptr;
   trace::Span Tracer("BuildIndex");
-  return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+  auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+  : MemIndex::build(std::move(*Slab), RefSlab());
+  vlog("Loaded {0} from {1} with estimated memory usage {2}",
+   UseDex ? "Dex" : "MemIndex", SymbolFilename,
+   Index->estimateMemoryUsage());
+  return Index;
 }
 
 } // namespace clangd
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -66,23 +66,44 @@
   return Requests;
 }
 
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overhead?
+static void MemSize(benchmark::State ) {
+  const auto Mem = buildMem();
+  for (auto _ : State)
+// Divide size of Mem by 1000 so that it will be correctly displayed in the
+// benchmark report (possible options for time units are ms, ns and us).
+State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() /
+   1000);
+}
+BENCHMARK(MemSize)->UseManualTime()->Unit(benchmark::kMillisecond);
+
+static void DexSize(benchmark::State ) {
+  const auto Dex = buildDex();
+  for (auto _ : State)
+State.SetIterationTime(Dex->estimateMemoryUsage() / 1000);
+}
+BENCHMARK(DexSize)->UseManualTime()->Unit(benchmark::kMillisecond);
+
 static void MemQueries(benchmark::State ) {
   const auto Mem = buildMem();
   const auto Requests = extractQueriesFromLogs();
   for (auto _ : State)
 for (const auto  : Requests)
   Mem->fuzzyFind(Request, [](const Symbol ) {});
 }
-BENCHMARK(MemQueries);
+BENCHMARK(MemQueries)->Unit(benchmark::kMillisecond);
 
 static void DexQueries(benchmark::State ) {
   const auto Dex = buildDex();
   const auto Requests = extractQueriesFromLogs();
   for (auto _ : State)
 for (const auto  : Requests)
   Dex->fuzzyFind(Request, [](const Symbol ) {});
 }
-BENCHMARK(DexQueries);
+BENCHMARK(DexQueries)->Unit(benchmark::kMillisecond);
 
 } // namespace
 } // namespace clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 165959.
ioeric added a comment.

- merge with origin/master


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52078

Files:
  clangd/index/FileIndex.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp

Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -15,6 +15,7 @@
 #include "index/FileIndex.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "gtest/gtest.h"
@@ -330,6 +331,21 @@
  FileURI("unittest:///test2.cc"))}));
 }
 
+TEST(FileIndexTest, CollectMacros) {
+  FileIndex M;
+  update(M, "f", "#define CLANGD 1");
+
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  bool SeenSymbol = false;
+  M.index().fuzzyFind(Req, [&](const Symbol ) {
+EXPECT_EQ(Sym.Name, "CLANGD");
+EXPECT_EQ(Sym.SymInfo.Kind, index::SymbolKind::Macro);
+SeenSymbol = true;
+  });
+  EXPECT_TRUE(SeenSymbol);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -223,12 +223,13 @@
 void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
 
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -276,11 +277,12 @@
 void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
-
   int global_var;
   int global_func();
 
+  // Make sure this is not in preamble.
+  #define MACRO X
+
   struct GlobalClass {};
 
   struct ClassWithMembers {
@@ -430,10 +432,11 @@
 TEST(CompletionTest, Kinds) {
   auto Results = completions(
   R"cpp(
-  #define MACRO X
   int variable;
   struct Struct {};
   int function();
+  // make sure MACRO is not included in preamble.
+  #define MACRO 10
   int X = ^
   )cpp",
   {func("indexFunction"), var("indexVariable"), cls("indexClass")});
@@ -1921,6 +1924,21 @@
   UnorderedElementsAre(Named("Clangd_Macro_Test")));
 }
 
+TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+  auto Results = completions(
+  R"cpp(#define CLANGD_PREAMBLE x
+
+  int x = 0;
+  #define CLANGD_MAIN x
+  void f() { CLANGD_^ }
+  )cpp",
+  {func("CLANGD_INDEX")});
+  // Index is overriden in code completion options, so the preamble symbol is
+  // not seen.
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
+Named("CLANGD_INDEX")));
+}
+
 TEST(CompletionTest, DeprecatedResults) {
   std::string Body = R"cpp(
 void TestClangd();
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -14,6 +14,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include 
 
@@ -41,10 +42,13 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
-
-  if (IsIndexMainAST)
+  if (IsIndexMainAST) {
 // We only collect refs when indexing main AST.
 CollectorOpts.RefFilter = RefKind::All;
+  }else {
+IndexOpts.IndexMacrosInPreprocessor = true;
+CollectorOpts.CollectMacro = true;
+  }
 
   SymbolCollector Collector(std::move(CollectorOpts));
   Collector.setPreprocessor(PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52222: [clangd] Merge ClangdServer::DynamicIndex into FileIndex. NFC.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/FileIndex.h:77
+  /// Update symbols from main file \p Path with symbols in \p TopLevelDecls.
+  void updateMain(PathRef Path, ASTContext ,
+  std::shared_ptr PP,

sammccall wrote:
> sammccall wrote:
> > can't this just take PathRef + ParsedAST?
> > (same data, but clearer semantics)
> You're still taking the top-level decls, these can just come from the 
> ParsedAST though?
Ah right. Sorry! Sent rL342473.


Repository:
  rL LLVM

https://reviews.llvm.org/D5



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

[clang-tools-extra] r342473 - [clangd] Get rid of Decls parameter in indexMainDecls. NFC

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Sep 18 06:35:16 2018
New Revision: 342473

URL: http://llvm.org/viewvc/llvm-project?rev=342473=rev
Log:
[clangd] Get rid of Decls parameter in indexMainDecls. NFC

It's already available in ParsedAST.

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=342473=342472=342473=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Sep 18 06:35:16 2018
@@ -83,7 +83,7 @@ std::unique_ptr makeUp
 }
 
 void onMainAST(PathRef Path, ParsedAST ) override {
-  FIndex->updateMain(Path, AST, AST.getLocalTopLevelDecls());
+  FIndex->updateMain(Path, AST);
 }
   };
   return llvm::make_unique(FIndex);

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=342473=342472=342473=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Tue Sep 18 06:35:16 2018
@@ -65,10 +65,9 @@ indexSymbols(ASTContext , std::share
 }
 
 std::pair
-indexMainDecls(ParsedAST , llvm::ArrayRef TopLevelDecls,
-   llvm::ArrayRef URISchemes) {
+indexMainDecls(ParsedAST , llvm::ArrayRef URISchemes) {
   return indexSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
-  TopLevelDecls,
+  AST.getLocalTopLevelDecls(),
   /*IsIndexMainAST=*/true, URISchemes);
 }
 
@@ -163,9 +162,8 @@ void FileIndex::updatePreamble(PathRef P
   PreambleIndex.reset(PreambleSymbols.buildMemIndex());
 }
 
-void FileIndex::updateMain(PathRef Path, ParsedAST ,
-   llvm::ArrayRef TopLevelDecls) {
-  auto Contents = indexMainDecls(AST, TopLevelDecls, URISchemes);
+void FileIndex::updateMain(PathRef Path, ParsedAST ) {
+  auto Contents = indexMainDecls(AST, URISchemes);
   MainFileSymbols.update(
   Path, llvm::make_unique(std::move(Contents.first)),
   llvm::make_unique(std::move(Contents.second)));

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.h?rev=342473=342472=342473=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.h (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.h Tue Sep 18 06:35:16 2018
@@ -73,9 +73,9 @@ public:
   void updatePreamble(PathRef Path, ASTContext ,
   std::shared_ptr PP);
 
-  /// Update symbols from main file \p Path with symbols in \p TopLevelDecls.
-  void updateMain(PathRef Path, ParsedAST ,
-  llvm::ArrayRef TopLevelDecls);
+  /// Update symbols and references from main file \p Path with
+  /// `indexMainDecls`.
+  void updateMain(PathRef Path, ParsedAST );
 
 private:
   std::vector URISchemes;
@@ -106,12 +106,12 @@ private:
   std::unique_ptr MergedIndex;  // Merge preamble and main index.
 };
 
-/// Retrieves symbols and refs of \p Decls in \p AST.
+/// Retrieves symbols and refs of local top level decls in \p AST (i.e.
+/// `AST.getLocalTopLevelDecls()`).
 /// Exposed to assist in unit tests.
 /// If URISchemes is empty, the default schemes in SymbolCollector will be 
used.
 std::pair
-indexMainDecls(ParsedAST , llvm::ArrayRef Decls,
-   llvm::ArrayRef URISchemes = {});
+indexMainDecls(ParsedAST , llvm::ArrayRef URISchemes = {});
 
 /// Idex declarations from \p AST and macros from \p PP that are declared in
 /// included headers.

Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=342473=342472=342473=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Tue Sep 18 
06:35:16 2018
@@ -314,14 +314,14 @@ TEST(FileIndexTest, Refs) {
   Test.Code = MainCode.code();
   Test.Filename = "test.cc";
   auto AST = Test.build();
-  Index.updateMain(Test.Filename, AST, AST.getLocalTopLevelDecls());
+  Index.updateMain(Test.Filename, AST);
   // Add test2.cc
   TestTU Test2;
   Test2.HeaderCode = HeaderCode;
   Test2.Code = MainCode.code();