[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:334
+  "'%0' will always evaluate to 'true' in constexpr context">,
+  InGroup>;
+

rsmith wrote:
> Looks like you decided not to make this a subgroup of tautological-compare? 
> (I'm OK with that, I just wanted to make sure this was intentional given the 
> prior discussion.)
Yes, I decided to go with a new group since the warning is more powerful than 
gcc's version of this warning.


>> The technically-correct thing would be "in a manifestly constant-evaluated 
>> expression".

I will use this message - Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518



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

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This looks good, but it is missing the context. I suppose we were previously 
not providing any references for namespace aliases, because we were filtering 
both the alias and the underlying decl?
Could you update the revision summary and/or commit message ?




Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:893
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;

nit: I suppose this line checks we are not regressing the "non-alias case", but 
they are already being tested in previous tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69624



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

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> It looks like this patch contains a few other changes, besides the changes to 
> the prototypes. In particular, the change to CGBuiltin.cpp, and there are a 
> few new lines in the .td files that don't correspond to anything in the old 
> versions. Is that accidental, or is it part of cleaning up the prototypes, 
> somehow?

The extra .td lines are because just those 3 intrinsics used a fixed-width 
modifier ("give me half, no matter the input") with multiple sizes of input so 
there's no way to represent that in the new scheme and they need to be split 
up. Notice the integer ones are already split up because there was no 
corrresponding "give me int32_t" modifier. That change is actually already a 
separate NFC commit in my local repository and I'd commit it like that so that 
the script worked cleanly.

The CGBuiltin change follows from dropping the heuristic 
hasFloatingProtoModifier when deciding what type to pass to CGBuiltin for the 
intrinsics. This affected vmulx and the vcvt intrinsics. In vcvt's case I 
eventually decided to support them by moving to an explicit '!' modifier and 
special-casing conversion because they make good use of having signedness on 
the type they're given. I didn't revisit vmulx after that change, but I'd be 
inclined to leave it as it is; I kind of think it's unlikely someone 
implementing that now would make use of the ! modifier, which seems like a 
pretty rare requirement.

There are two other things that I think are pretty straightforward, but do 
clutter this patch so I'll split them out: removing the special behaviour of 
'a' (it can be implemented in .td at a net -ve lines); and changing Type to use 
an enum instead of a series of bools. I'll upload new diffs and update this one.

The other


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

https://reviews.llvm.org/D69618



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 227233.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments
- Get rid of raw string literals inside macro calls, to not break stage1 
compilers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -40,9 +40,9 @@
 #include 
 
 using ::testing::AllOf;
+using ::testing::ElementsAre;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
-using ::testing::ElementsAre;
 
 namespace clang {
 namespace clangd {
@@ -1377,7 +1377,7 @@
   // Template body is not parsed until instantiation time on windows, which
   // results in arbitrary failures as function body becomes NULL.
   ExtraArgs.push_back("-fno-delayed-template-parsing");
-  for(const auto &Case : Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1420,7 +1420,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test =R"cpp(
+  auto Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -1489,6 +1489,70 @@
   EXPECT_EQ(apply(Test), Expected);
 }
 
+TEST_F(DefineInlineTest, TransformParamNames) {
+  std::pair Cases[] = {
+  {R"cpp(
+void foo(int, bool b, int T\
+est);
+void ^foo(int f, bool x, int z) {})cpp",
+   R"cpp(
+void foo(int f, bool x, int z){}
+)cpp"},
+  {R"cpp(
+#define PARAM int Z
+void foo(PARAM);
+
+void ^foo(int X) {})cpp",
+   "fail: Cant rename parameter inside macro body."},
+  {R"cpp(
+#define TYPE int
+#define PARAM TYPE Z
+#define BODY(x) 5 * (x) + 2
+template 
+void foo(PARAM, TYPE Q, TYPE, TYPE W = BODY(P));
+template 
+void ^foo(int Z, int b, int c, int d) {})cpp",
+   R"cpp(
+#define TYPE int
+#define PARAM TYPE Z
+#define BODY(x) 5 * (x) + 2
+template 
+void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){}
+)cpp"},
+  };
+  for (const auto &Case : Cases)
+EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+  auto Test = R"cpp(
+struct Foo {
+  struct Bar {
+template  class, template class Y,
+  int, int Z>
+void foo(X, Y, int W = 5 * Z + 2);
+  };
+};
+
+template  class V, template class W,
+  int X, int Y>
+void Foo::Bar::f^oo(U, W, int Q) {})cpp";
+  auto Expected = R"cpp(
+struct Foo {
+  struct Bar {
+template  class V, template class W,
+  int X, int Y>
+void foo(U, W, int Q = 5 * Y + 2){}
+  };
+};
+
+)cpp";
+  EXPECT_EQ(apply(Test), Expected);
+}
+
 TEST_F(DefineInlineTest, TransformInlineNamespaces) {
   auto Test = R"cpp(
 namespace a { inline namespace b { namespace { struct Foo{}; } } }
@@ -1527,7 +1591,7 @@
   void fo^o() { return ; })cpp",
"fail: Couldn't find semicolon for target declaration."},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1585,7 +1649,7 @@
   void TARGET(){ return; }
   )cpp"},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,107 @@
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Generates Replacements for changing template and function parameter names in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  llvm::DenseMap ParamToNewName;
+  llvm::DenseMap> RefLocs;
+  auto HandleParam = [&](const NamedDecl *DestParam,
+ const NamedDecl *SourceParam) {
+// No need to rename if parameters already have the same name.
+if (DestParam->getName() == SourceParam->getName())
+  return;
+std::string NewName;
+// Unnamed parameters won't be visited in findExplicitReferences. So add
+// them here.
+if (DestParam->getName().empty()) {
+  RefLocs[DestParam].push_back(DestParam->getLocation());
+  // If decl is unnamed in destination we pad the new name to avoid gluing
+  // w

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Roman, thanks for reviewing and the ideas.

> Can you be more specific what the question is?

I meant to ping if anybody has more comments on this patch. Otherwise I would 
go ahead and submit it.


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

https://reviews.llvm.org/D69435



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

[clang-tools-extra] 71aa3f7 - [clangd] Add parameter renaming to define-inline code action

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-31T09:23:09+01:00
New Revision: 71aa3f7b7e43bf7d2a8a38324b1f9f7b12bbbdfc

URL: 
https://github.com/llvm/llvm-project/commit/71aa3f7b7e43bf7d2a8a38324b1f9f7b12bbbdfc
DIFF: 
https://github.com/llvm/llvm-project/commit/71aa3f7b7e43bf7d2a8a38324b1f9f7b12bbbdfc.diff

LOG: [clangd] Add parameter renaming to define-inline code action

Summary:
When moving a function definition to declaration location we also need
to handle renaming of the both function and template parameters.

This patch achives that by making sure every parameter name and dependent type
in destination is renamed to their respective name in the source.

Reviewers: ilya-biryukov

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
index 6bce81bae306..f6966f619ade 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,107 @@ llvm::Expected qualifyAllDecls(const 
FunctionDecl *FD) {
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Generates Replacements for changing template and function parameter names 
in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  llvm::DenseMap ParamToNewName;
+  llvm::DenseMap> RefLocs;
+  auto HandleParam = [&](const NamedDecl *DestParam,
+ const NamedDecl *SourceParam) {
+// No need to rename if parameters already have the same name.
+if (DestParam->getName() == SourceParam->getName())
+  return;
+std::string NewName;
+// Unnamed parameters won't be visited in findExplicitReferences. So add
+// them here.
+if (DestParam->getName().empty()) {
+  RefLocs[DestParam].push_back(DestParam->getLocation());
+  // If decl is unnamed in destination we pad the new name to avoid gluing
+  // with previous token, e.g. foo(int^) shouldn't turn into foo(intx).
+  NewName = " ";
+}
+NewName.append(SourceParam->getName());
+ParamToNewName[DestParam->getCanonicalDecl()] = std::move(NewName);
+  };
+
+  // Populate mapping for template parameters.
+  auto *DestTempl = Dest->getDescribedFunctionTemplate();
+  auto *SourceTempl = Source->getDescribedFunctionTemplate();
+  assert(bool(DestTempl) == bool(SourceTempl));
+  if (DestTempl) {
+const auto *DestTPL = DestTempl->getTemplateParameters();
+const auto *SourceTPL = SourceTempl->getTemplateParameters();
+assert(DestTPL->size() == SourceTPL->size());
+
+for (size_t I = 0, EP = DestTPL->size(); I != EP; ++I)
+  HandleParam(DestTPL->getParam(I), SourceTPL->getParam(I));
+  }
+
+  // Populate mapping for function params.
+  assert(Dest->param_size() == Source->param_size());
+  for (size_t I = 0, E = Dest->param_size(); I != E; ++I)
+HandleParam(Dest->getParamDecl(I), Source->getParamDecl(I));
+
+  const SourceManager &SM = Dest->getASTContext().getSourceManager();
+  const LangOptions &LangOpts = Dest->getASTContext().getLangOpts();
+  // Collect other references in function signature, i.e parameter types and
+  // default arguments.
+  findExplicitReferences(
+  // Use function template in case of templated functions to visit template
+  // parameters.
+  DestTempl ? llvm::dyn_cast(DestTempl) : llvm::dyn_cast(Dest),
+  [&](ReferenceLoc Ref) {
+if (Ref.Targets.size() != 1)
+  return;
+const auto *Target =
+llvm::cast(Ref.Targets.front()->getCanonicalDecl());
+auto It = ParamToNewName.find(Target);
+if (It == ParamToNewName.end())
+  return;
+RefLocs[Target].push_back(Ref.NameLoc);
+  });
+
+  // Now try to generate edits for all the refs.
+  tooling::Replacements Replacements;
+  for (auto &Entry : RefLocs) {
+const auto *OldDecl = Entry.first;
+llvm::StringRef OldName = OldDecl->getName();
+llvm::StringRef NewName = ParamToNewName[OldDecl];
+for (SourceLocation RefLoc : Entry.second) {
+  CharSourceRange ReplaceRange;
+  // In case of unnamed parameters, we have an empty char range, whereas we
+  // have a tokenrange at RefLoc with named parameters.
+  if (OldName.empty())
+ReplaceRange = CharSourceRange::getCharRange(RefLoc, RefLoc);
+  else
+ReplaceRange = CharSourceRange::getTokenRange(RefLoc, RefLoc);
+  // If occurence is coming from a macro expansion, try to get back to the
+  // file range.
+  if (RefLoc.isMacroID()) {
+ReplaceR

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Did you see some significant perf improvements for Chromium?


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

https://reviews.llvm.org/D69435



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG71aa3f7b7e43: [clangd] Add parameter renaming to 
define-inline code action (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -40,9 +40,9 @@
 #include 
 
 using ::testing::AllOf;
+using ::testing::ElementsAre;
 using ::testing::HasSubstr;
 using ::testing::StartsWith;
-using ::testing::ElementsAre;
 
 namespace clang {
 namespace clangd {
@@ -1386,7 +1386,7 @@
   // Template body is not parsed until instantiation time on windows, which
   // results in arbitrary failures as function body becomes NULL.
   ExtraArgs.push_back("-fno-delayed-template-parsing");
-  for(const auto &Case : Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1429,7 +1429,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test =R"cpp(
+  auto Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -1498,6 +1498,70 @@
   EXPECT_EQ(apply(Test), Expected);
 }
 
+TEST_F(DefineInlineTest, TransformParamNames) {
+  std::pair Cases[] = {
+  {R"cpp(
+void foo(int, bool b, int T\
+est);
+void ^foo(int f, bool x, int z) {})cpp",
+   R"cpp(
+void foo(int f, bool x, int z){}
+)cpp"},
+  {R"cpp(
+#define PARAM int Z
+void foo(PARAM);
+
+void ^foo(int X) {})cpp",
+   "fail: Cant rename parameter inside macro body."},
+  {R"cpp(
+#define TYPE int
+#define PARAM TYPE Z
+#define BODY(x) 5 * (x) + 2
+template 
+void foo(PARAM, TYPE Q, TYPE, TYPE W = BODY(P));
+template 
+void ^foo(int Z, int b, int c, int d) {})cpp",
+   R"cpp(
+#define TYPE int
+#define PARAM TYPE Z
+#define BODY(x) 5 * (x) + 2
+template 
+void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){}
+)cpp"},
+  };
+  for (const auto &Case : Cases)
+EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
+}
+
+TEST_F(DefineInlineTest, TransformTemplParamNames) {
+  auto Test = R"cpp(
+struct Foo {
+  struct Bar {
+template  class, template class Y,
+  int, int Z>
+void foo(X, Y, int W = 5 * Z + 2);
+  };
+};
+
+template  class V, template class W,
+  int X, int Y>
+void Foo::Bar::f^oo(U, W, int Q) {})cpp";
+  auto Expected = R"cpp(
+struct Foo {
+  struct Bar {
+template  class V, template class W,
+  int X, int Y>
+void foo(U, W, int Q = 5 * Y + 2){}
+  };
+};
+
+)cpp";
+  EXPECT_EQ(apply(Test), Expected);
+}
+
 TEST_F(DefineInlineTest, TransformInlineNamespaces) {
   auto Test = R"cpp(
 namespace a { inline namespace b { namespace { struct Foo{}; } } }
@@ -1536,7 +1600,7 @@
   void fo^o() { return ; })cpp",
"fail: Couldn't find semicolon for target declaration."},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
@@ -1594,7 +1658,7 @@
   void TARGET(){ return; }
   )cpp"},
   };
-  for(const auto& Case: Cases)
+  for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -226,6 +226,107 @@
   return QualifiedFunc->substr(BodyBegin, BodyEnd - BodyBegin + 1);
 }
 
+/// Generates Replacements for changing template and function parameter names in
+/// \p Dest to be the same as in \p Source.
+llvm::Expected
+renameParameters(const FunctionDecl *Dest, const FunctionDecl *Source) {
+  llvm::DenseMap ParamToNewName;
+  llvm::DenseMap> RefLocs;
+  auto HandleParam = [&](const NamedDecl *DestParam,
+ const NamedDecl *SourceParam) {
+// No need to rename if parameters already have the same name.
+if (DestParam->getName() == SourceParam->getName())
+  return;
+std::string NewName;
+// Unnamed parameters won't be visited in findExplicitReferences. So add
+// them here.
+if (DestParam->getName().empty()) {
+  RefLocs[DestParam].push_back(DestParam->getLocation());
+  // If decl is unnamed in destination we pad the new name to avoid gluing
+  // with previous token, e.g. foo(

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59704 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 227237.
xbolva00 added a comment.

Fixed review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
  clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp

Index: clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+namespace std {
+constexpr bool is_constant_evaluated() noexcept {
+  return __builtin_is_constant_evaluated();
+}
+} // namespace std
+
+constexpr int fn1() {
+  if constexpr (std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn2() {
+  if constexpr (!std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn3() {
+  if constexpr (std::is_constant_evaluated() == false) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn4() {
+  if constexpr (__builtin_is_constant_evaluated() == true) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn5() {
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn1() {
+  if (std::is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn2() {
+  if (!__builtin_is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
Index: clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
===
--- clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -Wno-constant-evaluated -triple=x86_64-linux-gnu
 
 using size_t = decltype(sizeof(int));
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10590,8 +10590,24 @@
 return false;
   }
 
-  case Builtin::BI__builtin_is_constant_evaluated:
+  case Builtin::BI__builtin_is_constant_evaluated: {
+const auto *Callee = Info.CurrentCall->getCallee();
+if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+(Info.CallStackDepth == 1 ||
+ (Info.CallStackDepth == 2 && Callee->isInStdNamespace() &&
+  Callee->getIdentifier() &&
+  Callee->getIdentifier()->isStr("is_constant_evaluated" {
+  // FIXME: Find a better way to avoid duplicated diagnostics.
+  if (Info.EvalStatus.Diag)
+Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()
+   : Info.CurrentCall->CallLoc,
+diag::warn_is_constant_evaluated_always_true_constexpr)
+<< (Info.CallStackDepth == 1 ? "__builtin_is_constant_evaluated"
+ : "std::is_constant_evaluated");
+}
+
 return Success(Info.InConstantContext, E);
+  }
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -329,6 +329,10 @@
 def note_unimplemented_constexpr_lambda_feature_ast : Note<
 "unimplemented constexpr lambda feature: %0 (coming soon!)">;
 
+def warn_is_constant_evaluated_always_true_constexpr : Warning<
+  "'%0' will always evaluate to 'true' in a manifestly constant-evaluated expression">,
+  InGroup>;
+
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_escape : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Thanks you for the review and advices!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518



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

[PATCH] D69518: [Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[Dávid Bolvanský via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb06305e44949: [Diagnostics] Warn for 
std::is_constant_evaluated in constexpr mode (authored by xbolva00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69518

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
  clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp

Index: clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+namespace std {
+constexpr bool is_constant_evaluated() noexcept {
+  return __builtin_is_constant_evaluated();
+}
+} // namespace std
+
+constexpr int fn1() {
+  if constexpr (std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn2() {
+  if constexpr (!std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn3() {
+  if constexpr (std::is_constant_evaluated() == false) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn4() {
+  if constexpr (__builtin_is_constant_evaluated() == true) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int fn5() {
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn1() {
+  if (std::is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
+
+constexpr int nowarn2() {
+  if (!__builtin_is_constant_evaluated())
+return 0;
+  else
+return 1;
+}
Index: clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
===
--- clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -Wno-constant-evaluated -triple=x86_64-linux-gnu
 
 using size_t = decltype(sizeof(int));
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10593,8 +10593,24 @@
 return false;
   }
 
-  case Builtin::BI__builtin_is_constant_evaluated:
+  case Builtin::BI__builtin_is_constant_evaluated: {
+const auto *Callee = Info.CurrentCall->getCallee();
+if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+(Info.CallStackDepth == 1 ||
+ (Info.CallStackDepth == 2 && Callee->isInStdNamespace() &&
+  Callee->getIdentifier() &&
+  Callee->getIdentifier()->isStr("is_constant_evaluated" {
+  // FIXME: Find a better way to avoid duplicated diagnostics.
+  if (Info.EvalStatus.Diag)
+Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()
+   : Info.CurrentCall->CallLoc,
+diag::warn_is_constant_evaluated_always_true_constexpr)
+<< (Info.CallStackDepth == 1 ? "__builtin_is_constant_evaluated"
+ : "std::is_constant_evaluated");
+}
+
 return Success(Info.InConstantContext, E);
+  }
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -329,6 +329,10 @@
 def note_unimplemented_constexpr_lambda_feature_ast : Note<
 "unimplemented constexpr lambda feature: %0 (coming soon!)">;
 
+def warn_is_constant_evaluated_always_true_constexpr : Warning<
+  "'%0' will always evaluate to 'true' in a manifestly constant-evaluated expression">,
+  InGroup>;
+
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_escape : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 227241.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay set the repository for this revision to rC Clang.
MyDeveloperDay added a comment.

Address review comments, deduction guides with embedded parens


Repository:
  rC Clang

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

https://reviews.llvm.org/D69577

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4977,6 +4977,24 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> Afoo<1>)>;");
+  verifyFormat("A() -> A<(3 < 2)>;");
+  verifyFormat("A() -> A<((3) < (2))>;");
+
+  // Ensure not deduction guides.
+  verifyFormat("c()->f();");
+  verifyFormat("x()->foo<1>;");
+  verifyFormat("x = p->foo<3>();");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,6 +1350,58 @@
 }
   }
 
+  static FormatToken *untilMatchingParen(FormatToken *Current) {
+// for when MatchingParen is not yet established
+int ParenLevel = 0;
+while (Current) {
+  if (Current->is(tok::l_paren))
+ParenLevel++;
+  if (Current->is(tok::r_paren))
+ParenLevel--;
+  if (ParenLevel < 1)
+break;
+  Current = Current->Next;
+}
+return Current;
+  }
+
+  static bool isDeductionGuide(FormatToken &Current) {
+// Look for a deduction guide A(...) -> A<...>;
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.
+  FormatToken *TemplateCloser = Current.Next->Next;
+  int NestingLevel = 0;
+  while (TemplateCloser) {
+// Skip over an expressions in parens  A<(3 < 2)>;
+if (TemplateCloser->is(tok::l_paren)) {
+  // No Matching Paren yet so skip to matching paren
+  TemplateCloser = untilMatchingParen(TemplateCloser);
+}
+if (TemplateCloser->is(tok::less))
+  NestingLevel++;
+if (TemplateCloser->is(tok::greater))
+  NestingLevel--;
+if (NestingLevel < 1)
+  break;
+TemplateCloser = TemplateCloser->Next;
+  }
+  // Assuming we have found the end of the template ensure its followed
+  // with a ;
+  if (TemplateCloser && TemplateCloser->Next &&
+  TemplateCloser->Next->is(tok::semi) &&
+  Current.Previous->MatchingParen) {
+// Determine if the identifier `A` prior to the A<..>; is the same as
+// prior to the A(..)
+FormatToken *LeadingIdentifier =
+Current.Previous->MatchingParen->Previous;
+return (LeadingIdentifier &&
+LeadingIdentifier->TokenText == Current.Next->TokenText);
+  }
+}
+return false;
+  }
+
   void determineTokenType(FormatToken &Current) {
 if (!Current.is(TT_Unknown))
   // The token type is already known.
@@ -1397,6 +1449,10 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
+
+} else if (isDeductionGuide(Current)) {
+  // Deduction guides trailing arrow "...) -> A;".
+  Current.Type = TT_TrailingReturnArrow;
 } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

As discussed offline, it might make more sense to fix this in `targetDecls` 
itself. Considering how the `Alias` is handled for typedefs and usings, it 
feels like this is a mistake for namespace aliases to be marked in that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69624



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

[clang] 92aa0c2 - [cfi] Add flag to always generate .debug_frame

[David Candler via cfe-commits]

Author: David Candler
Date: 2019-10-31T09:48:30Z
New Revision: 92aa0c2dbcb723d102c508f6e7559330b637f912

URL: 
https://github.com/llvm/llvm-project/commit/92aa0c2dbcb723d102c508f6e7559330b637f912
DIFF: 
https://github.com/llvm/llvm-project/commit/92aa0c2dbcb723d102c508f6e7559330b637f912.diff

LOG: [cfi] Add flag to always generate .debug_frame

This adds a flag to LLVM and clang to always generate a .debug_frame
section, even if other debug information is not being generated. In
situations where .eh_frame would normally be emitted, both .debug_frame
and .eh_frame will be used.

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

Added: 
clang/test/Driver/fforce-dwarf-frame.c
llvm/test/CodeGen/ARM/dwarf-frame.ll

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/CodeGen/CommandFlags.inc
llvm/include/llvm/CodeGen/MachineFunction.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
llvm/lib/CodeGen/CFIInstrInserter.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
llvm/lib/Target/ARC/ARCRegisterInfo.cpp
llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/lib/Target/X86/X86InstrInfo.cpp
llvm/lib/Target/XCore/XCoreRegisterInfo.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index f8d94e352f28..0a47869af61b 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -97,6 +97,8 @@ CODEGENOPT(CFProtectionBranch , 1, 0) ///< if -fcf-protection 
is
 CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
 CODEGENOPT(StackSizeSection  , 1, 0) ///< Set when -fstack-size-section is 
enabled.
+CODEGENOPT(ForceDwarfFrameSection , 1, 0) ///< Set when -fforce-dwarf-frame is
+  ///< enabled.
 
 ///< Set when -fxray-always-emit-customevents is enabled.
 CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0)

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 2401a31ceb9a..07c27ebcefee 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1950,6 +1950,10 @@ def fdebug_prefix_map_EQ
   : Joined<["-"], "fdebug-prefix-map=">, Group,
 Flags<[CC1Option,CC1AsOption]>,
 HelpText<"remap file source paths in debug info">;
+def fforce_dwarf_frame : Flag<["-"], "fforce-dwarf-frame">, Group, 
Flags<[CC1Option]>,
+HelpText<"Always emit a debug frame section">;
+def fno_force_dwarf_frame : Flag<["-"], "fno-force-dwarf-frame">, 
Group, Flags<[CC1Option]>,
+HelpText<"Don't always emit a debug frame section">;
 def g_Flag : Flag<["-"], "g">, Group,
   HelpText<"Generate source-level debug information">;
 def gline_tables_only : Flag<["-"], "gline-tables-only">, Group,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 75a54d8f3c8a..c15dc0be0b15 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -485,6 +485,7 @@ static void initTargetOptions(llvm::TargetOptions &Options,
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
   Options.EnableDebugEntryValues = CodeGenOpts.EnableDebugEntryValues;
+  Options.ForceDwarfFrameSection = CodeGenOpts.ForceDwarfFrameSection;
 
   Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
   Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index c60dc76ae1bf..81e01aee1da9 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3339,6 +3339,10 @@ static void RenderDebugOptions(const ToolChain &TC, 
const Driver &D,
 CmdArgs.push_back("-generate-arange-section");
   }
 
+  if (Args.hasFlag(options::OPT_fforce_dwarf_frame,
+   options::OPT_fno_force_dwarf_frame, false))
+CmdArgs.push_back("-fforce-dwarf-frame");
+
   if (Args.hasFlag(options::OPT_fdebug_types_section,
options::OPT_fno_debug_types_section, false)) {
 if (!T.isOSBinFormatELF()) {

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index f197a67e7a38..0775d1021c54 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -769,6 +76

[PATCH] D67216: [cfi] Add flag to always generate .debug_frame

[David Candler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92aa0c2dbcb7: [cfi] Add flag to always generate .debug_frame 
(authored by dcandler).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67216

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/fforce-dwarf-frame.c
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
  llvm/lib/CodeGen/CFIInstrInserter.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
  llvm/lib/Target/ARC/ARCRegisterInfo.cpp
  llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
  llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
  llvm/test/CodeGen/ARM/dwarf-frame.ll

Index: llvm/test/CodeGen/ARM/dwarf-frame.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/dwarf-frame.ll
@@ -0,0 +1,38 @@
+; RUN: llc -mtriple armv7-unknown -frame-pointer=all -filetype=asm -o - %s | FileCheck %s --check-prefix=CHECK-NO-CFI
+; RUN: llc -mtriple armv7-unknown -frame-pointer=all -filetype=asm -force-dwarf-frame-section -o - %s | FileCheck %s --check-prefix=CHECK-ALWAYS-CFI
+
+declare void @dummy_use(i32*, i32)
+
+define void @test_basic() #0 {
+%mem = alloca i32, i32 10
+call void @dummy_use (i32* %mem, i32 10)
+  ret void
+}
+
+; CHECK-NO-CFI-LABEL: test_basic:
+; CHECK-NO-CFI:   .fnstart
+; CHECK-NO-CFI-NOT:   .cfi_sections .debug_frame
+; CHECK-NO-CFI-NOT:   .cfi_startproc
+; CHECK-NO-CFI:   @ %bb.0:
+; CHECK-NO-CFI:   push {r11, lr}
+; CHECK-NO-CFI-NOT:   .cfi_def_cfa_offset 8
+; CHECK-NO-CFI-NOT:   .cfi_offset lr, -4
+; CHECK-NO-CFI-NOT:   .cfi_offset r11, -8
+; CHECK-NO-CFI:   mov r11, sp
+; CHECK-NO-CFI-NOT:   .cfi_def_cfa_register r11
+; CHECK-NO-CFI-NOT:   .cfi_endproc
+; CHECK-NO-CFI:   .fnend
+
+; CHECK-ALWAYS-CFI-LABEL: test_basic:
+; CHECK-ALWAYS-CFI:   .fnstart
+; CHECK-ALWAYS-CFI:   .cfi_sections .debug_frame
+; CHECK-ALWAYS-CFI:   .cfi_startproc
+; CHECK-ALWAYS-CFI:   @ %bb.0:
+; CHECK-ALWAYS-CFI:   push {r11, lr}
+; CHECK-ALWAYS-CFI:   .cfi_def_cfa_offset 8
+; CHECK-ALWAYS-CFI:   .cfi_offset lr, -4
+; CHECK-ALWAYS-CFI:   .cfi_offset r11, -8
+; CHECK-ALWAYS-CFI:   mov r11, sp
+; CHECK-ALWAYS-CFI:   .cfi_def_cfa_register r11
+; CHECK-ALWAYS-CFI:   .cfi_endproc
+; CHECK-ALWAYS-CFI:   .fnend
Index: llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
===
--- llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
+++ llvm/lib/Target/XCore/XCoreRegisterInfo.cpp
@@ -203,7 +203,7 @@
 }
 
 bool XCoreRegisterInfo::needsFrameMoves(const MachineFunction &MF) {
-  return MF.getMMI().hasDebugInfo() || MF.getFunction().needsUnwindTableEntry();
+  return MF.needsFrameMoves();
 }
 
 const MCPhysReg *
Index: llvm/lib/Target/X86/X86InstrInfo.cpp
===
--- llvm/lib/Target/X86/X86InstrInfo.cpp
+++ llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -3963,9 +3963,7 @@
   MachineFunction &MF = *MBB.getParent();
   const X86FrameLowering *TFL = Subtarget.getFrameLowering();
   bool IsWin64Prologue = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
-  bool NeedsDwarfCFI =
-  !IsWin64Prologue &&
-  (MF.getMMI().hasDebugInfo() || MF.getFunction().needsUnwindTableEntry());
+  bool NeedsDwarfCFI = !IsWin64Prologue && MF.needsFrameMoves();
   bool EmitCFI = !TFL->hasFP(MF) && NeedsDwarfCFI;
   if (EmitCFI) {
 TFL->BuildCFI(MBB, I, DL,
Index: llvm/lib/Target/X86/X86FrameLowering.cpp
===
--- llvm/lib/Target/X86/X86FrameLowering.cpp
+++ llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -993,8 +993,7 @@
   bool NeedsWinFPO =
   !IsFunclet && STI.isTargetWin32() && MMI.getModule()->getCodeViewFlag();
   bool NeedsWinCFI = NeedsWin64CFI || NeedsWinFPO;
-  bool NeedsDwarfCFI =
-  !IsWin64Prologue && (MMI.hasDebugInfo() || Fn.needsUnwindTableEntry());
+  bool NeedsDwarfCFI = !IsWin64Prologue && MF.needsFrameMoves();
   Register FramePtr = TRI->getFrameRegister(MF);
   const Register MachineFramePtr =
   STI.isTarget64BitILP32()
@@ -1614,10 +1613,9 @@
   bool HasFP = hasFP(MF);
   uint64_t NumBytes = 0;
 
-  bool NeedsDwarfCFI =
-  (!MF.getTarget().getTargetTriple().isOSDarwin() &&
-   !MF.getTarget().getTargetTriple().isOSWindows()) &&
-  (MF.getMMI().hasDebugInfo() || MF.getFunction().needsUnwindTableEntry());
+  boo

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 227254.
whisperity added a comment.

- Removed //`This check`// from the documentation comment of the check's class 
too.


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

https://reviews.llvm.org/D69560

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-cvr-on.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-default.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s \
+// RUN:   cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN:   ]}' --
+
+struct T {};
+
+void memcpy(struct T *restrict dest, const struct T *restrict src) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent arguments for 'memcpy' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:63: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:38: note: at a call site, 'const struct T * restrict' might bind with same force as 'struct T *restrict'
+
+void merge(struct T *dst, const struct T *src1, const struct T *src2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 3 adjacent arguments for 'merge' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:65: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:27: note: at a call site, 'const struct T *' might bind with same force as 'struct T *'
+// CHECK-MESSAGES: :[[@LINE-4]]:49: note: at a call site, 'const struct T *' might bind with same force as 'struct T *'
+
+int equals(struct T a, struct T b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent arguments for 'equals' of similar type ('struct T') are
+// CHECK-MESSAGES: :[[@LINE-2]]:33: note: last argument in the adjacent range
+
+typedef struct {
+  int x;
+} S;
+
+int equals2(S l, S r) { return l.x == r.x; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent arguments for 'equals2' of similar type ('S') are
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: last argument in the adjacent range
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
@@ -0,0 +1,420 @@
+// RUN: %check_clang_tidy %s \
+// RUN:   cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN:   ]}' --
+
+void library(void *vp, void *vp2, void *vp3, int n, int m);
+// NO-WARN: The user has no chance to change only declared (usually library)
+// functions, so no diagnostic is made.
+
+struct T {};
+
+void create(T **out_t) {} // NO-WARN
+
+void copy(T *p, T *q, int n) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 2 adjacent arguments for 'copy' of similar type ('T *') are easily swapped by mistake [cppcoreguidelines-avoid-adjacent-arguments-of-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: last argument in the adjacent range here
+
+void mov(T *dst, const T *src) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 2 adjacent arguments for 'mov' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: last argument in the adjacent range here
+// CHECK-MESSAGES: :[[@LINE-3]]:18: note: at a call site, 'const T *' might bind with same force as 'T *'
+
+void compare01(T t1, T t2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'compar

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

[Whisperity via Phabricator via cfe-commits]

whisperity marked 11 inline comments as done.
whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp:488
+void AdjacentArgumentsOfSameTypeCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;

whisperity wrote:
> Eugene.Zelenko wrote:
> > Check seems to be useful for C and probably for Objective-C.
> I'm not knowledgeable about Objective-C at all to make a decision on how the 
> "adjacent argument ranges" could be calculated and what mixups are possible. 
> As for C, should a `cppcoreguidelines-` check be enabled for C? Or you mean 
> we should allow it to work, and the user will toggle how they see fit.
I've added a  `FIXME` for ObjC as I'm really not qualified in that language. C 
support has been added.


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

https://reviews.llvm.org/D69560



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

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 227253.
whisperity added a comment.

- Fix some comments, formatting and documentation
- Organised test files to be in the same directory as others on the Monorepo 
structure
- Helper functions moved from `namespace (anonymous)` to `static`.
- Added test for C and enabled check for C code.


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

https://reviews.llvm.org/D69560

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-cvr-on.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-default.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s \
+// RUN:   cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN:   ]}' --
+
+struct T {};
+
+void memcpy(struct T *restrict dest, const struct T *restrict src) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent arguments for 'memcpy' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:63: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:38: note: at a call site, 'const struct T * restrict' might bind with same force as 'struct T *restrict'
+
+void merge(struct T *dst, const struct T *src1, const struct T *src2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 3 adjacent arguments for 'merge' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:65: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:27: note: at a call site, 'const struct T *' might bind with same force as 'struct T *'
+// CHECK-MESSAGES: :[[@LINE-4]]:49: note: at a call site, 'const struct T *' might bind with same force as 'struct T *'
+
+int equals(struct T a, struct T b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent arguments for 'equals' of similar type ('struct T') are
+// CHECK-MESSAGES: :[[@LINE-2]]:33: note: last argument in the adjacent range
+
+typedef struct {
+  int x;
+} S;
+
+int equals2(S l, S r) { return l.x == r.x; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent arguments for 'equals2' of similar type ('S') are
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: last argument in the adjacent range
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
@@ -0,0 +1,420 @@
+// RUN: %check_clang_tidy %s \
+// RUN:   cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN:   ]}' --
+
+void library(void *vp, void *vp2, void *vp3, int n, int m);
+// NO-WARN: The user has no chance to change only declared (usually library)
+// functions, so no diagnostic is made.
+
+struct T {};
+
+void create(T **out_t) {} // NO-WARN
+
+void copy(T *p, T *q, int n) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 2 adjacent arguments for 'copy' of similar type ('T *') are easily swapped by mistake [cppcoreguidelines-avoid-adjacent-arguments-of-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: last argument in the adjacent range here
+
+void mov(T *dst, const T *src) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 2 adjacent arguments for 'mov' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: last argument in the adjacent range here
+// CHECK-MESSAGES: :[[@LINE-3]]:18: note: at a c

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: include/clang/Format/Format.h:500
+  /// bracket of a template parameter list.
+  bool SpaceBeforeTemplateParameterList;
+

I suggest we make this an enumeration

```
enum {
Unaltered,
Never,
Always,
}
```

Introducing this into a codebase means we need to select a suitable default, 
doing nothing would in my view be the best default and introducing anything new 
now will cause a tidal wave of changes on any existing formatted code base

LLVM alone it pretty much 50/50 on if they use a space or not


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

https://reviews.llvm.org/D14927



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

[PATCH] D69657: [AArch64][SVE] Implement several floating-point arithmetic intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin created this revision.
kmclaughlin added reviewers: huntergr, sdesmalen, dancgr.
Herald added subscribers: psnobl, rkruppe, hiraditya, kristof.beyls, tschuett.
Herald added a project: LLVM.

Adds intrinsics for the following:

- fabd, fadd, fsub & fsubr
- fmul, fmulx, fdiv & fdivr
- fmax, fmaxnm, fmin & fminnm
- fscale & ftsmul


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69657

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll
@@ -0,0 +1,530 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; FABD
+;
+
+define  @fabd_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_h:
+; CHECK: fabd z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fabd_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_s:
+; CHECK: fabd z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fabd_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_d:
+; CHECK: fabd z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FADD
+;
+
+define  @fadd_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_h:
+; CHECK: fadd z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fadd_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_s:
+; CHECK: fadd z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fadd_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_d:
+; CHECK: fadd z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FDIV
+;
+
+define  @fdiv_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_h:
+; CHECK: fdiv z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fdiv_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_s:
+; CHECK: fdiv z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fdiv_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_d:
+; CHECK: fdiv z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FDIVR
+;
+
+define  @fdivr_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_h:
+; CHECK: fdivr z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv8f16( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fdivr_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_s:
+; CHECK: fdivr z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv4f32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @fdivr_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_d:
+; CHECK: fdivr z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv2f64( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+;
+; FMAX
+;
+
+define  @fmax_h( %pg

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[MyDeveloperDay via Phabricator via cfe-commits]

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

Thanks for the patch, sorry I must have missed this coming in. LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D69649



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

[PATCH] D33944: git-clang-format: Add --cached option to format index

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Is this revision still relavant, I find it annoying that I have to run git add 
again on any files that are already added but have been formatted.

If you think this is useful and can rebase it perhaps we can go around the 
review cycle again with a fresh set of eyes


Repository:
  rC Clang

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

https://reviews.llvm.org/D33944



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

[PATCH] D69162: [clangd] Remove using-namespace present inside a namespace.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

One important question about running on the whole TU in all cases. Other than 
that LG




Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:110
 return false;
-  if (!dyn_cast(TargetDirective->getDeclContext()))
-return false;
+  auto DeclCtx = TargetDirective->getDeclContext();
   // FIXME: Unavailable for namespaces containing using-namespace decl.

NIT: use `auto*`
knowing it's a pointer is quite useful when reading and writing the code (e.g. 
suggests one need to use `->` instead of `.` to access the variable)



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:179
+
+  if (ContainingNS) {
+for (auto ReDeclNS : ContainingNS->redecls())

Could you explain why don't we always just run across the whole TU?
What disadvantages would that have?



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:182
+  findExplicitReferences(ReDeclNS, SelectRefToQualify);
+
+  } else {

NIT: remove empty line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69162



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Thanks Jorg! Given that the status is still "Needs Review", I assume I also 
still need an LGTM from Marshall, Louis or Eric, right?


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

https://reviews.llvm.org/D69520



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

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

In D69624#1728377 , @kadircet wrote:

> As discussed offline, it might make more sense to fix this in `targetDecls` 
> itself. Considering how the `Alias` is handled for typedefs and usings, it 
> feels like this is a mistake for namespace aliases to be marked in that way.


I took another look and it appears the typedefs also have the `Alias` relation 
set. So `targetDecl` is consistent there and the current fix seems ok.




Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:893
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;

kadircet wrote:
> nit: I suppose this line checks we are not regressing the "non-alias case", 
> but they are already being tested in previous tests.
Yeah, that's just validating that non-aliases also work properly and that's 
definitely redundant.
I'll keep it as is, it doesn't seem to cause too much noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69624



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

[clang] 4b6597f - Make flang driver stuff work on macOS

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2019-10-31T07:34:32-04:00
New Revision: 4b6597f49896529170fde38f5d5fb46d687e0c71

URL: 
https://github.com/llvm/llvm-project/commit/4b6597f49896529170fde38f5d5fb46d687e0c71
DIFF: 
https://github.com/llvm/llvm-project/commit/4b6597f49896529170fde38f5d5fb46d687e0c71.diff

LOG: Make flang driver stuff work on macOS

6bf55804 added special-case code for TY_PP_Fortran to
ToolChain::LookupTypeForExtension(), but
Darwin::LookupTypeForExtension() overrode that method without calling
the superclass implementation.

Make it call the superclass implementation to fix things.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/flang/flang.f90

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index ee08b8208d93..cb76fb0da18a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -737,7 +737,7 @@ Darwin::Darwin(const Driver &D, const llvm::Triple &Triple, 
const ArgList &Args)
   CudaInstallation(D, Triple, Args) {}
 
 types::ID MachO::LookupTypeForExtension(StringRef Ext) const {
-  types::ID Ty = types::lookupTypeForExtension(Ext);
+  types::ID Ty = ToolChain::lookupTypeForExtension(Ext);
 
   // Darwin always preprocesses assembly files (unless -x is used explicitly).
   if (Ty == types::TY_PP_Asm)

diff  --git a/clang/test/Driver/flang/flang.f90 
b/clang/test/Driver/flang/flang.f90
index 97e4847f8439..9d47c7c90225 100644
--- a/clang/test/Driver/flang/flang.f90
+++ b/clang/test/Driver/flang/flang.f90
@@ -1,7 +1,3 @@
-! D63607 made mac builders unhappy by failing this test, and it isn't
-! yet obvious why. Mark as unsupported as a temporary measure.
-! UNSUPPORTED: darwin
-
 ! Check that flang -fc1 is invoked when in --driver-mode=flang.
 
 ! This is a copy of flang_ucase.F90 because the driver has logic in it which



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

[PATCH] D69636: Make flang driver stuff work on macOS

[Nico Weber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b6597f49896: Make flang driver stuff work on macOS 
(authored by thakis).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69636

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/flang/flang.f90


Index: clang/test/Driver/flang/flang.f90
===
--- clang/test/Driver/flang/flang.f90
+++ clang/test/Driver/flang/flang.f90
@@ -1,7 +1,3 @@
-! D63607 made mac builders unhappy by failing this test, and it isn't
-! yet obvious why. Mark as unsupported as a temporary measure.
-! UNSUPPORTED: darwin
-
 ! Check that flang -fc1 is invoked when in --driver-mode=flang.
 
 ! This is a copy of flang_ucase.F90 because the driver has logic in it which
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -737,7 +737,7 @@
   CudaInstallation(D, Triple, Args) {}
 
 types::ID MachO::LookupTypeForExtension(StringRef Ext) const {
-  types::ID Ty = types::lookupTypeForExtension(Ext);
+  types::ID Ty = ToolChain::lookupTypeForExtension(Ext);
 
   // Darwin always preprocesses assembly files (unless -x is used explicitly).
   if (Ty == types::TY_PP_Asm)


Index: clang/test/Driver/flang/flang.f90
===
--- clang/test/Driver/flang/flang.f90
+++ clang/test/Driver/flang/flang.f90
@@ -1,7 +1,3 @@
-! D63607 made mac builders unhappy by failing this test, and it isn't
-! yet obvious why. Mark as unsupported as a temporary measure.
-! UNSUPPORTED: darwin
-
 ! Check that flang -fc1 is invoked when in --driver-mode=flang.
 
 ! This is a copy of flang_ucase.F90 because the driver has logic in it which
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -737,7 +737,7 @@
   CudaInstallation(D, Triple, Args) {}
 
 types::ID MachO::LookupTypeForExtension(StringRef Ext) const {
-  types::ID Ty = types::lookupTypeForExtension(Ext);
+  types::ID Ty = ToolChain::lookupTypeForExtension(Ext);
 
   // Darwin always preprocesses assembly files (unless -x is used explicitly).
   if (Ty == types::TY_PP_Asm)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
balazske marked 3 inline comments as done.
Closed by commit rG4980c1333fa4: [clang][analyzer] Using CallDescription in 
StreamChecker. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream.c
  clang/test/Analysis/stream.cpp

Index: clang/test/Analysis/stream.cpp
===
--- /dev/null
+++ clang/test/Analysis/stream.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
+
+typedef struct _IO_FILE FILE;
+extern FILE *fopen(const char *path, const char *mode);
+
+struct X {
+  int A;
+  int B;
+};
+
+void *fopen(X x, const char *mode) {
+  return new char[4];
+}
+
+void f1() {
+  X X1;
+  void *p = fopen(X1, "oo");
+} // no-warning
+
+void f2() {
+  FILE *f = fopen("file", "r");
+} // expected-warning {{Opened File never closed. Potential Resource leak}}
Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
 
 typedef __typeof__(sizeof(int)) size_t;
+typedef __typeof__(sizeof(int)) fpos_t;
 typedef struct _IO_FILE FILE;
 #define SEEK_SET	0	/* Seek from beginning of file.  */
 #define SEEK_CUR	1	/* Seek from current position.  */
@@ -9,36 +10,93 @@
 extern FILE *tmpfile(void);
 extern int fclose(FILE *fp);
 extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
+extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
 extern int fseek (FILE *__stream, long int __off, int __whence);
 extern long int ftell (FILE *__stream);
 extern void rewind (FILE *__stream);
+extern int fgetpos(FILE *stream, fpos_t *pos);
+extern int fsetpos(FILE *stream, const fpos_t *pos);
+extern void clearerr(FILE *stream);
+extern int feof(FILE *stream);
+extern int ferror(FILE *stream);
+extern int fileno(FILE *stream);
 
-void f1(void) {
-  FILE *p = fopen("foo", "r");
-  char buf[1024];
-  fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
+void check_fread() {
+  FILE *fp = tmpfile();
+  fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
 }
 
-void f2(void) {
-  FILE *p = fopen("foo", "r");
-  fseek(p, 1, SEEK_SET); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
+void check_fwrite() {
+  FILE *fp = tmpfile();
+  fwrite(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
 }
 
-void f3(void) {
-  FILE *p = fopen("foo", "r");
-  ftell(p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
+void check_fseek() {
+  FILE *fp = tmpfile();
+  fseek(fp, 0, 0); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_ftell() {
+  FILE *fp = tmpfile();
+  ftell(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_rewind() {
+  FILE *fp = tmpfile();
+  rewind(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_fgetpos() {
+  FILE *fp = tmpfile();
+  fpos_t pos;
+  fgetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_fsetpos() {
+  FILE *fp = tmpfile();
+  fpos_t pos;
+  fsetpos(fp, &pos); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_clearerr() {
+  FILE *fp = tmpfile();
+  clearerr(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_feof() {
+  FILE *fp = tmpfile();
+  feof(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void check_ferror() {
+  FILE *fp = tmpfile();
+  ferror(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
 }
 
-void f4(void) {
+void check_fileno() {
+  FILE *fp = tmpfile();
+  fileno(fp); // expected-warning {{Stream pointer might be NULL}}
+  fclose(fp);
+}
+
+void f_open(void) {
   FILE *p = fopen("foo", "r");
-  rewind(p); // expected-warning {{Stream pointer might be NULL}}
+  char buf[1024];
+  fread(buf, 1, 1, p); // expected-warning {{Stream pointer might be NULL}}
   fclose(p);
 }
 
-void f5(void) {
+void f_seek(void) {
   FILE *p = fopen("foo", "r");
   if (!p)
 return;
@@ -47,26 +105,20 @@
   fclose(p);
 }
 
-void f6(void) {
+void f_double_close(void) {
   FILE *p = fopen("foo", "r");
   fclose(p); 
   fclose(p); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
 }
 
-void f7(void) {
-  FILE *p = tmpfile();
-  ftell(p); // expected-warning {{Stream pointer might be NULL}}
-  fclose(p);
-}
-
-

[clang] 2573798 - Build fix after 4b6597f

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2019-10-31T07:43:09-04:00
New Revision: 257379855af245abb6c0fc11331d3f45ab0656dd

URL: 
https://github.com/llvm/llvm-project/commit/257379855af245abb6c0fc11331d3f45ab0656dd
DIFF: 
https://github.com/llvm/llvm-project/commit/257379855af245abb6c0fc11331d3f45ab0656dd.diff

LOG: Build fix after 4b6597f

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index cb76fb0da18a..e15f089b0d7e 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -737,7 +737,7 @@ Darwin::Darwin(const Driver &D, const llvm::Triple &Triple, 
const ArgList &Args)
   CudaInstallation(D, Triple, Args) {}
 
 types::ID MachO::LookupTypeForExtension(StringRef Ext) const {
-  types::ID Ty = ToolChain::lookupTypeForExtension(Ext);
+  types::ID Ty = ToolChain::LookupTypeForExtension(Ext);
 
   // Darwin always preprocesses assembly files (unless -x is used explicitly).
   if (Ty == types::TY_PP_Asm)



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

[PATCH] D69162: [clangd] Remove using-namespace present inside a namespace.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 227267.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69162

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

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -789,15 +789,21 @@
 map m;
   }
 )cpp"},
-  {// Available only for top level namespace decl.
+  {// FIXME: Does not remove usages outside the enclosing NS.
R"cpp(
-namespace aa {
-  namespace bb { struct map {}; }
-  using namespace b^b;
-}
-int main() { aa::map m; }
+  namespace aa {
+  namespace bb { struct map {}; }
+  using namespace b^b;
+  }
+  int main() { aa::map m; }
 )cpp",
-   "unavailable"},
+   R"cpp(
+  namespace aa {
+  namespace bb { struct map {}; }
+  
+  }
+  int main() { aa::map m; }
+)cpp"},
   {// FIXME: Unavailable for namespaces containing using-namespace decl.
R"cpp(
   namespace aa {
@@ -875,7 +881,116 @@
   int main() {
 std::vector V;
   }
-)cpp"}};
+)cpp"},
+  {// using inner namespaces.
+   R"cpp(
+  namespace A { namespace B { 
+  inline namespace ns1 { namespace std { struct vector {}; } }
+  using namespace st^d;
+  using namespace A::B::std;
+  using namespace B::std;
+  void func() { 
+vector V1;
+B::vector V2;
+A::B::vector V3;
+A::B::std::vector V4;  // Already qualified with TargetNS. 
+::A::B::std::vector V5;
+::A::B::ns1::std::vector V6;
+  }
+  }}
+)cpp",
+   R"cpp(
+  namespace A { namespace B { 
+  inline namespace ns1 { namespace std { struct vector {}; } }
+  
+  
+  
+  void func() { 
+std::vector V1;
+std::vector V2;
+std::vector V3;
+A::B::std::vector V4;  // Already qualified with TargetNS. 
+::A::B::std::vector V5;
+::A::B::ns1::std::vector V6;
+  }
+  }}
+)cpp"},
+  {// using outer namespaces.
+   R"cpp(
+  namespace std { struct vector {}; }
+  namespace A { namespace B { 
+  using namespace st^d;
+  void func() { 
+vector V1;
+B::vector V2;
+A::B::vector V3;
+::A::B::vector V4;
+  }
+  }}
+)cpp",
+   R"cpp(
+  namespace std { struct vector {}; }
+  namespace A { namespace B { 
+  
+  void func() { 
+std::vector V1;
+std::vector V2;
+std::vector V3;
+std::vector V4;
+  }
+  }}
+)cpp"},
+  {// Redecls of a namespace.
+   R"cpp(
+  namespace std { struct vector {}; }
+  namespace A { namespace B { using namespace st^d; }}
+  namespace A { namespace B { int func() { vector V1;}}}
+)cpp",
+   R"cpp(
+  namespace std { struct vector {}; }
+  namespace A { namespace B {  }}
+  namespace A { namespace B { int func() { std::vector V1;}}}
+)cpp"},
+  {// using namespace for outer and inner namespaces: remove inner.
+   R"cpp(
+  namespace out { namespace std { struct vector{}; }}
+  namespace A {
+  using namespace out;
+  using namespace st^d;
+  void func() { vector V;}
+  }
+)cpp",
+   R"cpp(
+  namespace out { namespace std { struct vector{}; }}
+  namespace A {
+  using namespace out;
+  
+  void func() { std::vector V;}
+  }
+)cpp"},
+  {// using namespace for outer and inner namespaces: remove outer.
+   R"cpp(
+  namespace out { namespace std { struct vector{}; }}
+  namespace A {
+  using namespace ou^t;
+  using namespace std;
+  void func() { vector V;}
+  }
+)cpp",
+   R"cpp(
+  namespace out { namespace std { struct vector{}; }}
+  namespace A {
+  
+  using namespace out::std;
+  void func() { vector V;}
+  }
+)cpp"},
+  {// Not available for contexts that are not global or namespaces.
+   R"cpp(
+  namespace std { struct vector {}; }
+  int main() { if(true) { using namespace st^d;}}
+)cpp",
+   "unavailable"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -18,6 +18,7 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Refactoring/RecursiveS

[PATCH] D69657: [AArch64][SVE] Implement several floating-point arithmetic intrinsics

[Sander de Smalen via Phabricator via cfe-commits]

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

Thanks @kmclaughlin, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69657



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

[PATCH] D69162: [clangd] Remove using-namespace present inside a namespace.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 marked 3 inline comments as done.
usaxena95 added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:179
+
+  if (ContainingNS) {
+for (auto ReDeclNS : ContainingNS->redecls())

ilya-biryukov wrote:
> Could you explain why don't we always just run across the whole TU?
> What disadvantages would that have?
Working with a using decl inside the `ContainingNS` is quite similar to our 
initial version with GlobalNS. 
For example we take advantage of that to replace the whole qualifier with just 
TargetNS. We use the fact that since `using namespace TargetNS;` was valid 
inside `ContainingNS` then qualifying the reference with just `TargetNS` would 
also be valid inside `ContainingNS`.

When we are outside `ContainingNS`,  we would have to fully qualify the ref to 
be 100% correct. To reduce the qualification we might have to figure out which 
is enclosing NS for the reference.

```
namespace aa {
namespace bb { 
struct map {}; 
}
using namespace b^b;
}
int main() { 
 aa::map m1;  // We do not qualify this currently.
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69162



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1388
   // results in arbitrary failures as function body becomes NULL.
   ExtraArgs.push_back("-fno-delayed-template-parsing");
+  for (const auto &Case : Cases)

You might need something like this in some of the new tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This is failing on Windows: http://45.33.8.238/win/1481/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D69162: [clangd] Remove using-namespace present inside a namespace.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: fail - 33521 tests passed, 1 failed and 463 were skipped.

  failed: LLVM.tools/llvm-objdump/X86/disassemble-functions.test

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69162



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

[clang-tools-extra] 1c66d09 - [clangd] Add fno-delayed-parsing to new define inline tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-31T13:22:52+01:00
New Revision: 1c66d09b739a8d9717ba4e9507649bc45ddf7f0d

URL: 
https://github.com/llvm/llvm-project/commit/1c66d09b739a8d9717ba4e9507649bc45ddf7f0d
DIFF: 
https://github.com/llvm/llvm-project/commit/1c66d09b739a8d9717ba4e9507649bc45ddf7f0d.diff

LOG: [clangd] Add fno-delayed-parsing to new define inline tests

To unbreak windows buildbots.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 126aab61ada5..5a6df2e03e67 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1529,6 +1529,7 @@ est);
 void foo(PARAM, TYPE b, TYPE c, TYPE d = BODY(x)){}
 )cpp"},
   };
+  ExtraArgs.push_back("-fno-delayed-template-parsing");
   for (const auto &Case : Cases)
 EXPECT_EQ(apply(Case.first), Case.second) << Case.first;
 }
@@ -1559,6 +1560,7 @@ TEST_F(DefineInlineTest, TransformTemplParamNames) {
 };
 
 )cpp";
+  ExtraArgs.push_back("-fno-delayed-template-parsing");
   EXPECT_EQ(apply(Test), Expected);
 }
 



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

Re: [PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Kadir Çetinkaya via cfe-commits]

done. sorry for forgetting about this so soon.

On Thu, Oct 31, 2019 at 12:58 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added inline comments.
>
>
> 
> Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1388
>// results in arbitrary failures as function body becomes NULL.
>ExtraArgs.push_back("-fno-delayed-template-parsing");
> +  for (const auto &Case : Cases)
> 
> You might need something like this in some of the new tests.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68937/new/
>
> https://reviews.llvm.org/D68937
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 733777a - [clangd] Fix namespace aliases in findExplicitReferences

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2019-10-31T13:35:25+01:00
New Revision: 733777a81662c40960e9298bb59da8c39a14f8d5

URL: 
https://github.com/llvm/llvm-project/commit/733777a81662c40960e9298bb59da8c39a14f8d5
DIFF: 
https://github.com/llvm/llvm-project/commit/733777a81662c40960e9298bb59da8c39a14f8d5.diff

LOG: [clangd] Fix namespace aliases in findExplicitReferences

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: merge_guards_bot, MaskRay, jkorous, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 1ab80b72a90b..4e61d22dd7fb 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@ class ExplicitReferenceColletor
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  DynTypedNode::create(*NNSL->getNestedNameSpecifier()),
+  DeclRelation::Alias)}};
+}
 if (const TypeLoc *TL = N.get())
   return refInTypeLoc(*TL);
 if (const CXXCtorInitializer *CCI = N.get()) {

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 6b0e51b228b8..8f7c7aaeaefe 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@ TEST_F(FindExplicitReferencesTest, All) {
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto &C : Cases) {



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

[PATCH] D69624: [clangd] Fix namespace aliases in findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ilya-biryukov marked an inline comment as done.
Closed by commit rG733777a81662: [clangd] Fix namespace aliases in 
findExplicitReferences (authored by ilya-biryukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69624

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  DynTypedNode::create(*NNSL->getNestedNameSpecifier()),
+  DeclRelation::Alias)}};
+}
 if (const TypeLoc *TL = N.get())
   return refInTypeLoc(*TL);
 if (const CXXCtorInitializer *CCI = N.get()) {


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -882,6 +882,28 @@
   "0: targets = {x}, decl\n"
   "1: targets = {fptr}, decl\n"
   "2: targets = {a}, decl\n"},
+  // Namespace aliases should be handled properly.
+  {
+  R"cpp(
+namespace ns { struct Type {} }
+namespace alias = ns;
+namespace rec_alias = alias;
+
+void foo() {
+  $0^ns::$1^Type $2^a;
+  $3^alias::$4^Type $5^b;
+  $6^rec_alias::$7^Type $8^c;
+}
+   )cpp",
+  "0: targets = {ns}\n"
+  "1: targets = {ns::Type}, qualifier = 'ns::'\n"
+  "2: targets = {a}, decl\n"
+  "3: targets = {alias}\n"
+  "4: targets = {ns::Type}, qualifier = 'alias::'\n"
+  "5: targets = {b}, decl\n"
+  "6: targets = {rec_alias}\n"
+  "7: targets = {ns::Type}, qualifier = 'rec_alias::'\n"
+  "8: targets = {c}, decl\n"},
   };
 
   for (const auto &C : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -674,10 +674,14 @@
   return refInDecl(D);
 if (auto *E = N.get())
   return refInExpr(E);
-if (auto *NNSL = N.get())
-  return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
-   explicitReferenceTargets(DynTypedNode::create(
-   *NNSL->getNestedNameSpecifier()))}};
+if (auto *NNSL = N.get()) {
+  // (!) 'DeclRelation::Alias' ensures we do not loose namespace aliases.
+  return {ReferenceLoc{
+  NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
+  explicitReferenceTargets(
+  

[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks, mostly LG!

The only important comment I have left is about limiting the number of 
references. Others are NITs.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:219
+  // us whether the ref results is completed.
+  RQuest.Limit = 100;
+  if (auto ID = getSymbolID(RenameDecl))

Why do we need to limit the number of references in the first place?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:223
+
+  // Helper.
+  auto ToRange = [](const SymbolLocation &L) {

Maybe remove this comment?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:224
+  // Helper.
+  auto ToRange = [](const SymbolLocation &L) {
+Range R;

Why not extract this as a helper function?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:233
+  llvm::StringMap>
+  AffectedFiles; // absolute file path => rename ocurrences in that file.
+  Index->refs(RQuest, [&](const Ref &R) {

NIT: maybe put this comment on the line before the declaration?
Should lead to better formatting


Also, this comment would be most useful on the function declaration. Maybe move 
it there?



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+llvm::Expected
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+  llvm::StringRef NewName, const SymbolIndex *Index,

NIT: Maybe call it `renameWithIndex` instead?
It should capture what this function is doing better...

But up to you



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:289
+
+  // The cross-file rename is purely based on the index, as we don't want to
+  // build all ASTs for affected files, which may cause a performance hit.

Maybe move this comment to the function itself?
It definitely does a great job of capturing what this function is doing and 
what the trade-offs are.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:277
+  //   - file on VFS for bar.cc;
+  Annotations MainCode("class  [[Fo^o]] {};");
+  auto MainFilePath = testPath("main.cc");

hokein wrote:
> ilya-biryukov wrote:
> > Thinking whether we can encode these transformations in a nicer way?
> > If we could convert the contents dirty buffer and replacements to something 
> > like:
> > ```
> > class [[Foo |-> newName]] {};
> > ```
> > Just a proposal, maybe there's a better syntax here.
> > 
> > We can easily match strings instead of matching ranges in the tests. This 
> > has the advantage of having textual diffs in case something goes wrong - 
> > much more pleasant than looking at the offsets in the ranges.
> > 
> > WDYT?
> I agree textual diff would give a more friendly results when there are 
> failures in the tests.
> 
> I don't have a better way to encode the transformation in the annotation 
> code, I think a better way maybe to use a hard-coded new name, and applying 
> the actual replacements on the testing code, and verify the the text diffs.
> 
> If we want to do this change, I'd prefer to do it in a followup, since it 
> would change the existing testcases as well. What do you think?
> 
Maybe I'm overthinking it... For rename having just the range is probably 
enough.



Comment at: clang-tools-extra/clangd/unittests/SyncAPI.cpp:103
+  Server.rename(File, Pos, NewName, /*WantFormat=*/true,
+/*DirtyBuffaer*/ nullptr, capture(Result));
   return std::move(*Result);

s/DirtyBuffaer/DirtyBuffer

also use `/*DirtryBuffer=*/` to be consistent with `/*WantFormat=*/` from the 
previous line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D68937: [clangd] Add parameter renaming to define-inline code action

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Tests are happy again after 1c66d09 
 , thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68937



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

[PATCH] D69628: [Clang] Pragma vectorize_width() implies vectorize(enable), take 3

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 227273.
SjoerdMeijer added a comment.

Thanks Michael!

- moved the handling of vectorize.enable to 1 place,
- that should have also sorted the relative ordering, and duplication of the 
metadata in some cases,
- created a separate file for the new tests, and added some more tests (also 
making sure there are no duplicates).


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

https://reviews.llvm.org/D69628

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp

Index: clang/test/CodeGenCXX/pragma-loop.cpp
===
--- clang/test/CodeGenCXX/pragma-loop.cpp
+++ clang/test/CodeGenCXX/pragma-loop.cpp
@@ -161,20 +161,20 @@
 // CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}
 // CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
 
-// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}
+// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE:.*]]}
 // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
 // CHECK: ![[DISTRIBUTE_DISABLE]] = !{!"llvm.loop.distribute.enable", i1 false}
 // CHECK: ![[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}
 // CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}
+// CHECK: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
 
-// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[INTENABLE_1:.*]], ![[FOLLOWUP_VECTOR_3:.*]]}
-// CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3:.*]]}
 // CHECK: ![[FOLLOWUP_VECTOR_3]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_3:.*]]}
 // CHECK: ![[AFTER_VECTOR_3]] = distinct !{![[AFTER_VECTOR_3]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 // CHECK: ![[ISVECTORIZED]] = !{!"llvm.loop.isvectorized"}
 // CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}
 
-// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}
+// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]], ![[VECTORIZE_ENABLE]]}
 // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
 
@@ -185,7 +185,7 @@
 // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_6:.*]]}
 // CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 
-// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]]}
+// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]], ![[VECTORIZE_ENABLE]]}
 // CHECK: ![[WIDTH_5]] = !{!"llvm.loop.vectorize.width", i32 5}
 
 // CHECK: ![[LOOP_8]] = distinct !{![[LOOP_8]], ![[WIDTH_5:.*]]}
@@ -207,11 +207,11 @@
 // CHECK: ![[AFTER_VECTOR_12]] = distinct !{![[AFTER_VECTOR_12]], ![[ISVECTORIZED:.*]], ![[UNROLL_24:.*]]}
 // CHECK: ![[UNROLL_24]] = !{!"llvm.loop.unroll.count", i32 24}
 
-// CHECK: ![[LOOP_13]] = distinct !{![[LOOP_13]], ![[WIDTH_8:.*]], ![[INTERLEAVE_16:.*]], ![[FOLLOWUP_VECTOR_13:.*]]}
+// CHECK: ![[LOOP_13]] = distinct !{![[LOOP_13]], ![[WIDTH_8:.*]], ![[INTERLEAVE_16:.*]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_13:.*]]}
 // CHECK: ![[INTERLEAVE_16]] = !{!"llvm.loop.interleave.count", i32 16}
 // CHECK: ![[FOLLOWUP_VECTOR_13]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_13:.*]]}
 // CHECK: ![[AFTER_VECTOR_13]] = distinct !{![[AFTER_VECTOR_13]], ![[ISVECTORIZED:.*]], ![[UNROLL_32:.*]]}
 // CHECK: ![[UNROLL_32]] = !{!"llvm.loop.unroll.count", i32 32}
 
-// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]]}
+// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]], ![[VECTORIZE_ENABLE]]}
 // CHECK: ![[WIDTH_10]] = !{!"llvm.loop.vectorize.width", i32 10}
Index: clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void loop1(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}loop1{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP1:.*]]
+
+  #pragma clang loop vectorize(enable) vectorize_width(1)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+// Here, vectorize.enable should be set, obviously, but also check that
+// metadata isn't added twice.
+void loop2(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}loop2{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP2:.*]]
+
+  #pragma clang loop vectorize(enable) vectorize_width(2)
+  for (int i 

[PATCH] D69316: [OpenMP 5.0] target update list items need not be contiguous (Sema)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

What about codegen tests?




Comment at: clang/test/OpenMP/target_update_ast_print.cpp:25
+  U marr[10][10][10];
+#pragma omp target update to(marr[2][0:2][0:2])
+

It seems to me, the tests are not updated. All non-contiguous tests must pass 
only if OpenMP version is 50


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69316



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

[PATCH] D69590: [RISCV] Fix ILP32D lowering for double+double/double+int return types

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Thanks James - won't this still leave problems for structs that need flattening?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69590



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:219
+  // us whether the ref results is completed.
+  RQuest.Limit = 100;
+  if (auto ID = getSymbolID(RenameDecl))

ilya-biryukov wrote:
> Why do we need to limit the number of references in the first place?
I was thinking this is for performance, if the index returns too many results 
(the number of affected files >> our limit), we will ends up doing many 
unnecessary work (like resolving URIs).  Removed it now, it seems a premature 
optimization, we can revisit this afterwards.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+llvm::Expected
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+  llvm::StringRef NewName, const SymbolIndex *Index,

ilya-biryukov wrote:
> NIT: Maybe call it `renameWithIndex` instead?
> It should capture what this function is doing better...
> 
> But up to you
I would keep the current name, as it is symmetrical to the `renameWithinFile`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> Did you see some significant perf improvements for Chromium?

I don't see major improvements in browser benchmarks like Speedometer, but in 
the Blink garbage collector, which queues destructors to be executed later, 
this change proved to reduce the number of them by 5%. That, in turn, reduces 
GC time on the main thread which is significant for latency.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Thanks. Would you mind committing this on my behalf? It seems I wasn't migrated 
from SVN access to git access. It may take some time to sort out.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69649



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 227278.
hokein marked 8 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -38,8 +38,8 @@
 llvm::Expected>
 runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
 
-llvm::Expected>
-runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, StringRef NewName);
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -96,11 +96,11 @@
   return std::move(*Result);
 }
 
-llvm::Expected> runRename(ClangdServer &Server,
-PathRef File, Position Pos,
-llvm::StringRef NewName) {
-  llvm::Optional>> Result;
-  Server.rename(File, Pos, NewName, /*WantFormat=*/true, capture(Result));
+llvm::Expected runRename(ClangdServer &Server, PathRef File,
+Position Pos, llvm::StringRef NewName) {
+  llvm::Optional> Result;
+  Server.rename(File, Pos, NewName, /*WantFormat=*/true,
+/*DirtyBuffer=*/nullptr, capture(Result));
   return std::move(*Result);
 }
 
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -9,8 +9,10 @@
 #include "Annotations.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/Ref.h"
 #include "refactor/Rename.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -18,9 +20,68 @@
 namespace clangd {
 namespace {
 
+using testing::Eq;
+using testing::Matches;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
 MATCHER_P2(RenameRange, Code, Range, "") {
   return replacementToEdit(Code, arg).range == Range;
 }
+MATCHER_P2(EqualFileEdit, Code, Ranges, "") {
+  using ReplacementMatcher = testing::Matcher;
+  std::vector Expected;
+  for (const auto &R : Ranges)
+Expected.push_back(RenameRange(Code, R));
+  auto Matcher =
+  testing::internal::UnorderedElementsAreArrayMatcher(
+  Expected.begin(), Expected.end());
+  return arg.InitialCode == Code && Matches(Matcher)(arg.Replacements);
+}
+
+std::unique_ptr buildRefSlab(const Annotations &Code,
+  llvm::StringRef SymbolName,
+  llvm::StringRef Path) {
+  RefSlab::Builder Builder;
+  TestTU TU;
+  TU.HeaderCode = Code.code();
+  auto Symbols = TU.headerSymbols();
+  const auto &SymbolID = findSymbol(Symbols, SymbolName).ID;
+  for (const auto &Range : Code.ranges()) {
+Ref R;
+R.Kind = RefKind::Reference;
+R.Location.Start.setLine(Range.start.line);
+R.Location.Start.setColumn(Range.start.character);
+R.Location.End.setLine(Range.end.line);
+R.Location.End.setColumn(Range.end.character);
+auto U = URI::create(Path).toString();
+R.Location.FileURI = U.c_str();
+Builder.insert(SymbolID, R);
+  }
+
+  return std::make_unique(std::move(Builder).build());
+}
+
+RenameInputs renameInputs(const Annotations &Code, llvm::StringRef NewName,
+  llvm::StringRef Path,
+  const SymbolIndex *Index = nullptr,
+  bool CrossFile = false) {
+  RenameInputs Inputs;
+  Inputs.Pos = Code.point();
+  Inputs.MainFileCode = Code.code();
+  Inputs.MainFilePath = Path;
+  Inputs.NewName = NewName;
+  Inputs.Index = Index;
+  Inputs.AllowCrossFile = CrossFile;
+  return Inputs;
+}
+
+std::vector> toVector(FileEdits FE) {
+  std::vector> Results;
+  for (auto &It : FE)
+Results.emplace_back(It.first().str(), std::move(It.getValue()));
+  return Results;
+}
 
 TEST(R

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Thanks!


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

https://reviews.llvm.org/D69435



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59816 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a project: clang.

Use of evalCall is replaced by preCall and postCall.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69662

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -49,22 +49,21 @@
   }
 };
 
-class StreamChecker : public Checker {
+class StreamChecker
+: public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
-  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
 private:
   using FnCheck = std::function;
 
-  CallDescriptionMap Callbacks = {
-  {{"fopen"}, &StreamChecker::evalFopen},
-  {{"tmpfile"}, &StreamChecker::evalFopen},
+  CallDescriptionMap PreCallbacks = {
   {{"fclose", 1}, &StreamChecker::evalFclose},
   {{"fread", 4},
std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)},
@@ -89,10 +88,17 @@
std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)},
   };
 
+  CallDescriptionMap PostCallbacks = {
+  {{"fopen"}, &StreamChecker::evalFopen},
+  {{"tmpfile"}, &StreamChecker::evalFopen},
+  };
+
   void evalFopen(const CallEvent &Call, CheckerContext &C) const;
   void evalFclose(const CallEvent &Call, CheckerContext &C) const;
   void evalFseek(const CallEvent &Call, CheckerContext &C) const;
 
+  void checkCall(const CallEvent &Call, CheckerContext &C,
+ const CallDescriptionMap &Callbacks) const;
   void checkArgNullStream(const CallEvent &Call, CheckerContext &C,
   unsigned ArgI) const;
   bool checkNullStream(SVal SV, CheckerContext &C,
@@ -107,29 +113,38 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
 
+void StreamChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+  checkCall(Call, C, PreCallbacks);
+}
+
+void StreamChecker::checkPostCall(const CallEvent &Call,
+  CheckerContext &C) const {
+  checkCall(Call, C, PostCallbacks);
+}
 
-bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+void StreamChecker::checkCall(
+const CallEvent &Call, CheckerContext &C,
+const CallDescriptionMap &Callbacks) const {
   const auto *FD = dyn_cast_or_null(Call.getDecl());
   if (!FD || FD->getKind() != Decl::Function)
-return false;
+return;
 
   // Recognize "global C functions" with only integral or pointer arguments
   // (and matching name) as stream functions.
   if (!Call.isGlobalCFunction())
-return false;
+return;
   for (auto P : Call.parameters()) {
 QualType T = P->getType();
 if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
-  return false;
+  return;
   }
 
   const FnCheck *Callback = Callbacks.lookup(Call);
   if (!Callback)
-return false;
+return;
 
   (*Callback)(this, Call, C);
-
-  return C.isDifferent();
 }
 
 void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const {


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -49,22 +49,21 @@
   }
 };
 
-class StreamChecker : public Checker {
+class StreamChecker
+: public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
-  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
 private:
   using FnCheck = std::function;
 
-  CallDescriptionMap Callbacks = {
-  {{"fopen"}, &StreamChecker::evalFopen},
-  {{"tmpfile"}, &StreamChecker::evalFopen},
+  CallDescriptionMap PreCallbacks = {
   {{"fclose", 1}, &StreamChecker::evalFclose},
   {{"fread", 4},
std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)},
@@ -89,10 +88,17 @@
std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)},
   };
 
+  CallDescriptionMap PostCallbacks = {
+  {{"fopen"}, &StreamChecker::evalFopen},
+  {{"tmpfile"}, &StreamChecker::evalFopen},
+  };
+
   void evalFopen(const CallEvent &Call, CheckerContext &C) const;
   void evalFclose(const C

[clang] 8d7bd57 - [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell Balan via cfe-commits]

Author: Mitchell Balan
Date: 2019-10-31T11:08:05-04:00
New Revision: 8d7bd57526486cab9e3daba9934042c405d7946b

URL: 
https://github.com/llvm/llvm-project/commit/8d7bd57526486cab9e3daba9934042c405d7946b
DIFF: 
https://github.com/llvm/llvm-project/commit/8d7bd57526486cab9e3daba9934042c405d7946b.diff

LOG: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" 
Parameter

Summary:
This fixes an edge case in the `SpacesInSquareBrackets` option where an initial 
`&ref` lambda parameter is not padded with an initial space.

`int foo = [&bar ]() {}` is fixed to give `int foo = [ &bar ]() {}`

Reviewers: MyDeveloperDay, klimek, sammccall

Reviewed by: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 1ed35597d075..98b87c0f5a25 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine &Line,
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index e0ebef1f7ced..ea03ee1c3cc9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@ TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) 
{
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {



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

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d7bd5752648: [clang-format] Fix SpacesInSquareBrackets for 
Lambdas with Initial "&ref"… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69649

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous &&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

BTW, this very contrived benchmark shows 5x: 
http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew


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

https://reviews.llvm.org/D69435



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

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Never mind, I got it resolved and pushed. Sorry for the noise.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69649



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Sam Elliott via Phabricator via cfe-commits]

lenary updated this revision to Diff 227284.
lenary added a comment.

- More conservative march/mabi defaults on riscv-unknown-elf
- Add clang release notes about this change to -march/-mabi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-gnutools.c

Index: clang/test/Driver/riscv-gnutools.c
===
--- clang/test/Driver/riscv-gnutools.c
+++ clang/test/Driver/riscv-gnutools.c
@@ -1,19 +1,19 @@
 // Check gnutools are invoked with propagated values for -mabi and -march.
 
 // RUN: %clang -target riscv32 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D %s
 
 // RUN: %clang -target riscv32 -fno-integrated-as -march=rv32g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32D-MARCH-G %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as %s -###  -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64 %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D %s
 
 // RUN: %clang -target riscv64 -fno-integrated-as -march=rv64g %s -### -c \
-// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64-MARCH-G %s
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP64D-MARCH-G %s
 
-// MABI-ILP32: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32"
-// MABI-ILP32-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-ILP32D: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32gc"
+// MABI-ILP32D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "ilp32d" "-march" "rv32g"
 
-// MABI-ILP64: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64"
-// MABI-ILP64-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64" "-march" "rv64g"
+// MABI-ILP64D: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d"
+// MABI-ILP64D-MARCH-G: "{{.*}}as{{(.exe)?}}" "-mabi" "lp64d" "-march" "rv64g"
Index: clang/test/Driver/riscv-abi.c
===
--- clang/test/Driver/riscv-abi.c
+++ clang/test/Driver/riscv-abi.c
@@ -1,9 +1,5 @@
-// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
-// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
@@ -14,6 +10,10 @@
 
 // CHECK-ILP32F: "-target-abi" "ilp32f"
 
+// RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -march=rv32ifd -mabi=ilp32d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32D %s
 
@@ -24,12 +24,8 @@
 
 // CHECK-RV32-LP64: error: unknown target ABI 'lp64'
 
-// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
-// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
 // RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
 // RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
@@ -40,6 +36,10 @@
 
 // CHECK-LP64F: "-target-abi" "lp64f"
 
+// RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64D  %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -march=rv64d -mabi=lp64d 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64D %s
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -709,11 +709,9 @@
 StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
 CmdArgs.push_back("-mabi");
 CmdArgs.push_back(ABIName.data());
-if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-  StringRef MArch = A->getValue();
-  CmdArgs.push_back("-march");
-  CmdArgs.push_back(MArch.data());
-}
+StringRef MArchName = riscv::getR

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 227283.
mibintc added a comment.

I followed up on some code review remarks from @rjmccall. I dropped the CODEGEN 
option and fixed some code formatting. I changed the spelling of the 
enumeration values for RoundingMode and ExceptionMode to match those proposed 
in https://reviews.llvm.org/D65994


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fast-math.c
  clang/test/Driver/fp-model.c
  llvm/include/llvm/Target/TargetOptions.h

Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -107,7 +107,7 @@
   public:
 TargetOptions()
 : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
-  NoNaNsFPMath(false), NoTrappingFPMath(false),
+  NoNaNsFPMath(false), NoTrappingFPMath(true),
   NoSignedZerosFPMath(false),
   HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
   GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
Index: clang/test/Driver/fp-model.c
===
--- /dev/null
+++ clang/test/Driver/fp-model.c
@@ -0,0 +1,130 @@
+// Test that incompatible combinations of -ffp-model= options
+// and other floating point options get a warning diagnostic.
+//
+// REQUIRES: clang-driver
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN %s
+// WARN: warning: overriding '-ffp-model=fast' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN1 %s
+// WARN1: warning: overriding '-ffp-model=fast' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fassociative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN2 %s
+// WARN2: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN3 %s
+// WARN3: warning: overriding '-ffp-model=strict' option with '-ffast-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffinite-math-only -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN4 %s
+// WARN4: warning: overriding '-ffp-model=strict' option with '-ffinite-math-only' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN5 %s
+// WARN5: warning: overriding '-ffp-model=strict' option with '-ffp-contract=fast' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN6 %s
+// WARN6: warning: overriding '-ffp-model=strict' option with '-ffp-contract=off' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN7 %s
+// WARN7: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-infinities -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN8 %s
+// WARN8: warning: overriding '-ffp-model=strict' option with '-fno-honor-infinities' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-honor-nans -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN9 %s
+// WARN9: warning: overriding '-ffp-model=strict' option with '-fno-honor-nans' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-rounding-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNa %s
+// WARNa: warning: overriding '-ffp-model=strict' option with '-fno-rounding-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-signed-zeros -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNb %s
+// WARNb: warning: overriding '-ffp-model=strict' option with '-fno-signed-zeros' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -fno-trapping-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNc %s
+// WARNc: warning: overriding '-ffp-model=strict' option with '-fno-trapping-math' [-Woverriding-t-option]
+
+// RUN: %clang -### -ffp-model=strict -freciprocal-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARNd %s
+// WARNd: warning: overridin

[PATCH] D67508: [RISCV] support mutilib in baremetal environment

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I have just updated D69383 .

It now defaults to `rv{XLEN}imac` on ELF, rather than `rv{XLEN}gc`. This is to 
help this multilib issue.

In the future, we want to implement MULTILIB_REUSE properly. Hopefully before 
Clang 10.0.0 is released.


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

https://reviews.llvm.org/D67508



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

[PATCH] D69664: [Diagnostics] Teach -Wnull-dereference about address_space attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 created this revision.
xbolva00 added reviewers: aaron.ballman, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang should not warn for:
test.c:2:12: warning: indirection of non-volatile null pointer will be deleted,

not trap [-Wnull-dereference]
  return *(int __attribute__((address_space(256))) *) 0;
 ^~

Solves PR42292.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69664

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/exprs.c


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,13 @@
   test18_e(); // expected-error {{too few arguments to function call, expected 
at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_as_256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null 
pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null 
pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) 
&&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_through_null));
+}
 }
 
 static void DiagnoseDirectIsaAccess(Sema &S, const ObjCIvarRefExpr *OIRE,


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,13 @@
   test18_e(); // expected-error {{too few arguments to function call, expected at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_as_256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) &&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 5 inline comments as done.
mibintc added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:238
 CODEGENOPT(UnsafeFPMath  , 1, 0) ///< Allow unsafe floating point optzns.
+CODEGENOPT(RoundingFPMath, 1, 0) ///< Rounding floating point optzns.
 CODEGENOPT(UnwindTables  , 1, 0) ///< Emit unwind tables.

mibintc wrote:
> rjmccall wrote:
> > Why do we need both a code-gen option and a language option?
> The main reason i added it to LangOptions.h is because I saw the FPContract 
> support in there and I thought I'd get on that bandwagon.  My ultimate goal, 
> after committing the command line options, is to add support for controlling 
> rounding mode and exception behavior with pragma's embedded in the functions, 
> similar to https://reviews.llvm.org/D69272.  
> 
> There's a patch here that I like, to add rounding-mode and exception-behavior 
> to FPOptions https://reviews.llvm.org/D65994, but it hasn't been committed 
> yet.
> 
I dropped the code-gen option.



Comment at: clang/include/clang/Basic/LangOptions.h:366
+  FPEB = Value;
+}
+

rjmccall wrote:
> Everything here is a "setting", and in the context of this type they're all 
> FP.  Please name these methods something like `getRoundingMode()`.
> 
> Does this structure really need to exist as opposed to tracking the 
> dimensions separately?  Don't we already track some of this somewhere?  We 
> should subsume that state into these values rather than tracking them 
> separately.
I fixed the spelling, I also dropped the structure and used the ENUM_OPT macro 
instead of writing out the setter and getter. Look OK now?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D69664: [Diagnostics] Teach -Wnull-dereference about address_space attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 227286.
xbolva00 added a comment.

Added testcase with typedefed int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69664

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/exprs.c


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,14 @@
   test18_e(); // expected-error {{too few arguments to function call, expected 
at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_AS256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null 
pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
+   *(int_AS256*)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null 
pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) 
&&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_through_null));
+}
 }
 
 static void DiagnoseDirectIsaAccess(Sema &S, const ObjCIvarRefExpr *OIRE,


Index: clang/test/Sema/exprs.c
===
--- clang/test/Sema/exprs.c
+++ clang/test/Sema/exprs.c
@@ -179,11 +179,14 @@
   test18_e(); // expected-error {{too few arguments to function call, expected at least 2, have 0}}
 }
 
+typedef int __attribute__((address_space(256))) int_AS256;
 // PR7569
 void test19() {
   *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null pointer}} \
   // expected-note {{consider using __builtin_trap}}
   *(volatile int*)0 = 0;  // Ok.
+   *(int __attribute__((address_space(256))) *)0 = 0; // Ok.
+   *(int_AS256*)0 = 0; // Ok.
 
   // rdar://9269271
   int x = *(int*)0;  // expected-warning {{indirection of non-volatile null pointer}} \
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -483,15 +483,17 @@
   // only handles the pattern "*null", which is a very syntactic check.
   if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts()))
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) &&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&
+UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull) &&
 !UO->getType().isVolatileQualified()) {
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-  S.PDiag(diag::warn_indirection_through_null)
-<< UO->getSubExpr()->getSourceRange());
-S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
-S.PDiag(diag::note_indirection_through_null));
-  }
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::warn_indirection_through_null)
+<< UO->getSubExpr()->getSourceRange());
+  S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
+S.PDiag(diag::note_indirection_through_null));
+}
 }
 
 static void DiagnoseDirectIsaAccess(Sema &S, const ObjCIvarRefExpr *OIRE,
___
cfe-commits mailing list
cfe-commits@li

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D69435#1728674 , @AntonBikineev 
wrote:

> BTW, this very contrived benchmark shows 5x: 
> http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew


There is no difference in perf for GCC.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69664: [Diagnostics] Teach -Wnull-dereference about address_space attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:486
 if (UO->getOpcode() == UO_Deref &&
-UO->getSubExpr()->IgnoreParenCasts()->
-  isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull) 
&&
+!isTargetAddressSpace(
+UO->getSubExpr()->getType()->getPointeeType().getAddressSpace()) &&

With this patch, we no longer warn for:

*(int __attribute__((address_space(0))) *) 0;

Worth to handle this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69664



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

[PATCH] D69632: [libTooling] Add Stencil constructor.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69632



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

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, jdoerfert, Anastasia.
Herald added a subscriber: wdng.

This attempted to always use the default address space void pointer
type instead of preserving the source address space.


https://reviews.llvm.org/D69666

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/address-space.c


Index: clang/test/CodeGen/address-space.c
===
--- clang/test/CodeGen/address-space.c
+++ clang/test/CodeGen/address-space.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck 
-check-prefixes=CHECK,AMDGCN %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-enable-var-scope -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck 
-enable-var-scope -check-prefixes=CHECK,AMDGCN %s
 
 // CHECK: @foo = common addrspace(1) global
 int foo __attribute__((address_space(1)));
@@ -10,11 +10,11 @@
 // CHECK: @a = common global
 int a __attribute__((address_space(0)));
 
-// CHECK-LABEL: define i32 @test1() 
+// CHECK-LABEL: define i32 @test1()
 // CHECK: load i32, i32 addrspace(1)* @foo
 int test1() { return foo; }
 
-// CHECK-LABEL: define i32 @test2(i32 %i) 
+// CHECK-LABEL: define i32 @test2(i32 %i)
 // CHECK: load i32, i32 addrspace(1)*
 // CHECK-NEXT: ret i32
 int test2(int i) { return ban[i]; }
@@ -45,3 +45,17 @@
   MyStruct s = pPtr[0];
   pPtr[0] = s;
 }
+
+// Make sure the right address space is used when doing arithmetic on a void
+// pointer. Make sure no invalid bitcast is introduced.
+
+// CHECK-LABEL: @void_ptr_arithmetic_test(
+// X86: [[ALLOCA:%.*]] = alloca i8 addrspace(1)*
+// X86-NEXT: store i8 addrspace(1)* %arg, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: load i8 addrspace(1)*, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: getelementptr i8, i8 addrspace(1)*
+// X86-NEXT: ret i8 addrspace(1)*
+void __attribute__((address_space(1)))*
+void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
+return arg + 4;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3258,7 +3258,7 @@
   // GNU void* casts amount to no-ops since our void* type is i8*, but this is
   // future proof.
   if (elementType->isVoidType() || elementType->isFunctionType()) {
-Value *result = CGF.Builder.CreateBitCast(pointer, CGF.VoidPtrTy);
+Value *result = CGF.EmitCastToVoidPtr(pointer);
 result = CGF.Builder.CreateGEP(result, index, "add.ptr");
 return CGF.Builder.CreateBitCast(result, pointer->getType());
   }


Index: clang/test/CodeGen/address-space.c
===
--- clang/test/CodeGen/address-space.c
+++ clang/test/CodeGen/address-space.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck -check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck -check-prefixes=CHECK,AMDGCN %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck -enable-var-scope -check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -triple amdgcn -emit-llvm < %s | FileCheck -enable-var-scope -check-prefixes=CHECK,AMDGCN %s
 
 // CHECK: @foo = common addrspace(1) global
 int foo __attribute__((address_space(1)));
@@ -10,11 +10,11 @@
 // CHECK: @a = common global
 int a __attribute__((address_space(0)));
 
-// CHECK-LABEL: define i32 @test1() 
+// CHECK-LABEL: define i32 @test1()
 // CHECK: load i32, i32 addrspace(1)* @foo
 int test1() { return foo; }
 
-// CHECK-LABEL: define i32 @test2(i32 %i) 
+// CHECK-LABEL: define i32 @test2(i32 %i)
 // CHECK: load i32, i32 addrspace(1)*
 // CHECK-NEXT: ret i32
 int test2(int i) { return ban[i]; }
@@ -45,3 +45,17 @@
   MyStruct s = pPtr[0];
   pPtr[0] = s;
 }
+
+// Make sure the right address space is used when doing arithmetic on a void
+// pointer. Make sure no invalid bitcast is introduced.
+
+// CHECK-LABEL: @void_ptr_arithmetic_test(
+// X86: [[ALLOCA:%.*]] = alloca i8 addrspace(1)*
+// X86-NEXT: store i8 addrspace(1)* %arg, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: load i8 addrspace(1)*, i8 addrspace(1)** [[ALLOCA]]
+// X86-NEXT: getelementptr i8, i8 addrspace(1)*
+// X86-NEXT: ret i8 addrspace(1)*
+void __attribute__((address_space(1)))*
+void_ptr_arithmetic_test(void __attribute__((address_space(1))) *arg) {
+return arg + 4;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3258,7 +3258,7 @@
   // GNU void* casts amount to no-ops since our void* type is i8*, but this is
   // future proof.
   if (elementType->isVoidType() || elementType->is

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> There is no difference in perf for GCC.

Yes, but that's because gcc still optimizes the call to noinlined function. In 
the common scenario which this check addresses (destructor defaulted in .cpp 
file) gcc is not able to optimize the call.


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

https://reviews.llvm.org/D69435



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

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Yes, thanks, looks a lot better.  Just a few tweaks now.




Comment at: clang/include/clang/Basic/LangOptions.h:348
   VersionTuple getOpenCLVersionTuple() const;
+
 };

Spurious change.



Comment at: clang/include/clang/Driver/Options.td:1152
+def frounding_math : Flag<["-"], "frounding-math">, Group, 
Flags<[CC1Option]>;
+def fno_rounding_math : Flag<["-"], "fno-rounding-math">, Group, 
Flags<[CC1Option]>;
 def ftrapping_math : Flag<["-"], "ftrapping-math">, Group, 
Flags<[CC1Option]>;

It looks like both of these can now be written with `BooleanFFlag`.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:148
+llvm_unreachable("Unsupported FP Exception Behavior");
+  }
+

Please make functions that do these translations, and please make them use 
exhaustive switches with `llvm_unreachable` at the end.



Comment at: clang/test/Driver/clang_f_opts.c:323
 // RUN: -fprofile-values  \
-// RUN: -frounding-math   \
 // RUN: -fschedule-insns  \

Looks like the intent of this test is that you pull this to the lines above, to 
test that we don't emit an error on it.  You should also test `-ffp-model`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D69657: [AArch64][SVE] Implement several floating-point arithmetic intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin updated this revision to Diff 227299.
kmclaughlin added a comment.

- Removed duplicate //AdvSIMD_Pred2VectorArg_Intrinsic// class after rebase


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

https://reviews.llvm.org/D69657

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-fp-arith.ll
@@ -0,0 +1,530 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; FABD
+;
+
+define  @fabd_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_h:
+; CHECK: fabd z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fabd_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_s:
+; CHECK: fabd z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fabd_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fabd_d:
+; CHECK: fabd z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fabd.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FADD
+;
+
+define  @fadd_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_h:
+; CHECK: fadd z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fadd_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_s:
+; CHECK: fadd z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fadd_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fadd_d:
+; CHECK: fadd z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fadd.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FDIV
+;
+
+define  @fdiv_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_h:
+; CHECK: fdiv z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv8f16( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fdiv_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_s:
+; CHECK: fdiv z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv4f32( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fdiv_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fdiv_d:
+; CHECK: fdiv z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdiv.nxv2f64( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+;
+; FDIVR
+;
+
+define  @fdivr_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_h:
+; CHECK: fdivr z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv8f16( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fdivr_s( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_s:
+; CHECK: fdivr z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv4f32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @fdivr_d( %pg,  %a,  %b) {
+; CHECK-LABEL: fdivr_d:
+; CHECK: fdivr z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fdivr.nxv2f64( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+;
+; FMAX
+;
+
+define  @fmax_h( %pg,  %a,  %b) {
+; CHECK-LABEL: fmax_h:
+; CHECK: fmax z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmax.nxv8f16( %pg,
+  

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

The functionality looks acceptable.  Trying to parse the whole thing still 
looks fragile to me.  I expect code owner to take a look at this change.




Comment at: clang/lib/Format/TokenAnnotator.cpp:1371
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.

Maybe make use of some `TT_TemplateOpener`?



Comment at: clang/unittests/Format/FormatTest.cpp:4987
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> Afoo<1>)>;");

Does `A() -> A>)>` (C++11 `>>`) work?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69577



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

[PATCH] D69673: [clangd] Implement semantic highlightings via findExplicitReferences

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

To keep the logic of finding locations of interesting AST nodes in one
place.

The advantage is better coverage of various AST nodes, both now and in
the future: as new nodes get added to `findExplicitReferences`, semantic
highlighting will automatically pick them up.

The drawback of this change is that we have to traverse declarations
inside our file twice in order to highlight dependent names, 'auto'
and 'decltype'. Hopefully, this should not affect the actual latency
too much, most time should be spent in building the AST and not
traversing it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69673

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

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -589,6 +589,15 @@
   R"cpp(
   void $Function[[foo]]();
   using ::$Function[[foo]];
+)cpp",
+  // Highlighting of template template arguments.
+  R"cpp(
+  template  class $TemplateParameter[[TT]],
+template  class ...$TemplateParameter[[TTs]]>
+  struct $Class[[Foo]] {
+$Class[[Foo]]<$TemplateParameter[[TT]], $TemplateParameter[[TTs]]...>
+  *$Field[[t]];
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "SemanticHighlighting.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
@@ -15,10 +16,16 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Casting.h"
 #include 
 
 namespace clang {
@@ -103,12 +110,12 @@
 return kindForDecl(TD);
   return llvm::None;
 }
-// Given a set of candidate declarations, if the declarations all have the same
-// highlighting kind, return that highlighting kind, otherwise return None.
-template 
-llvm::Optional kindForCandidateDecls(IteratorRange Decls) {
+
+llvm::Optional kindForReference(const ReferenceLoc &R) {
   llvm::Optional Result;
-  for (NamedDecl *Decl : Decls) {
+  for (const NamedDecl *Decl : R.Targets) {
+if (!canHighlightName(Decl->getDeclName()))
+  return llvm::None;
 auto Kind = kindForDecl(Decl);
 if (!Kind || (Result && Kind != Result))
   return llvm::None;
@@ -117,27 +124,48 @@
   return Result;
 }
 
-// Collects all semantic tokens in an ASTContext.
-class HighlightingTokenCollector
-: public RecursiveASTVisitor {
-  std::vector Tokens;
-  ParsedAST &AST;
-
+/// Consumes source locations and maps them to text ranges for highlightings.
+class HighlightingsBuilder {
 public:
-  HighlightingTokenCollector(ParsedAST &AST) : AST(AST) {}
-
-  std::vector collectTokens() {
-Tokens.clear();
-TraverseAST(AST.getASTContext());
-// Add highlightings for macro expansions as they are not traversed by the
-// visitor.
-for (const auto &M : AST.getMacros().Ranges)
-  Tokens.push_back({HighlightingKind::Macro, M});
+  HighlightingsBuilder(const SourceManager &SM, const LangOptions &LO)
+  : SM(SM), LO(LO) {}
+
+  void addToken(HighlightingToken T) { Tokens.push_back(T); }
+
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+if (Loc.isInvalid())
+  return;
+if (Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
+
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from
+// another file for a class from another file).
+// There are also cases with macros where the spelling loc will not be in
+// the main file and the highlighting would be incorrect.
+if (!isInsideMainFile(Loc, SM))
+  

[PATCH] D69666: clang: Fix assert on void pointer arithmetic with address_space

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Is the description reversed?

This attempts to preserve the source address space instead of always using the 
default address space for void pointer type.


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

https://reviews.llvm.org/D69666



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

[PATCH] D69673: [clangd] Implement semantic highlightings via findExplicitReferences

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 59818 tests passed, 0 failed and 762 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69673



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

[PATCH] D69263: [clangd] Implement cross-file rename.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+llvm::Expected
+renameOutsideFile(const NamedDecl *RenameDecl, llvm::StringRef MainFilePath,
+  llvm::StringRef NewName, const SymbolIndex *Index,

hokein wrote:
> ilya-biryukov wrote:
> > NIT: Maybe call it `renameWithIndex` instead?
> > It should capture what this function is doing better...
> > 
> > But up to you
> I would keep the current name, as it is symmetrical to the `renameWithinFile`.
Yeah, those go together. Using `renameWithIndex` would mean the other one 
should be `renameWithAST` (or similar).
Feel free to keep as is too.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:216
+// Return all rename occurrences (per the index) outside of the main file,
+// grouped by the file name.
+llvm::StringMap>

NIT: grouped by the **absolute** file name? Or is there a better term than 
"absolute" to describe this?



Comment at: clang-tools-extra/clangd/refactor/Rename.h:23
 
+using DirtyBufferGetter =
+std::function(PathRef Path)>;

Could you document what does it mean for this function to return `llvm::None`?
I assume we read the contents from disk instead.

Also worth documenting what should be returned for `MainFilePath`? 
`llvm::None`? same value as `MainFileCode`?



Comment at: clang-tools-extra/clangd/refactor/Rename.h:24
+using DirtyBufferGetter =
+std::function(PathRef Path)>;
+

NIT: maybe use `function_ref`? We definitely don't store this function anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69263



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

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 requested changes to this revision.
gribozavr2 added inline comments.
This revision now requires changes to proceed.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:21
 
+const char DefaultStringNames[] = "basic_string";
+

poelmanc wrote:
> aaron.ballman wrote:
> > I think the default should probably be `::std::basic_string` to avoid 
> > getting other things named `basic_string`?
> The prior code had `basic_string` hard-coded at lines 27, 31, and 50 so I 
> initially set the default to `basic_string` just to keep the default behavior 
> unchanged.
> 
> I just now tried setting it to each of `std::basic_string`, 
> `::std::basic_string`, and even `std::string;std::wstring` and the 
> `readability-redundant-string-init.cpp` tests failed with any of those 
> settings, so I've left it set to `basic_string`.
I think we should change the checker behavior so that it checks against the 
fully-qualified name.

We are defining a user-settable option, and changing its behavior later to 
allow fully-qualified names could be difficult.

The reason why tests failed for you is because below the name "basic_string" is 
used as a method name (the name of the constructor). That checker should be 
rewritten to check the name of the type.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69548



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

[PATCH] D68923: Don't warn about missing declarations for partial template specializations

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Gentle ping.

I'm open to suggestions to simplify the condition, but I think templates, 
partial specializations and instantiations are three different things that we 
all need to exclude here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68923



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

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:67-70
+// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to
+// return CtorExpr->getSourceRange(). However, starting with c++17, parsing
+// the expression 'std::string Name = ""' results in a CtorExpr whose
+// SourceRange includes just '""' rather than the previous 'Name = ""'.

poelmanc wrote:
> aaron.ballman wrote:
> > Are you sure that this behavioral difference isn't just a bug in Clang? I 
> > can't think of why the source range should differ based on language mode, 
> > so I wonder if the correct thing to do is fix how we calculate the source 
> > range in Clang?
> Thanks and no, I'm not sure at all. At the end of my summary paragraph I 
> wrote:
> 
> > Or, is it possible that this change in SourceRange is an unintentional 
> > difference in the parsing code? If so fixing that might make more sense.
> 
> Before starting this patch, I built a Debug build of LLVM and stepped through 
> LLVM parsing code while running clang-tidy on the 
> readability-redundant-string-init.cpp file. I set breakpoints and inspected 
> variable values under both -std c++14 and -std c++17. The call stacks were 
> clearly different. I could sometimes inspect character positions in the file 
> and see where an expression's SourceLoc was set. However, a SourceLoc is a 
> single character position; I couldn't figure out where an expression's 
> SourceRange was even set. So I gave up and went down the current approach.
> 
> Should we ping the LLVM mailing list and see if this change rings a bell with 
> anyone there?
Yes, I believe that changing Clang to produce consistent source ranges in C++14 
and C++17 is the best way to go.

Manual parsing below is not a very maintainable solution.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69238



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

[PATCH] D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable()

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

A few comments inline.




Comment at: clang/tools/driver/CMakeLists.txt:34
   ${tablegen_deps}
+  SUPPORT_PLUGINS
   )

This is now a behavior change because you're always passing this.

You'll want to bring back the conditional statement with a body like:
```
set(support_plugins SUPPORT_PLUGINS)
```

Then instead of passing `SUPPORT_PLUGINS` here you pass `${support_plugins}`



Comment at: llvm/cmake/modules/AddLLVM.cmake:787
+  if (ARG_SUPPORT_PLUGINS)
+set(LLVM_NO_DEAD_STRIP 1)
+  endif()

By convention in LLVM we use `On` rather than `1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69638



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:57
+  /// Constructs a string representation of the StencilInterfacePart.
+  /// StencilInterfaceParts generated by the `selection` and `run` functions do
+  /// not have a unique string representation.

s/Part// (2x)



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:82
+Stencil makeStencil(RangeSelector Selector);
+inline Stencil makeStencil(Stencil S) { return S; }
 

I feel like this should be an implementation detail of `cat` (or other stencil 
combinators who want to support such implicit conversions). Users writing 
stencils should use concrete factory functions below.





Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:137
+/// number.
+Stencil sequence(std::vector Parts);
 

It is the same operation as `cat`, right? So WDYT about `cat_vector`?



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:144
 
-namespace tooling {
-// DEPRECATED: These are temporary aliases supporting client migration to the
-// `transformer` namespace.
-using Stencil = transformer::Stencil;
-using StencilPart = transformer::StencilPart;
-namespace stencil {
-using transformer::access;
-using transformer::addressOf;
-using transformer::cat;
-using transformer::deref;
-using transformer::dPrint;
-using transformer::expression;
-using transformer::ifBound;
-using transformer::run;
-using transformer::selection;
-using transformer::text;
-/// \returns the source corresponding to the identified node.
-/// FIXME: Deprecated. Write `selection(node(Id))` instead.
-inline transformer::StencilPart node(llvm::StringRef Id) {
-  return selection(tooling::node(Id));
+/// General-purpose Stencil factory function. Concatenates 0+ stencil pieces
+/// into a single stencil. Arguments can be raw text, ranges in the matched 
code

I think it is better to remove the first sentence.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:150
 }
-} // namespace stencil
-} // namespace tooling
+Stencil cat(std::vector Parts);
+

Like I said in the other review, I don't feel great about an overload set with 
universal references and anything else.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:152
+
+/// Convenience function for creating a stencil-based MatchConsumer. See `cat`
+/// for more details.

"Creates a MatchConsumer that evaluates a given Stencil."



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:154
+/// for more details.
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);

I'm a bit confused by the name. The function returns a MatchConsumer, but it is 
called "stencil". What is the intended usage?



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:155
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);
+  return [S](const ast_matchers::MatchFinder::MatchResult &R) {

I'm not sure the convenience of being able to avoid to say "cat(...)" at the 
callsite is worth the API complexity. In other words, my suggestion is to make 
this function take a single stencil.



Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:97
+// StencilInteface or just Stencil?
+// unique_ptr or shared_ptr?
+

I think these comments can be deleted now.



Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:262
+
+template  class StencilImpl : public StencilInterface {
   T Data;

There isn't much logic left in StencilImpl.

Would the code be simpler if we made the various Data classes implement 
StencilInterface directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:955
 CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
-tok::comma))
+tok::comma, tok::star, tok::arrow))
   CurrentToken->Previous->Type = TT_OverloadedOperator;

MyDeveloperDay wrote:
> MyDeveloperDay wrote:
> > sammccall wrote:
> > > I'm confused about this: ISTM that where we were previously always 
> > > treating star as a pointer, now we're always treating it as an operator 
> > > name.
> > > Whereas it's sometimes an operator name (immediately after the `operator` 
> > > keyword) and sometimes a pointer (following a type name).
> > > 
> > > Maybe we should shift the OverloadedOperator labelling outside the loop 
> > > (since AFAICT it should only apply to the first token) and then keep the 
> > > loop to mark stars/amps elsewhere in the operator name as 
> > > PointerOrReference?
> > Let me take another look..
> @sammccall  In trying to understand what you were suggesting I tried the 
> following:
> 
> ```
> bool
> Foo::operator*(void *) const {
>   return true;
> }
> ```
> 
> The second `*` will be as seen correctly below a PointerOrReference by the 
> nature that we already hit the OverloadedOperatorLParen as the loop only goes 
> until it see a `(` `)` or `;`
> 
> I'm trying to think of a case where that perhaps wouldn't be the case? such 
> that we might get 2 OverloadedOperators
> 
> ```
> M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=bool L=4 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x1cea7c36c40 Text='bool'
>  M=1 C=1 T=FunctionDeclarationName S=1 B=0 BK=0 P=80 Name=identifier L=84 
> PPK=2 FakeLParens= FakeRParens=0 II=0x1cea7c42298 Text='Foo'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=coloncolon L=86 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text='::'
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=520 Name=operator L=94 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x1cea7c36ed8 Text='operator'
>  M=0 C=0 T=OverloadedOperator S=0 B=0 BK=0 P=34 Name=star L=95 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='*'
>  M=0 C=0 T=OverloadedOperatorLParen S=0 B=0 BK=0 P=34 Name=l_paren L=96 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='('
>  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=void L=100 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x1cea7c368e0 Text='void'
>  M=0 C=1 T=PointerOrReference S=1 B=0 BK=0 P=230 Name=star L=102 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='*'
>  M=0 C=0 T=Unknown S=0 B=0 BK=0 P=54 Name=r_paren L=103 PPK=2 FakeLParens= 
> FakeRParens=0 II=0x0 Text=')'
>  M=0 C=1 T=TrailingAnnotation S=1 B=0 BK=0 P=170 Name=const L=109 PPK=2 
> FakeLParens= FakeRParens=0 II=0x1cea7c363e8 Text='const'
>  M=0 C=0 T=FunctionLBrace S=1 B=0 BK=1 P=23 Name=l_brace L=111 PPK=2 
> FakeLParens= FakeRParens=0 II=0x0 Text='{'
> ```
I mean something like a conversion operator to pointer: `bool Foo::operator 
void *();`
The star appears before the operator arg list, but it's still a pointer.

So the testcases I'd try here would be:

```
Foo::operator*(); // should be OverloadedOperator
Foo::operator void *(); // should be PointerOrReference
Foo::operator()(void *); // should be PointerOrReference
Foo::operator*(void *); // should be OverloadedOperator, PointerOrReference
operator*(int(*)(), class Foo); // OverloadedOperator, PointerOrReference (this 
one is silly and doesn't need to work)
```



Comment at: clang/lib/Format/TokenAnnotator.cpp:2105
+  if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) &&
+  Next->Next && Next->Next->is(tok::star)) {
+// For operator void*(), operator char*(), operator Foo*().

I'm suspicious of code that handles star but not amp, though maybe I shouldn't 
be.


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

https://reviews.llvm.org/D69573



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

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:67-70
+// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to
+// return CtorExpr->getSourceRange(). However, starting with c++17, parsing
+// the expression 'std::string Name = ""' results in a CtorExpr whose
+// SourceRange includes just '""' rather than the previous 'Name = ""'.

gribozavr2 wrote:
> poelmanc wrote:
> > aaron.ballman wrote:
> > > Are you sure that this behavioral difference isn't just a bug in Clang? I 
> > > can't think of why the source range should differ based on language mode, 
> > > so I wonder if the correct thing to do is fix how we calculate the source 
> > > range in Clang?
> > Thanks and no, I'm not sure at all. At the end of my summary paragraph I 
> > wrote:
> > 
> > > Or, is it possible that this change in SourceRange is an unintentional 
> > > difference in the parsing code? If so fixing that might make more sense.
> > 
> > Before starting this patch, I built a Debug build of LLVM and stepped 
> > through LLVM parsing code while running clang-tidy on the 
> > readability-redundant-string-init.cpp file. I set breakpoints and inspected 
> > variable values under both -std c++14 and -std c++17. The call stacks were 
> > clearly different. I could sometimes inspect character positions in the 
> > file and see where an expression's SourceLoc was set. However, a SourceLoc 
> > is a single character position; I couldn't figure out where an expression's 
> > SourceRange was even set. So I gave up and went down the current approach.
> > 
> > Should we ping the LLVM mailing list and see if this change rings a bell 
> > with anyone there?
> Yes, I believe that changing Clang to produce consistent source ranges in 
> C++14 and C++17 is the best way to go.
> 
> Manual parsing below is not a very maintainable solution.
> Yes, I believe that changing Clang to produce consistent source ranges in 
> C++14 and C++17 is the best way to go.

+1, if this is possible to do.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69238



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1371
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.

lichray wrote:
> Maybe make use of some `TT_TemplateOpener`?
We can't use TT_TemplateOpener  because like MatchParen it hasn't been set yet 
on the downstream tokens



Comment at: clang/unittests/Format/FormatTest.cpp:4987
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> Afoo<1>)>;");

lichray wrote:
> Does `A() -> A>)>` (C++11 `>>`) work?
this should work because we are skipping everything in between the `()`


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

https://reviews.llvm.org/D69577



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 227317.
MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added a comment.

Add additional test case


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

https://reviews.llvm.org/D69577

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4977,6 +4977,25 @@
   verifyFormat("void f() { auto a = b->c(); }");
 }
 
+TEST_F(FormatTest, DeductionGuides) {
+  verifyFormat("template  A(const T &, const T &) -> A;");
+  verifyFormat("template  explicit A(T &, T &&) -> A;");
+  verifyFormat("template  S(Ts...) -> S;");
+  verifyFormat(
+  "template \n"
+  "array(T &&... t) -> array, sizeof...(T)>;");
+  verifyFormat("A() -> Afoo<3>())>;");
+  verifyFormat("A() -> A>)>;");
+  verifyFormat("A() -> Afoo<1>)>;");
+  verifyFormat("A() -> A<(3 < 2)>;");
+  verifyFormat("A() -> A<((3) < (2))>;");
+
+  // Ensure not deduction guides.
+  verifyFormat("c()->f();");
+  verifyFormat("x()->foo<1>;");
+  verifyFormat("x = p->foo<3>();");
+}
+
 TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
   // Avoid breaking before trailing 'const' or other trailing annotations, if
   // they are not function-like.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,6 +1350,58 @@
 }
   }
 
+  static FormatToken *untilMatchingParen(FormatToken *Current) {
+// for when MatchingParen is not yet established
+int ParenLevel = 0;
+while (Current) {
+  if (Current->is(tok::l_paren))
+ParenLevel++;
+  if (Current->is(tok::r_paren))
+ParenLevel--;
+  if (ParenLevel < 1)
+break;
+  Current = Current->Next;
+}
+return Current;
+  }
+
+  static bool isDeductionGuide(FormatToken &Current) {
+// Look for a deduction guide A(...) -> A<...>;
+if (Current.Previous && Current.Previous->is(tok::r_paren) &&
+Current.startsSequence(tok::arrow, tok::identifier, tok::less)) {
+  // Find the TemplateCloser.
+  FormatToken *TemplateCloser = Current.Next->Next;
+  int NestingLevel = 0;
+  while (TemplateCloser) {
+// Skip over an expressions in parens  A<(3 < 2)>;
+if (TemplateCloser->is(tok::l_paren)) {
+  // No Matching Paren yet so skip to matching paren
+  TemplateCloser = untilMatchingParen(TemplateCloser);
+}
+if (TemplateCloser->is(tok::less))
+  NestingLevel++;
+if (TemplateCloser->is(tok::greater))
+  NestingLevel--;
+if (NestingLevel < 1)
+  break;
+TemplateCloser = TemplateCloser->Next;
+  }
+  // Assuming we have found the end of the template ensure its followed
+  // with a ;
+  if (TemplateCloser && TemplateCloser->Next &&
+  TemplateCloser->Next->is(tok::semi) &&
+  Current.Previous->MatchingParen) {
+// Determine if the identifier `A` prior to the A<..>; is the same as
+// prior to the A(..)
+FormatToken *LeadingIdentifier =
+Current.Previous->MatchingParen->Previous;
+return (LeadingIdentifier &&
+LeadingIdentifier->TokenText == Current.Next->TokenText);
+  }
+}
+return false;
+  }
+
   void determineTokenType(FormatToken &Current) {
 if (!Current.is(TT_Unknown))
   // The token type is already known.
@@ -1397,6 +1449,10 @@
!Current.Previous->is(tok::kw_operator)) {
   // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
+
+} else if (isDeductionGuide(Current)) {
+  // Deduction guides trailing arrow " A(...) -> A;".
+  Current.Type = TT_TrailingReturnArrow;
 } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
   Current.Type = determineStarAmpUsage(Current,
Contexts.back().CanBeExpression &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Could you, please, give us a bit more context and provide more info for the 
questions @rjmccall and I asked before?

Is the problem specifically because autolink is not supported on device side? 
Or is disabling autolink just a convoluted way to avoid calling 
EmitModuleLinkOptions()?
If it's the former, then we should just disable it unconditionally -- we 
already filter out some other flags  (e.g. asan).
If it's the latter, then tweaking autolink option behavior is just covering the 
problem. Sooner or later EmitModuleLinkOptions() will be called by something 
else and we'll be back to where we are now.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 5 inline comments as done.
ymandel added a comment.

Thanks for the review! I agreed w/ all the comments, just responding early w/ 
those I had further thoughts/questions on.




Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:82
+Stencil makeStencil(RangeSelector Selector);
+inline Stencil makeStencil(Stencil S) { return S; }
 

gribozavr2 wrote:
> I feel like this should be an implementation detail of `cat` (or other 
> stencil combinators who want to support such implicit conversions). Users 
> writing stencils should use concrete factory functions below.
> 
> 
Agreed, but I wasn't sure if it was worth introducing a "detail" namespace for 
just one (overloaded) function. I liked when these were private methods of 
Stencil, but we don't have that option now. Should I use a `namespace detail`? 
something else? 



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:137
+/// number.
+Stencil sequence(std::vector Parts);
 

gribozavr2 wrote:
> It is the same operation as `cat`, right? So WDYT about `cat_vector`?
not quite. `cat` is "smart" and optimizes case of one argument to just return 
that argument. in the future, it might also handle the case of 0 arguments 
specially. `sequence` just blindly constructs a sequence stencil.

that said, i can see the argument that the user doesn't care about such details 
and instead of the three functions:
```
cat(...)
cat(vector)
sequence(vector)
```

we should just have the two
```
cat(...)
catVector(vector)
```
with no "plain" sequence constructor. WDYT?



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:150
 }
-} // namespace stencil
-} // namespace tooling
+Stencil cat(std::vector Parts);
+

gribozavr2 wrote:
> Like I said in the other review, I don't feel great about an overload set 
> with universal references and anything else.
Sorry, I'd forgotten we'd discussed this before. I see your point -- I was 
surprised it worked, but when it did just barelled along. That should have been 
a red flag. :)  I'm happy to rework this, per the comment above,.



Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:154
+/// for more details.
+template  MatchConsumer stencil(Ts &&... Parts) {
+  Stencil S = cat(std::forward(Parts)...);

gribozavr2 wrote:
> I'm a bit confused by the name. The function returns a MatchConsumer, but it 
> is called "stencil". What is the intended usage?
Intended use is something like `makeRule(matcher, change(stencil(...)))`.

The naming is the same as we previously had for `text()`. Since `MatchConsumer` 
is so general, the function name describes its argument, not its particular 
type.  But, I'm open to other suggestions.



Comment at: clang/lib/Tooling/Transformer/Stencil.cpp:262
+
+template  class StencilImpl : public StencilInterface {
   T Data;

gribozavr2 wrote:
> There isn't much logic left in StencilImpl.
> 
> Would the code be simpler if we made the various Data classes implement 
> StencilInterface directly?
Yes! I plan to do that in a followup revision, though I was really tempted to 
do it here already. :)

That said, we could also use a `llvm::PointerSumType` and implement it in 
functional style, fwiw. I'm fine either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69613



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

LGTM modulo the isFirstDeclComment. Will approve after we resolve that 
discussion.




Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:45
+  isDefaulted(),
+  unless(anyOf(isFirstDecl(), isVirtual(),
+   ofClass(cxxRecordDecl(

The "isFirstDecl" part probably will always work, however, it is semantically 
not really the right predicate to check. What we want to check is where the 
constructor is declared -- in the class or outside the class, correct? If so, 
then the matcher should be looking at the DeclContext of the constructor decl.


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

https://reviews.llvm.org/D69435



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

[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked an inline comment as done.
mibintc added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1152
+def frounding_math : Flag<["-"], "frounding-math">, Group, 
Flags<[CC1Option]>;
+def fno_rounding_math : Flag<["-"], "fno-rounding-math">, Group, 
Flags<[CC1Option]>;
 def ftrapping_math : Flag<["-"], "ftrapping-math">, Group, 
Flags<[CC1Option]>;

rjmccall wrote:
> It looks like both of these can now be written with `BooleanFFlag`.
BooleanFFlag doesn't work, there's a FIXME message saying that prefixes don't 
work, currently they are only being used for unimplemented options.
llvm/clang/lib/Driver/ToolChains/Clang.cpp:2301:17: error: ‘OPT_frounding_math’ 
is not a member of ‘clang::driver::options’
 optID = options::OPT_frounding_math;
 ^



Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D69678: [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

When a target does not support pragma detect_mismatch, an llvm.linker.options
metadata with an empty entry is created, which causes diagnostic in backend
since backend expects name/value pair in llvm.linker.options entries.

This patch fixes that.


https://reviews.llvm.org/D69678

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/pragma-detect_mismatch.c


Index: clang/test/CodeGen/pragma-detect_mismatch.c
===
--- clang/test/CodeGen/pragma-detect_mismatch.c
+++ clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | 
FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1930,6 +1930,8 @@
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }


Index: clang/test/CodeGen/pragma-detect_mismatch.c
===
--- clang/test/CodeGen/pragma-detect_mismatch.c
+++ clang/test/CodeGen/pragma-detect_mismatch.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple thumbv7-windows -fms-extensions -emit-llvm -o - \
+// RUN:   | FileCheck %s
+// RUN: %clang_cc1 %s -triple amdgcn-amd-amdhsa -fms-extensions -emit-llvm -o \
+// RUN:   - | FileCheck -check-prefix=AMD %s
 
 #pragma detect_mismatch("test", "1")
 
@@ -9,3 +13,4 @@
 // CHECK: !llvm.linker.options = !{![[test:[0-9]+]], ![[test2:[0-9]+]]}
 // CHECK: ![[test]] = !{!"/FAILIFMISMATCH:\22test=1\22"}
 // CHECK: ![[test2]] = !{!"/FAILIFMISMATCH:\22test2=2\22"}
+// AMD-NOT: !llvm.linker.options
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1930,6 +1930,8 @@
 void CodeGenModule::AddDetectMismatch(StringRef Name, StringRef Value) {
   llvm::SmallString<32> Opt;
   getTargetCodeGenInfo().getDetectMismatchOption(Name, Value, Opt);
+  if (Opt.empty())
+return;
   auto *MDOpts = llvm::MDString::get(getLLVMContext(), Opt);
   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D57829#1729094 , @tra wrote:

> Could you, please, give us a bit more context and provide more info for the 
> questions @rjmccall and I asked before?
>
> Is the problem specifically because autolink is not supported on device side? 
> Or is disabling autolink just a convoluted way to avoid calling 
> EmitModuleLinkOptions()?
>  If it's the former, then we should just disable it unconditionally -- we 
> already filter out some other flags  (e.g. asan).
>  If it's the latter, then tweaking autolink option behavior is just covering 
> the problem. Sooner or later EmitModuleLinkOptions() will be called by 
> something else and we'll be back to where we are now.


we need to disable autolink for device compilation because the linker options 
are intended for host compilation.

Previously I said the backend does not support linker option, which was 
incorrect. The diagnostic about invalid linker option was due to a clang 
codegen bug due to detect_mismatch, which I have a fix

https://reviews.llvm.org/D69678

Even with that fix, we still need to disable auto link for device compilation, 
since it is intended for host compilation.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69678: [CodeGen] Fix invalid llvm.linker.options about pragma detect_mismatch

[John McCall via Phabricator via cfe-commits]

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

Seems reasonable.


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

https://reviews.llvm.org/D69678



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:389-392
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+  JA.isDeviceOffloading(Action::OFK_HIP))
+Default = false;

Shouldn't it be `true`, considering that we do want to **disable** autolinking 
by default for device-side CUDA/HIP?

If we don't support autolinking at all for CUDA/HIP, perhaps we should just 
return `true` here.


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

https://reviews.llvm.org/D57829



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

[clang] 19f1dc7 - Remove unneeded template alias, causes issues with some MSVC version

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-10-31T14:20:54-07:00
New Revision: 19f1dc7b527eade11dae9425c420cc9f450393b6

URL: 
https://github.com/llvm/llvm-project/commit/19f1dc7b527eade11dae9425c420cc9f450393b6
DIFF: 
https://github.com/llvm/llvm-project/commit/19f1dc7b527eade11dae9425c420cc9f450393b6.diff

LOG: Remove unneeded template alias, causes issues with some MSVC version

I built locally with the latest MSVC in c++14 and c++17, but it does not
complain for me. Osman Zakir on llvm-dev reports that they run into
compile errors here.

In any case, it seems prefereable to reuse clang's LLVM.h header to
bring in llvm::Optional and Expected.

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeStmtGen.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp 
b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index c71301598bde..5b47489e65e0 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -14,13 +14,11 @@
 #include "PrimType.h"
 #include "Program.h"
 #include "State.h"
+#include "clang/Basic/LLVM.h"
 
 using namespace clang;
 using namespace clang::interp;
 
-template  using Expected = llvm::Expected;
-template  using Optional = llvm::Optional;
-
 namespace clang {
 namespace interp {
 



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

[clang] d816d9b - [clang][ScanDeps] Fix issue with multiple commands with the same input.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2019-10-31T14:22:01-07:00
New Revision: d816d9bdc585bbf77a7a1c47a7199fd9e0c34402

URL: 
https://github.com/llvm/llvm-project/commit/d816d9bdc585bbf77a7a1c47a7199fd9e0c34402
DIFF: 
https://github.com/llvm/llvm-project/commit/d816d9bdc585bbf77a7a1c47a7199fd9e0c34402.diff

LOG: [clang][ScanDeps] Fix issue with multiple commands with the same input.

Previously, given a CompilationDatabase with two commands for the same
source file we would report that file twice with the union of the
dependencies for each command both times.

This was due to the way `ClangTool` runs actions given an input source
file (see the comment in `DependencyScanningTool.cpp`). This commit adds
a `SingleCommandCompilationDatabase` that is created with each
`CompileCommand` in the original CDB, which is then used for each
`ClangTool` invocation. This gives us a single run of
`DependencyScanningAction` per `CompileCommand`.

I looked at using `AllTUsToolExecutor` which is a parallel tool
executor, but I'm not sure it's suitable for `clang-scan-deps` as it
does a lot more sharing of state than `AllTUsToolExecutor` expects.

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

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/test/ClangScanDeps/Inputs/regular_cdb.json
clang/test/ClangScanDeps/error.cpp
clang/test/ClangScanDeps/regular_cdb.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 78b49e4fa0c5..a3a8c6988819 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -26,9 +26,7 @@ class DependencyScanningTool {
   ///
   /// \param Compilations The reference to the compilation database that's
   /// used by the clang tool.
-  DependencyScanningTool(
-  DependencyScanningService &Service,
-  const clang::tooling::CompilationDatabase &Compilations);
+  DependencyScanningTool(DependencyScanningService &Service);
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
@@ -36,13 +34,13 @@ class DependencyScanningTool {
   ///
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, dependency file contents otherwise.
-  llvm::Expected getDependencyFile(const std::string &Input,
-StringRef CWD);
+  llvm::Expected
+  getDependencyFile(const tooling::CompilationDatabase &Compilations,
+StringRef CWD);
 
 private:
   const ScanningOutputFormat Format;
   DependencyScanningWorker Worker;
-  const tooling::CompilationDatabase &Compilations;
 };
 
 } // end namespace dependencies

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 4b10f24167a8..f643c538f8f9 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -23,14 +23,12 @@ namespace tooling{
 namespace dependencies{
 
 DependencyScanningTool::DependencyScanningTool(
-DependencyScanningService &Service,
-const tooling::CompilationDatabase &Compilations)
-: Format(Service.getFormat()), Worker(Service), Compilations(Compilations) 
{
+DependencyScanningService &Service)
+: Format(Service.getFormat()), Worker(Service) {
 }
 
-llvm::Expected
-DependencyScanningTool::getDependencyFile(const std::string &Input,
-  StringRef CWD) {
+llvm::Expected DependencyScanningTool::getDependencyFile(
+const tooling::CompilationDatabase &Compilations, StringRef CWD) {
   /// Prints out all of the gathered dependencies into a string.
   class MakeDependencyPrinterConsumer : public DependencyConsumer {
   public:
@@ -140,6 +138,16 @@ DependencyScanningTool::getDependencyFile(const 
std::string &Input,
 std::string ContextHash;
   };
 
+  
+  // We expect a single command here because if a source file occurs multiple
+  // times in the original CDB, then `computeDependencies` would run the
+  // `DependencyScanningAction` once for every time the input occured in the
+  // CDB. Instead we split up the CDB into single command chunks to avoid this
+  // behavior.
+  assert(Compilations.getAllCompileCommands().size() == 1 &&
+ "Expected a compilation database with a single command!");
+  std::string Input = Compilations.getAllCompileCommands().front().Filename;
+  
   if (Format == ScanningOutputForma

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked an inline comment as done.
AntonBikineev added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:45
+  isDefaulted(),
+  unless(anyOf(isFirstDecl(), isVirtual(),
+   ofClass(cxxRecordDecl(

gribozavr2 wrote:
> The "isFirstDecl" part probably will always work, however, it is semantically 
> not really the right predicate to check. What we want to check is where the 
> constructor is declared -- in the class or outside the class, correct? If so, 
> then the matcher should be looking at the DeclContext of the constructor decl.
Thanks for the comment.
> What we want to check is where the constructor is declared -- in the class or 
> outside the class, correct? 
I was following the Standard's wording, which says that trivial destructor is 
not user-provided, where [[ 
http://eel.is/c++draft/dcl.fct.def.default#def:user-provided | user-provided ]] 
means
> user-declared and not explicitly defaulted or deleted on its **first 
> declaration**.
Technically, I think both ways should work as C++ doesn't allow more than 2 
special function declarations.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

[Michael Spencer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd816d9bdc585: [clang][ScanDeps] Fix issue with multiple 
commands with the same input. (authored by Bigcheese).

Changed prior to commit:
  https://reviews.llvm.org/D69643?vs=227178&id=227347#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69643

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/error.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -108,6 +108,24 @@
   return ObjFileName.str();
 }
 
+class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
+public:
+  SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
+  : Command(std::move(Cmd)) {}
+
+  virtual std::vector
+  getCompileCommands(StringRef FilePath) const {
+return {Command};
+  }
+
+  virtual std::vector getAllCompileCommands() const {
+return {Command};
+  }
+
+private:
+  tooling::CompileCommand Command;
+};
+
 /// Takes the result of a dependency scan and prints error / dependency files
 /// based on the result.
 ///
@@ -147,11 +165,6 @@
 
   llvm::cl::PrintOptionValues();
 
-  // By default the tool runs on all inputs in the CDB.
-  std::vector> Inputs;
-  for (const auto &Command : Compilations->getAllCompileCommands())
-Inputs.emplace_back(Command.Filename, Command.Directory);
-
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
   std::make_unique(
@@ -221,8 +234,12 @@
 #endif
   std::vector> WorkerTools;
   for (unsigned I = 0; I < NumWorkers; ++I)
-WorkerTools.push_back(std::make_unique(
-Service, *AdjustingCompilations));
+WorkerTools.push_back(std::make_unique(Service));
+
+  std::vector Inputs;
+  for (tooling::CompileCommand Cmd :
+   AdjustingCompilations->getAllCompileCommands())
+Inputs.emplace_back(Cmd);
 
   std::vector WorkerThreads;
   std::atomic HadErrors(false);
@@ -237,20 +254,22 @@
 auto Worker = [I, &Lock, &Index, &Inputs, &HadErrors, &WorkerTools,
&DependencyOS, &Errs]() {
   while (true) {
-std::string Input;
-StringRef CWD;
+const SingleCommandCompilationDatabase *Input;
+std::string Filename;
+std::string CWD;
 // Take the next input.
 {
   std::unique_lock LockGuard(Lock);
   if (Index >= Inputs.size())
 return;
-  const auto &Compilation = Inputs[Index++];
-  Input = Compilation.first;
-  CWD = Compilation.second;
+  Input = &Inputs[Index++];
+  tooling::CompileCommand Cmd = Input->getAllCompileCommands()[0];
+  Filename = std::move(Cmd.Filename);
+  CWD = std::move(Cmd.Directory);
 }
 // Run the tool on it.
-auto MaybeFile = WorkerTools[I]->getDependencyFile(Input, CWD);
-if (handleDependencyToolResult(Input, MaybeFile, DependencyOS, Errs))
+auto MaybeFile = WorkerTools[I]->getDependencyFile(*Input, CWD);
+if (handleDependencyToolResult(Filename, MaybeFile, DependencyOS, Errs))
   HadErrors = true;
   }
 };
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===
--- clang/test/ClangScanDeps/regular_cdb.cpp
+++ clang/test/ClangScanDeps/regular_cdb.cpp
@@ -9,11 +9,11 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources \
-// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 //
 // Make sure we didn't produce any dependency files!
 // RUN: not cat %t.dir/regular_cdb.d
@@ -40,6 +40,9 @@
 // CHECK1-NEXT: Inputs{{/|\\}}header.h
 // CHECK1-NEXT: Inputs{{/|\\}}header2.h
 
+// CHECK3: regular_cdb_input.o
 // CHECK2: regular_cdb_input.cpp
 // CHECK2-NEXT: Inputs{{/|\\}}header.h
 // CHECK

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 227352.
nridge marked 12 inline comments as done.
nridge added a comment.

Address latest comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -490,7 +490,7 @@
 struct Foo {
   Foo();
   Foo(Foo&&);
-  Foo(const char*);
+  $ConstructorLoc[[Foo]](const char*);
 };
 
 Foo f();
@@ -507,6 +507,8 @@
   Foo ab$7^c;
   Foo ab$8^cd("asdf");
   Foo foox = Fo$9^o("asdf");
+  Foo abcde$10^("asdf");
+  Foo foox2 = Foo$11^("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
@@ -517,12 +519,16 @@
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  // FIXME: Target the constructor as well.
   EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
-  ElementsAre(Sym("Foo"), Sym("abcd")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
-  // First one is class definition, second is the constructor.
-  ElementsAre(Sym("Foo"), Sym("Foo")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")), ElementsAre(Sym("abcd")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("10")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
@@ -2192,7 +2198,7 @@
 const char *DeducedType;
   } Tests[] = {
   {"^auto i = 0;", "int"},
-  {"^auto f(){ return 1;};", "int"}
+  {"^auto f(){ return 1;};", "int"},
   };
   for (Test T : Tests) {
 Annotations File(T.AnnotatedCode);
Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -60,6 +60,8 @@
   int c;
 };
 
+using A^lias = Child2;
+
 int main() {
   Ch^ild2 ch^ild2;
   ch^ild2.c = 1;
Index: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
@@ -24,7 +24,7 @@
 namespace clangd {
 namespace {
 
-using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 auto CreateExpectedSymbolDetails = [](const std::string &name,
   const std::string &container,
@@ -329,7 +329,7 @@
 auto AST = TestTU::withCode(TestInput.code()).build();
 
 EXPECT_THAT(getSymbolInfo(AST, TestInput.point()),
-ElementsAreArray(T.second))
+UnorderedElementsAreArray(T.second))
 << T.first;
   }
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,18 @@
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation &SearchedLocation;
-
-public:
-  DeclarationFinder(const SourceLocation &SearchedLocation)
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  SourceLocation Loc,
-  index::IndexDataCons

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1133
+  DeclRelationSet Relations =
+  DeclRelation::TemplatePattern | DeclRelation::Alias;
+  auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);

sammccall wrote:
> Are you sure you don't want type hierarchy to work on aliases to record 
> types? (i.e. specify underlying rather than alias)
Good catch! I added a test case to `TypeHierarchyTests` for this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237



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

[clang-tools-extra] b6429cd - Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2019-10-31T17:37:27-04:00
New Revision: b6429cdd65ffa28591c5b0da37244ab66d0b1785

URL: 
https://github.com/llvm/llvm-project/commit/b6429cdd65ffa28591c5b0da37244ab66d0b1785
DIFF: 
https://github.com/llvm/llvm-project/commit/b6429cdd65ffa28591c5b0da37244ab66d0b1785.diff

LOG: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

Summary: This fixes issue #163, among other improvements to go-to-definition.

Reviewers: sammccall

Subscribers: jkorous, mgrang, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 3ee04f031795..0cfa53558114 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,18 @@ SymbolLocation getPreferredLocation(const Location 
&ASTLoc,
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation &SearchedLocation;
-
-public:
-  DeclarationFinder(const SourceLocation &SearchedLocation)
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  SourceLocation Loc,
-  index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-// Skip non-semantic references.
-if (Roles & static_cast(index::SymbolRole::NameReference))
-  return true;
-
-if (Loc == SearchedLocation) {
-  auto IsImplicitExpr = [](const Expr *E) {
-if (!E)
-  return false;
-// We assume that a constructor expression is implict (was inserted by
-// clang) if it has an invalid paren/brace location, since such
-// experssion is impossible to write down.
-if (const auto *CtorExpr = dyn_cast(E))
-  return CtorExpr->getParenOrBraceRange().isInvalid();
-// Ignore implicit conversion-operator AST node.
-if (const auto *ME = dyn_cast(E)) {
-  if (isa(ME->getMemberDecl()))
-return ME->getMemberLoc().isInvalid();
-}
-return isa(E);
-  };
-
-  if (IsImplicitExpr(ASTNode.OrigE))
-return true;
-  // Find and add definition declarations (for GoToDefinition).
-  // We don't use parameter `D`, as Parameter `D` is the canonical
-  // declaration, which is the first declaration of a redeclarable
-  // declaration, and it could be a forward declaration.
-  if (const auto *Def = getDefinition(D)) {
-Decls.insert(Def);
-  } else {
-// Couldn't find a definition, fall back to use `D`.
-Decls.insert(D);
-  }
-}
-return true;
+std::vector getDeclAtPosition(ParsedAST &AST, SourceLocation Pos,
+DeclRelationSet Relations) {
+  FileID FID;
+  unsigned Offset;
+  std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+  SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
+  std::vector Result;
+  if (const SelectionTree::Node *N = Selection.commonAncestor()) {
+auto Decls = targetDecl(N->ASTNode, Relations);
+Result.assign(Decls.begin(), Decls.end());
   }
-};
-
-std::vector getDeclAtPosition(ParsedAST &AST,
-SourceLocation Pos) {
-  DeclarationFinder Finder(Pos);
-  index::IndexingOptions IndexOpts;
-  IndexOpts.SystemSymbolFilter =
-  index::IndexingOptions::SystemSymbolFilterKind::All;
-  IndexOpts.IndexFunctionLocals = true;
-  IndexOpts.IndexParametersInDeclarations = true;
-  IndexOpts.IndexTemplateParameters = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
- AST.getLocalTopLevelDecls(), Finder, IndexOpts);
-
-  return Finder.getFoundDecls();
+  return Result;
 }
 
 llvm::Optional makeLocation(ASTContext &AST, SourceLocation TokLoc,
@@ -258,14 +194,13 @@ std::vector locateSymbolAt(Par

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

Based on the comments it looks like you confirmed this matches MSVC's behavior, 
at least in this regard. I haven't stared at this deeply to think of all the 
corner cases, though.

To Nico's point, I think this change doesn't move us away from conformance, 
it's a bug in our compatibility. We've created a bad situation here were we are 
buggily implementing MSVC pre-processor rules, and nobody should have to add a 
third set of ifdefs to deal with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69626



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

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6429cdd65ff: Refactor getDeclAtPosition() to use 
SelectionTree + targetDecl() (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -490,7 +490,7 @@
 struct Foo {
   Foo();
   Foo(Foo&&);
-  Foo(const char*);
+  $ConstructorLoc[[Foo]](const char*);
 };
 
 Foo f();
@@ -507,6 +507,8 @@
   Foo ab$7^c;
   Foo ab$8^cd("asdf");
   Foo foox = Fo$9^o("asdf");
+  Foo abcde$10^("asdf");
+  Foo foox2 = Foo$11^("asdf");
 }
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
@@ -517,12 +519,16 @@
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
+  // FIXME: Target the constructor as well.
   EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
-  ElementsAre(Sym("Foo"), Sym("abcd")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
-  // First one is class definition, second is the constructor.
-  ElementsAre(Sym("Foo"), Sym("Foo")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("8")), ElementsAre(Sym("abcd")));
+  // FIXME: Target the constructor as well.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("10")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
+  ElementsAre(Sym("Foo", T.range("ConstructorLoc";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
@@ -2192,7 +2198,7 @@
 const char *DeducedType;
   } Tests[] = {
   {"^auto i = 0;", "int"},
-  {"^auto f(){ return 1;};", "int"}
+  {"^auto f(){ return 1;};", "int"},
   };
   for (Test T : Tests) {
 Annotations File(T.AnnotatedCode);
Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -60,6 +60,8 @@
   int c;
 };
 
+using A^lias = Child2;
+
 int main() {
   Ch^ild2 ch^ild2;
   ch^ild2.c = 1;
Index: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
@@ -24,7 +24,7 @@
 namespace clangd {
 namespace {
 
-using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 auto CreateExpectedSymbolDetails = [](const std::string &name,
   const std::string &container,
@@ -329,7 +329,7 @@
 auto AST = TestTU::withCode(TestInput.code()).build();
 
 EXPECT_THAT(getSymbolInfo(AST, TestInput.point()),
-ElementsAreArray(T.second))
+UnorderedElementsAreArray(T.second))
 << T.first;
   }
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,18 @@
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation &SearchedLocation;
-
-public:
-  DeclarationFinder(const SourceLocation &SearchedLocation)
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
-  }
-
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  Sourc