[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D59135#1422822 , @thorsten-klein 
wrote:

> Sorry for delay. I am currently not able to build master (Error: "sort" is 
> not a member of "llvm").
>  And if I try to build 6.0.1-rc3 I can build clang-tidy but I am not able to 
> build corresponding unittests "ClangTidyTests".
>  I will try to solve as soon as possible.


The patches need to be for llvm trunk / git master.


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

https://reviews.llvm.org/D59135



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

[PATCH] D59578: [X86] Remove getCPUKindCanonicalName which seems to be unused.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added a reviewer: erichkeane.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D59578

Files:
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h


Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -121,8 +121,6 @@
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1538,18 +1538,6 @@
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)
\
-  case CK_##ENUM:  
\
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently


Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -121,8 +121,6 @@
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1538,18 +1538,6 @@
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)\
-  case CK_##ENUM:  \
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r356537 - [X86] Separate PentiumPro and i686. They aren't aliases in the backend.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed Mar 20 00:31:18 2019
New Revision: 356537

URL: http://llvm.org/viewvc/llvm-project?rev=356537&view=rev
Log:
[X86] Separate PentiumPro and i686. They aren't aliases in the backend.

PentiumPro has HasNOPL set in the backend. i686 does not.

Despite having a function that looks like it canonicalizes alias names. It
doesn't seem to be called. So I don't think this is a functional change. But its
good to be consistent between the backend and frontend.

Modified:
cfe/trunk/include/clang/Basic/X86Target.def
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/include/clang/Basic/X86Target.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/X86Target.def?rev=356537&r1=356536&r2=356537&view=diff
==
--- cfe/trunk/include/clang/Basic/X86Target.def (original)
+++ cfe/trunk/include/clang/Basic/X86Target.def Wed Mar 20 00:31:18 2019
@@ -66,7 +66,7 @@ PROC(PentiumMMX, "pentium-mmx", PROC_32_
 /// i686-generation processors, P6 / Pentium M microarchitecture based.
 //@{
 PROC(PentiumPro, "pentiumpro", PROC_32_BIT)
-PROC_ALIAS(PentiumPro, "i686")
+PROC(i686, "i686", PROC_32_BIT)
 PROC(Pentium2, "pentium2", PROC_32_BIT)
 PROC(Pentium3, "pentium3", PROC_32_BIT)
 PROC_ALIAS(Pentium3, "pentium3m")

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=356537&r1=356536&r2=356537&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Mar 20 00:31:18 2019
@@ -122,6 +122,7 @@ bool X86TargetInfo::initFeatureMap(
   case CK_i586:
   case CK_Pentium:
   case CK_PentiumPro:
+  case CK_i686:
   case CK_Lakemont:
 break;
 
@@ -926,6 +927,7 @@ void X86TargetInfo::getTargetDefines(con
 Builder.defineMacro("__tune_pentium2__");
 LLVM_FALLTHROUGH;
   case CK_PentiumPro:
+  case CK_i686:
 defineCPUMacros(Builder, "i686");
 defineCPUMacros(Builder, "pentiumpro");
 break;


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

[PATCH] D59567: [X86] Add __popcntd and __popcntq to ia32intrin.h to match gcc and icc. Remove popcnt feature flag from _popcnt32/_popcnt64 and move to ia32intrin.h to match gcc

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

IIRC we don't use libcalls for popcnt - we just expand




Comment at: clang/lib/Headers/ia32intrin.h:35
+///
+/// This intrinsic corresponds to the  POPCNT  instruction.
+///

Change this as we don't require POPCNT any more?


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

https://reviews.llvm.org/D59567



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

[PATCH] D59354: [clangd] Print arguments in template specializations

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/AST/TypePrinter.cpp:1635
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {

ilya-biryukov wrote:
> NIT: consider also adding a test in the clang code.
> Some developers don't build `tools-extra`.
It seems the amount of test infrastructure for running this in clang might be 
overwhelming.
It's just not worth it probably, having a test in clangd is good enough!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59354



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

[PATCH] D59354: [clangd] Print arguments in template specializations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 191460.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59354

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp
  clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,21 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const auto &Kind = A.getArgument().getKind();
+  assert(Kind != TemplateArgument::Null &&
+ "TemplateArgumentKind can not be null!");
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1668,7 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -394,23 +394,80 @@
   Annotations Header(R"(
 // Primary template and explicit specialization are indexed, instantiation
 // is not.
-template  struct [[Tmpl]] {T $xdecl[[x]] = 0;};
-template <> struct $specdecl[[Tmpl]] {};
-template  struct $partspecdecl[[Tmpl]] {};
-extern template struct Tmpl;
-template struct Tmpl;
+template  class $barclasstemp[[Bar]] {};
+template  class Z, int Q>
+struct [[Tmpl]] { T $xdecl[[x]] = 0; };
+
+// template-template, non-type and type full spec
+template <> struct $specdecl[[Tmpl]] {};
+
+// template-template, non-type and type partial spec
+template  struct $partspecdecl[[Tmpl]] {};
+// instantiation
+extern template struct Tmpl;
+// instantiation
+template struct Tmpl;
+
+template  class $fooclasstemp[[Foo]] {};
+// parameter-packs full spec
+template<> class $parampack[[Foo]], int, double> {};
+// parameter-packs partial spec
+template class $parampackpartial[[Foo]] {};
+
+template  class $bazclasstemp[[Baz]] {};
+// non-type parameter-packs full spec
+template<> class $parampacknontype[[Baz]]<3, 5, 8> {};
+// non-type parameter-packs partial spec
+template class $parampacknontypepartial[[Baz]] {};
+
+template  class ...> class $fozclasstemp[[Foz]] {};
+// template-template parameter-packs full spec
+template<> class $parampacktempltempl[[Foz]] {};
+// template-template parameter-packs partial spec
+template class T>
+class $parampacktempltemplpartial[[Foz]] {};
   )");
   runSymbolCollector(Header.code(), /*Main=*/"");
-  EXPECT_THAT(Symbols,
-  UnorderedElementsAre(
-  AllOf(QName("Tmpl"), DeclRange(Header.range()),
-ForCodeCompletion(true)),
-  AllOf(QName("Tmpl"), DeclRange(Header.range("specdecl")),
-ForCodeCompletion(false)),
-  AllOf(QName("Tmpl"), DeclRange(Header.range("partspecdecl")),
-ForCodeCompletion(false)),
-  AllOf(QName("Tmpl::x"), DeclRange(Header.range("xdecl")),
-ForCodeCompletion(false;
+  EXPECT_THAT(Symbols.size(), 14U);
+  EXPECT_THAT(
+  Symbols,
+  AllOf(
+  Contains(AllOf(QName("Tmpl"), DeclRange(Header.range()),
+ ForCodeCompletion(true))),
+  Contains(AllOf(QName("Bar"), DeclRange(Header.range("barclasstemp")),
+ ForCodeCompletion(true))),
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range("fooclasstemp")),
+ ForCodeCompletion(true))),
+  Contains(AllOf(QName("Baz"), DeclRange(Header.range("bazclasstemp")),
+ ForCodeCompletion(true))),
+  Contains(AllOf(QName("Foz"), DeclRange(Header.range("fozclasstemp")),
+ ForCodeCompletion(true))),
+  Contains(AllOf(QName("T

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(

Same comment as in the other patch -- I would prefer that the source is inlined 
into the ASSERT_TRUE, with implicit string concatenation and "\n"s, if raw 
strings don't work.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:75
+  ASSERT_TRUE(PrintedOMPStmtMatches(Source, OMPStandaloneDirectivekMatcher(),
+"#pragma omp barrier\n"));
+}

Could you also add an assertion that `OMPStructuredBlockMatcher()` matches zero 
AST nodes?  Similarly in every test that has no structured blocks.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:121
+
+TEST(OMPStructuredBlock, TestDistributeParallelForSimdDirective0) {
+  const char *Source =

This test file repeats this pattern many times, only with different OpenMP 
directives:

TEST(OMPStructuredBlock, Test~~~Directive0)
TEST(OMPStructuredBlock, Test~~~Directive1)
TEST(OMPStructuredBlock, Test~~~DirectiveCollapse1)
TEST(OMPStructuredBlock, Test~~~DirectiveCollapse2)
TEST(OMPStructuredBlock, Test~~~DirectiveCollapse22)

WDYT about deduplicating it using parameterized tests?  For example, see 
AssignmentTest in 
`llvm/tools/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp`.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:656
+
+TEST(OMPStructuredBlock, DISABLED_TestParallelMaster0XFAIL) {
+  const char *Source =

Is it only a pretty-printer problem or something more severe?



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:704
+
+StatementMatcher OMPSectionsDirectivekMatcher() {
+  return stmt(

Extra "k" before "Matcher".



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:712
+
+StatementMatcher OMPSectionDirectivekMatcher() {
+  return stmt(isOMPStructuredBlock(),

Extra "k" before "Matcher".



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:1481
+
+TEST(OMPStructuredBlock, TesteamsDistributeParallelForSimdDirective0) {
+  const char *Source =

"TestTeams"



Comment at: unittests/AST/PrintTest.h:1
+//===- unittests/AST/PrintTest.h 
--===//
+//

"ASTPrint.h"?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

r356541 - [clangd] Print arguments in template specializations

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Mar 20 02:43:38 2019
New Revision: 356541

URL: http://llvm.org/viewvc/llvm-project?rev=356541&view=rev
Log:
[clangd] Print arguments in template specializations

Reviewers: ilya-biryukov

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

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/TypePrinter.cpp

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=356541&r1=356540&r2=356541&view=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Wed Mar 20 02:43:38 2019
@@ -1632,6 +1632,21 @@ static const TemplateArgument &getArgume
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const auto &Kind = A.getArgument().getKind();
+  assert(Kind != TemplateArgument::Null &&
+ "TemplateArgumentKind can not be null!");
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1668,7 @@ static void printTo(raw_ostream &OS, Arr
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 


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

[clang-tools-extra] r356541 - [clangd] Print arguments in template specializations

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Mar 20 02:43:38 2019
New Revision: 356541

URL: http://llvm.org/viewvc/llvm-project?rev=356541&view=rev
Log:
[clangd] Print arguments in template specializations

Reviewers: ilya-biryukov

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/index/MemIndex.cpp
clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=356541&r1=356540&r2=356541&view=diff
==
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Wed Mar 20 02:43:38 2019
@@ -11,9 +11,12 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/TemplateBase.h"
+#include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ScopedPrinter.h"
 
@@ -50,6 +53,22 @@ SourceLocation findNameLoc(const clang::
   return SM.getSpellingLoc(D->getLocation());
 }
 
+static llvm::Optional>
+getTemplateSpecializationArgLocs(const NamedDecl &ND) {
+  if (auto *Func = llvm::dyn_cast(&ND)) {
+if (auto *Args = Func->getTemplateSpecializationArgsAsWritten())
+  return Args->arguments();
+  } else if (auto *Cls =
+ llvm::dyn_cast(&ND)) {
+if (auto *Args = Cls->getTemplateArgsAsWritten())
+  return Args->arguments();
+  } else if (auto *Var = llvm::dyn_cast(&ND))
+return Var->getTemplateArgsInfo().arguments();
+  // We return None for ClassTemplateSpecializationDecls because it does not
+  // contain TemplateArgumentLoc information.
+  return llvm::None;
+}
+
 std::string printQualifiedName(const NamedDecl &ND) {
   std::string QName;
   llvm::raw_string_ostream OS(QName);
@@ -60,6 +79,19 @@ std::string printQualifiedName(const Nam
   // namespaces to query: the preamble doesn't have a dedicated list.
   Policy.SuppressUnwrittenScope = true;
   ND.printQualifiedName(OS, Policy);
+  if (auto Args = getTemplateSpecializationArgLocs(ND))
+printTemplateArgumentList(OS, *Args, Policy);
+  else if (auto *Cls = llvm::dyn_cast(&ND)) {
+if (auto STL = Cls->getTypeAsWritten()
+   ->getTypeLoc()
+   .getAs()) {
+  llvm::SmallVector ArgLocs;
+  ArgLocs.reserve(STL.getNumArgs());
+  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+ArgLocs.push_back(STL.getArgLoc(I));
+  printTemplateArgumentList(OS, ArgLocs, Policy);
+}
+  }
   OS.flush();
   assert(!StringRef(QName).startswith("::"));
   return QName;

Modified: clang-tools-extra/trunk/clangd/index/MemIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/MemIndex.cpp?rev=356541&r1=356540&r2=356541&view=diff
==
--- clang-tools-extra/trunk/clangd/index/MemIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp Wed Mar 20 02:43:38 2019
@@ -38,15 +38,6 @@ bool MemIndex::fuzzyFind(
   for (const auto Pair : Index) {
 const Symbol *Sym = Pair.second;
 
-// FIXME: Enable fuzzy find on template specializations once we start
-// storing template arguments in the name. Currently we only store name for
-// class template, which would cause duplication in the results.
-if (Sym->SymInfo.Properties &
-(static_cast(
- index::SymbolProperty::TemplateSpecialization) |
- static_cast(
- index::SymbolProperty::TemplatePartialSpecialization)))
-  continue;
 // Exact match against all possible scopes.
 if (!Req.AnyScope && !llvm::is_contained(Req.Scopes, Sym->Scope))
   continue;

Modified: clang-tools-extra/trunk/clangd/index/dex/Dex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Dex.cpp?rev=356541&r1=356540&r2=356541&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Dex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Dex.cpp Wed Mar 20 02:43:38 2019
@@ -86,15 +86,6 @@ void Dex::buildIndex() {
   llvm::DenseMap> TempInvertedIndex;
   for (DocID SymbolRank = 0; SymbolRank < Symbols.size(); ++SymbolRank) {
 const auto *Sym = Symbols[SymbolRank];
-// FIXME: Enable fuzzy find on template specializations once we start
-// storing tem

[PATCH] D59354: [clangd] Print arguments in template specializations

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356541: [clangd] Print arguments in template specializations 
(authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59354?vs=191460&id=191462#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59354

Files:
  lib/AST/TypePrinter.cpp


Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,21 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const auto &Kind = A.getArgument().getKind();
+  assert(Kind != TemplateArgument::Null &&
+ "TemplateArgumentKind can not be null!");
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1668,7 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 


Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,21 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const auto &Kind = A.getArgument().getKind();
+  assert(Kind != TemplateArgument::Null &&
+ "TemplateArgumentKind can not be null!");
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1668,7 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D55170#1432832 , @reuk wrote:

> @klimek I agree that the rule is somewhat arbitrary. However, it's the style 
> rule of an established codebase with many users (I don't have a concrete 
> number, but the project has 1400 stars on github 
> ). I've found this patch useful when 
> contributing to JUCE and I thought others might too.


Oh wow, wasn't aware how popular this is. Sigh. I'd like us to push them to 
change their style guide, but I guess that's not going to fly.


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

https://reviews.llvm.org/D55170



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Make limit a hardcoded value rather then a command line argument
- Limit diags per include directive


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -73,7 +74,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -603,7 +603,177 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST build(const std::string &MainFile,
+const llvm::StringMap &Files) {
+  std::vector Cmd = {"clang", MainFile.c_str()};
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = MainFile;
+  Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.Contents = Files.lookup(MainFile);
+  Inputs.FS = buildTestFS(Files);
+  Inputs.Opts = ParseOptions();
+  auto PCHs = std::make_shared();
+  auto CI = buildCompilerInvocation(Inputs);
+  assert(CI && "Failed to build compilation invocation.");
+  auto Preamble =
+  buildPreamble(MainFile, *CI,
+/*OldPreamble=*/nullptr,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs, PCHs,
+/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+  Preamble, PCHs);
+  if (!AST.hasValue()) {
+ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+llvm_unreachable("Failed to build TestTU!");
+  }
+  return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "/clangd-test/a.h:1:1: C++ requires a "
+ "type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "#include \"b.h\""},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "In file included from: "
+   "/clangd-test/a.h:1:10:\n/clangd-test/b.h:1:1: C++ "
+   "requires a type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+  Annotations Main(R"cpp(
+#include $a[["a.h"]]
+#include $b[["b.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(
+  build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range("a"), "/clangd-test/a.h:1:1: C++ requires a type "
+"specifier for all declarations"),
+  Diag(Main.range("b"), "/clangd-test/b.h:1:1: C++ requires a type "
+"specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, PreferExpansionLocation) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+#include "b.h"
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {
+  {MainFile, Main.code()},
+  {testPath("a.h"), "#include \"b.h\"\n"},
+  {testPath("b.h"), "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+  

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D55170#1432929 , @MyDeveloperDay 
wrote:

> > I don't think risk is what matters - in the end it's about cost, and cost 
> > is a very technical reason
>
> I'm really sorry I'm not trying to be difficult its just over the years I 
> keep seeing this being said in reviews? but what is the cost? I assume you 
> don't mean a financial cost? could you define cost as a problem so we can 
> address it? what would it take to make `cost` not an issue?


The cost is financial, as it's developer time, which costs real money to 
companies. In the end, to support this, people like myself who are doing this 
as part of their job spend time that they'd otherwise spend to make other 
things better that might be more important. To make cost not an issue we'd need 
to change clang-format to make changes like this significantly simpler to 
review & maintain. I'll add a code comment in a moment, given that this is 
probably something that we'll allow given it fulfills the criteria we've set 
ourselves.


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

https://reviews.llvm.org/D55170



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

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:2546-2560
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
-   tok::kw_new, tok::kw_delete) &&
-  (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
- (Left.is(tok::r_square) && Left.MatchingParen &&
-  Left.MatchingParen->is(TT_LambdaLSquare))) &&
-Line.Type != LT_PreprocessorDirective);
+((Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, 
tok::kw_while,
+   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+   TT_ObjCForIn) ||
+  Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
+  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,

I'm really confused about the changes os parens. It seems like this should just 
change the spaceRequiredBeforeParens(...), but instead seems to change parens 
in a way that completely changes the logic? (or alternatively is phabricator 
showing this wrong?)


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

https://reviews.llvm.org/D55170



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

r356543 - Fix -Wdocumentation warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Wed Mar 20 03:28:08 2019
New Revision: 356543

URL: http://llvm.org/viewvc/llvm-project?rev=356543&view=rev
Log:
Fix -Wdocumentation warning. NFCI.

Modified:
cfe/trunk/include/clang/Tooling/FixIt.h

Modified: cfe/trunk/include/clang/Tooling/FixIt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/FixIt.h?rev=356543&r1=356542&r2=356543&view=diff
==
--- cfe/trunk/include/clang/Tooling/FixIt.h (original)
+++ cfe/trunk/include/clang/Tooling/FixIt.h Wed Mar 20 03:28:08 2019
@@ -77,7 +77,7 @@ CharSourceRange getExtendedRange(const T
 /// if (!x) return foo();
 /// // S2:
 /// if (!x) { return 3; }
-//}
+///   }
 /// \endcode
 /// then
 /// \code


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

[PATCH] D59407: [clangd] Add RelationSlab

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D59407#1432543 , @nridge wrote:

> If it's (B), how many bytes should the index be? Are the space gains worth 
> the complexity, given that `SymbolID` is only 8 bytes to begin with? (As 
> compared to say, the filenames in `Ref`, which can be much longer, making 
> this sort of optimization more clearly worth it.)


That was the case, I agree with you we should rather do that while serializing 
where we have variable length integers and duplication is more severe(all three 
slabs make use of same SymbolID pool).




Comment at: clang-tools-extra/clangd/index/Index.h:43
+public:
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;

nridge wrote:
> kadircet wrote:
> > gribozavr wrote:
> > > `struct Relation`?  And in the comments for it, please explain which way 
> > > the relationship is directed (is the SymbolID in the key the subtype?  or 
> > > is the SymbolID in the ArrayRef the subtype?).
> > Ah exactly my thoughts, forget to mention this.
> > 
> > I believe current usage is the counter-intuitive one. For example, we will 
> > most likely query with something like:
> > `getRelations(SymbolID, baseOf)` to get all relations where `SymbolID` is 
> > `baseOf` something else(which says get children of `SymbolID`)
> > 
> > So that this valueType can be read like, 
> > ```
> > `SymbolID` is `RelationKind` every `SymbolID inside array`
> > ```
> > WDYT?
> The way I was thinking of it is that `getRelations(SymbolID, baseOf)` would 
> return all the bases of `SymbolID`. However, the opposite interpretation is 
> also reasonable, we just need to pick one and document it. I'm happy to go 
> with your suggested one.
It looks like IndexingAPI is also using the interpretation I suggested, so 
let's move with that one if you don't have any other concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: include/clang/Basic/TokenKinds.h:80
  K == tok::utf8_string_literal || K == tok::utf16_string_literal ||
- K == tok::utf32_string_literal;
 }

This change looks pretty good, minus the changes outside Format/ :( Do you see 
any chance to fix this by extending the token merging? It's unlikely that the 
community will be convinced to add some minor parts of C# to clang proper just 
to support clang-format.


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

https://reviews.llvm.org/D58404



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

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:401
+  * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``)
+Allow short if/else if statements even if the else is a compound statement.
+

I'd try to make this either unconditionally what we do, or decide against doing 
it.



Comment at: clang/include/clang/Format/Format.h:263-264
 SIS_WithoutElse,
-/// Always put short ifs on the same line if
-/// the else is not a compound statement or not.
+/// If Else statements have no braces don't put them
+/// on the same line.
+/// \code

This seems weird - why would we want this?


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

https://reviews.llvm.org/D59408



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(

gribozavr wrote:
> Same comment as in the other patch -- I would prefer that the source is 
> inlined into the ASSERT_TRUE, with implicit string concatenation and "\n"s, 
> if raw strings don't work.
I still don't understand the reasoning here.
It feels like a change just for the sake of the change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59595: Remove the unused return value in ASTImporter::Imported [NFC]

[Raphael Isemann via Phabricator via cfe-commits]

teemperor created this revision.
teemperor added reviewers: balazske, martong.
Herald added subscribers: cfe-commits, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

`ASTImporter::Imported` currently returns a Decl, but that return value is not 
used by the ASTImporter (or anywhere else)
nor is it documented.


Repository:
  rC Clang

https://reviews.llvm.org/D59595

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ExternalASTMerger.cpp
  lldb/include/lldb/Symbol/ClangASTImporter.h
  lldb/source/Symbol/ClangASTImporter.cpp


Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -937,7 +937,7 @@
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -262,7 +262,7 @@
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
Index: clang/lib/AST/ExternalASTMerger.cpp
===
--- clang/lib/AST/ExternalASTMerger.cpp
+++ clang/lib/AST/ExternalASTMerger.cpp
@@ -110,7 +110,7 @@
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's 
origin
   /// map is kept up to date.  Also set the appropriate flags.
-  Decl *Imported(Decl *From, Decl *To) override {
+  void Imported(Decl *From, Decl *To) override {
 if (auto *ToDC = dyn_cast(To)) {
   const bool LoggingEnabled = Parent.LoggingEnabled();
   if (LoggingEnabled)
@@ -153,7 +153,6 @@
   ToContainer->getPrimaryContext()->setMustBuildLookupTable();
   assert(Parent.CanComplete(ToContainer));
 }
-return To;
   }
   ASTImporter &GetReverse() { return Reverse; }
 };
Index: clang/include/clang/AST/ASTImporter.h
===
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -425,7 +425,7 @@
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);


Index: lldb/source/Symbol/ClangASTImporter.cpp
===
--- lldb/source/Symbol/ClangASTImporter.cpp
+++ lldb/source/Symbol/ClangASTImporter.cpp
@@ -937,7 +937,7 @@
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {
Index: lldb/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/include/lldb/Symbol/ClangASTImporter.h
@@ -262,7 +262,7 @@
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
Index: clang/lib/AST/ExternalASTMerger.cpp
===
--- clang/lib/AST/ExternalASTMerger.cpp
+++ clang/lib/AST/ExternalASTMerger.cpp
@@ -110,7 +110,7 @@
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's origin
   /// map is kept up to date.  Also set the appropriate flags.
-  Decl *Imported(Decl *From, Decl *To) override {
+  void Imported(Decl *From, Decl *To) override {
 if (auto *ToDC = dyn_cast(To)) {
   const bool LoggingEnabled = Parent.LoggingEnabled();
   if (LoggingEnabled)
@@ -153,7 +153,6 @@
   ToContainer->getPrimaryContext()->setMustBuildLookupTable();
  

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Could you do a rebase? I've tried to land it but there are too many merge 
conflicts currently.


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

https://reviews.llvm.org/D52150



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

[clang-tools-extra] r356547 - [clang-tidy] Parallelize clang-tidy-diff.py

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Wed Mar 20 04:30:05 2019
New Revision: 356547

URL: http://llvm.org/viewvc/llvm-project?rev=356547&view=rev
Log:
[clang-tidy] Parallelize clang-tidy-diff.py

This patch has 2 rationales:

- large patches lead to long command lines and often cause max command line 
length restrictions imposed by OS;
- clang-tidy runs on modified files are independent and can be done in 
parallel, the same as done for run-clang-tidy.

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


Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356547&r1=356546&r2=356547&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20 
04:30:05 2019
@@ -24,10 +24,88 @@ Example usage for git/svn users:
 """
 
 import argparse
+import glob
 import json
+import multiprocessing
+import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
+import threading
+import traceback
+import yaml
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if timeout is not None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except Exception as e:
+  with lock:
+sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' +
+   ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
+
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # The fixes suggested by clang-tidy >= 4.0.0 are given under
+  # the top level key 'Diagnostics' in the output yaml files
+  mergekey="Diagnostics"
+  merged=[]
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+content = yaml.safe_load(open(replacefile, 'r'))
+if not content:
+  continue # Skip empty files.
+merged.extend(content.get(mergekey, []))
+
+  if merged:
+# MainSourceFile: The key is required by the definition inside
+# include/clang/Tooling/ReplacementsYaml.h, but the value
+# is actually never used inside clang-apply-replacements,
+# so we set it to '' here.
+output = { 'MainSourceFile': '', mergekey: merged }
+with open(mergefile, 'w') as out:
+  yaml.safe_dump(output, out)
+  else:
+# Empty the file:
+open(mergefile, 'w').close()
 
 
 def main():
@@ -48,6 +126,10 @@ def main():
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=1,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +166,7 @@ def main():
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +184,83 @@ def main():
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',',

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE356547: [clang-tidy] Parallelize clang-tidy-diff.py 
(authored by zinovy.nis, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57662?vs=189562&id=191474#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py

Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -24,10 +24,88 @@
 """
 
 import argparse
+import glob
 import json
+import multiprocessing
+import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
+import threading
+import traceback
+import yaml
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if timeout is not None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except Exception as e:
+  with lock:
+sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' +
+   ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
+
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # The fixes suggested by clang-tidy >= 4.0.0 are given under
+  # the top level key 'Diagnostics' in the output yaml files
+  mergekey="Diagnostics"
+  merged=[]
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+content = yaml.safe_load(open(replacefile, 'r'))
+if not content:
+  continue # Skip empty files.
+merged.extend(content.get(mergekey, []))
+
+  if merged:
+# MainSourceFile: The key is required by the definition inside
+# include/clang/Tooling/ReplacementsYaml.h, but the value
+# is actually never used inside clang-apply-replacements,
+# so we set it to '' here.
+output = { 'MainSourceFile': '', mergekey: merged }
+with open(mergefile, 'w') as out:
+  yaml.safe_dump(output, out)
+  else:
+# Empty the file:
+open(mergefile, 'w').close()
 
 
 def main():
@@ -48,6 +126,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=1,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +166,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +184,83 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
-
-  quote = "";
-  if sys.platform == 'win32':
-line_filter_json=re.sub(r'"', r'"""', line_filter_json)
-  else:
-quote = "'";
+  max_task_count = args.j
+  if max_task_count == 0:
+  max_task_count = multiprocessing.cpu_count()
+  max_task_count = min(len(lines_by_file), max_task_count)
 
-  # Run clang-tidy on files containing changes.
-  command = [args.clang_tidy_binary]
-  command.append('-line-filter=' + quote + line_filter_

[clang-tools-extra] r356548 - [clang-tidy] Cosmetic fix

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Wed Mar 20 04:34:29 2019
New Revision: 356548

URL: http://llvm.org/viewvc/llvm-project?rev=356548&view=rev
Log:
[clang-tidy] Cosmetic fix

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


Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356548&r1=356547&r2=356548&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20 
04:34:29 2019
@@ -87,8 +87,8 @@ def merge_replacement_files(tmpdir, merg
   """Merge all replacement files in a directory into a single file"""
   # The fixes suggested by clang-tidy >= 4.0.0 are given under
   # the top level key 'Diagnostics' in the output yaml files
-  mergekey="Diagnostics"
-  merged=[]
+  mergekey = "Diagnostics"
+  merged = []
   for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
 content = yaml.safe_load(open(replacefile, 'r'))
 if not content:


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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping?


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

https://reviews.llvm.org/D57687



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:436
+static void
+AlignTokenSequenceAll(unsigned Start, unsigned End, unsigned Column,
+  F &&Matches,

VelocityRa wrote:
> klimek wrote:
> > I don't think the 'All' postfix in the name is helpful. What are you trying 
> > to say with that name?
> I'm not particularly fond of `All` either, suggestions welcome.
> 
> As the comment above explains, `All` refers to the fact that it operates on 
> all tokens, instead of being limited to certain cases like 
> `AlignTokenSequence` is. Maybe I should name //this// one 
> `AlignTokenSequence` and the other one `AlignTokenSequenceOuterScope`, or 
> something.
How about calling this AlignMacroSequence and the other one AlignMacros.
Comment here would be something like (as I don't think "scope" plays a role??)
// Aligns a sequence of macro definitions.

The problem is that I think the alignTokensAll don't make sense either as a 
function.
We should be able to inline this into alignConsecutiveMacros, and pull the 
std::function handed in either into its own function, or just define it as the 
first step in alignConsecutiveMacros.



Comment at: lib/Format/WhitespaceManager.cpp:437
+AlignTokenSequenceAll(unsigned Start, unsigned End, unsigned Column,
+  F &&Matches,
+  SmallVector &Changes) {

VelocityRa wrote:
> klimek wrote:
> > Why an rvalue ref? Also, this is used only once, so why make this a 
> > template?
> It's an rvalue ref, because that's also the case for `AlignTokenSequence` 
> (wasn't sure why, so I left it as is).
> It's used only once, but the function is more generic that way, no? That's 
> the point of its generic name.
> Tell me if I should change it.
I think we need to look at this from first principles. We can fix the old code 
later if it doesn't make sense :)



Comment at: lib/Format/WhitespaceManager.cpp:442
+
+  for (unsigned i = Start; i != End; ++i) {
+if (Changes[i].NewlinesBefore > 0) {

VelocityRa wrote:
> klimek wrote:
> > llvm style: use an upper case I for index vars.
> Ok, I assume your style changed because this is copied from 
> `AlignTokenSequence`.
Yea, sorry, those are in violation (we should fix that at some point, but *not* 
in this patch :)




Comment at: lib/Format/WhitespaceManager.cpp:500
+if (Changes[i].NewlinesBefore != 0) {
+  EndOfSequence = i;
+  // If there is a blank line, or if the last line didn't contain any

VelocityRa wrote:
> klimek wrote:
> > Why set EndOfSequence outside the if below?
> It's from `AlignTokens`. I think it's because due to some of the loop logic, 
> it ends up not checking up to the point of the the last token.
> Without setting this and calling `AlignCurrentSequence()` once more at the 
> end, the last line of a macro group does not get properly aligned, the tests 
> fail.
I was suggesting to move it inside the if below, did you try that (sounds like 
you tried to remove it).


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

https://reviews.llvm.org/D28462



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

r356551 - Correct this attribute group documentation to have a heading, which fixes the docs builder.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Mar 20 04:58:38 2019
New Revision: 356551

URL: http://llvm.org/viewvc/llvm-project?rev=356551&view=rev
Log:
Correct this attribute group documentation to have a heading, which fixes the 
docs builder.

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=356551&r1=356550&r2=356551&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Mar 20 04:58:38 2019
@@ -606,6 +606,7 @@ Query for this feature with ``__has_attr
 
 def PassObjectSizeDocs : Documentation {
   let Category = DocCatVariable; // Technically it's a parameter doc, but eh.
+  let Heading = "pass_object_size, pass_dynamic_object_size";
   let Content = [{
 .. Note:: The mangling of functions with parameters that are annotated with
   ``pass_object_size`` is subject to change. You can get around this by


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

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59520#1435728 , @sunfish wrote:

> In D59520#1434854 , @aaron.ballman 
> wrote:
>
> > > Removes errnoneous use of diag::err_alias_is_definition, which turned out 
> > > to be ineffective anyway since functions can be defined later in the 
> > > translation unit and avoid detection.
> >
> > If you need to keep the prohibition that these attributes not be applied to 
> > functions with definitions, there are ways to accomplish that (we merge 
> > declarations and can decide what to do at that point if we see a 
> > declaration that is a definition). Given that, do you still want to lift 
> > this restriction?
>
>
> These attributes don't make sense on definitions. So right now, they are 
> silently ignored when applied to definitions. That's effectively the current 
> behavior too, because the existing check doesn't work as intended, so the 
> change here doesn't change anything in practice yet.
>
> A diagnostic would be slightly tidier, but I'm not familiar enough with clang 
> to do this quickly and didn't want to delay the rest of the patch while I 
> investigated. Do you know the specific places we'd need to change to diagnose 
> this?


I'd be fine addressing it in a follow-up patch, but silently ignoring 
attributes is something I consider a bug. Users have a *very* difficult time 
seeing the difference between "accepted and doing its thing" and "silently 
ignored" depending on the attribute semantics, so that's why we explicitly tell 
them when we're ignoring an attribute.

As for where to handle this, I'd follow our typical "merge" pattern for 
attributes. 1) Add `Sema::mergeImportNameAttr()` that does all your error 
checking for definitions and creates an attribute if there's no issues, 2) From 
`handleImportNameAttr()`, handle your typical semantic checks and then calls 
`S.mergeImportNameAttr()` to try to create the attribute, and if it is created, 
attach it to the declaration, 3) hook the new merge operation up in 
`mergeDeclAttribute()` in SemaDecl.cpp.

If you don't address it as part of this patch, can you add a bug to bugzilla so 
that we get this tracked (if you don't plan to fix the issue soon) and add a 
test case to this patch demonstrating that we are silently ignoring the 
attribute with a FIXME comment?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59520



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(

lebedev.ri wrote:
> gribozavr wrote:
> > Same comment as in the other patch -- I would prefer that the source is 
> > inlined into the ASSERT_TRUE, with implicit string concatenation and "\n"s, 
> > if raw strings don't work.
> I still don't understand the reasoning here.
> It feels like a change just for the sake of the change.
These variables don't improve readability.  Also, if there are multiple such 
variables in one test, it is easy to mistakenly use an incorrect one in the 
ASSERT_TRUE line.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59595: Remove the unused return value in ASTImporter::Imported [NFC]

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

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

I would prefer an observer-like solution but I am not against this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59595



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2976
+
+return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+   Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||

If SLS_All is supposed to not change anything, should we fall through here if 
(SLS_All)?


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

https://reviews.llvm.org/D57687



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

[PATCH] D59546: [clang-format] structured binding in range for detected as Objective C

[Manuel Klimek via Phabricator via cfe-commits]

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

lg


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

https://reviews.llvm.org/D59546



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

I don't understand yet why running clang-format locally would not do the same 
change?


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

https://reviews.llvm.org/D54881



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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:33
+// If MergedSyms is provided, increments a symbols Reference count once for
+// every file it was mentioned in.
+void mergeRefs(llvm::ArrayRef> RefSlabs,

nit: s/every file/every slab/?

There is no "file" in this context.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:46
+auto It = MergedSyms->find(Sym.first);
+assert(It != MergedSyms->end() && "Reference to unknown symbol!");
+// Note that we increment References per each file mentioning the

I think we can trigger this assertion with an incomplete background index. For 
example, we've seen references of a symbol in some files but we've not parsed 
the file that contains the decls.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:50
+// FileIndex keeps only oneslab per file.
+// This contradicts with behavior inside clangd-indexer, it only counts
+// translation units since it processes each header multiple times.

kadircet wrote:
> ioeric wrote:
> > this is a bit of a code smell. FileIndex is not supposed to know anything 
> > about clangd-indexer etc.
> I've actually just left the comment for review so that we can decide what to 
> do about discrepancy(with my reasoning). It is not like FileIndex is somehow 
> tied to clangd-indexer now?
the comment is useful. I just think it's in the wrong place. If we define the 
reference counting behavior for `FileSymbols`, `FileSymbols`wouldn't need to 
know about clangd-indexer or background-index. This comment can then live in 
background index instead.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:170
   I.first->second = mergeSymbol(I.first->second, Sym);
+// We re-count number of references while merging refs from scratch.
+I.first->getSecond().References = 0;

kadircet wrote:
> ioeric wrote:
> > kadircet wrote:
> > > ioeric wrote:
> > > > note that references might not always exist. SymbolCollector can be set 
> > > > up to not collect references.
> > > I thought we wouldn't make any promises about `References` in such a 
> > > case, do we?
> > Is this documented somewhere?
> > 
> > `References` and `RefSlab` have been two independent things. `References` 
> > existed before `RefSlab`. Now might be a good time to unify them, but I 
> > think we should think about how this is reflected in their definitions 
> > (documentation etc) and other producers and consumers of the structures.
> Both of them are only produced by SymbolCollector, which has an option 
> `CountReferences` with a comment saying `// Populate the Symbol.References 
> field.`.
> Unfortunately that's not what it does, instead it always sets 
> `Symol.References` field to `1` for every symbol collected during indexing of 
> a single TU.
> Then it is up to the consumers to merge those `1`s. So even though we say:
> ```
> /// The number of translation units that reference this symbol from their main
> /// file. This number is only meaningful if aggregated in an index.```
> in the comments of `Symbol::References` it is totally up-to-the consumer who 
> performs the merging.
> 
> The option controls whether Symbol Occurences should be collected or not.
> 
> I see two possible options:
>  - Un-tie SymbolCollector from Symbol::References and rather define it as:
> ```
> /// This field is for the convenience of an aggregated symbol index
> ```
>  - Rather change Index structure's(consumers of this information) to 
> store(and build) this information internally if they plan to make use of it ?
> 
> WDYT?
> Unfortunately that's not what it does, instead it always sets 
> Symol.References field to 1 for every symbol collected during indexing of a 
> single TU.
Looking at the code, I think `Symol.References` is indeed only populated (i.e. 
set to 1) when `CountReferences` is enabled. Honestly, we should probably get 
rid of `CountReferences`; I don't see a reason not to count references. 

But the statement `count one per TU` still stands in SymbolCollector, as it 
always only handles one TU at a time. 

Actually, all I'm asking in the initial comment is to define the `References` 
counting behavior when there is no reference ;) I think a reasonable solution 
is to keep the `references` untouched when there is no reference, so that 
`References` aggregation via `mergeSymbol` can still work (for one-TU-per-Slab 
use cases). 

And we should probably also document the new behavior e.g. `DuplicateHandling` 
in FileIndex.h seems to be a good place.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:180
 }
 // FIXME: aggregate symbol reference count based on references.
 break;

this FIXME can be removed?



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:189

[clang-tools-extra] r356553 - Revert rL356547 : [clang-tidy] Cosmetic fix

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Wed Mar 20 06:24:33 2019
New Revision: 356553

URL: http://llvm.org/viewvc/llvm-project?rev=356553&view=rev
Log:
Revert rL356547 : [clang-tidy] Cosmetic fix
Differential Revision: https://reviews.llvm.org/D57662

[clang-tidy] Parallelize clang-tidy-diff.py

This patch has 2 rationales:

- large patches lead to long command lines and often cause max command line 
length restrictions imposed by OS;
- clang-tidy runs on modified files are independent and can be done in 
parallel, the same as done for run-clang-tidy.

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

Reverted to fix buildbot: 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/45547/steps/test/logs/stdio

Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356553&r1=356552&r2=356553&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20 
06:24:33 2019
@@ -24,88 +24,10 @@ Example usage for git/svn users:
 """
 
 import argparse
-import glob
 import json
-import multiprocessing
-import os
 import re
-import shutil
 import subprocess
 import sys
-import tempfile
-import threading
-import traceback
-import yaml
-
-is_py2 = sys.version[0] == '2'
-
-if is_py2:
-import Queue as queue
-else:
-import queue as queue
-
-
-def run_tidy(task_queue, lock, timeout):
-  watchdog = None
-  while True:
-command = task_queue.get()
-try:
-  proc = subprocess.Popen(command,
-  stdout=subprocess.PIPE,
-  stderr=subprocess.PIPE)
-
-  if timeout is not None:
-watchdog = threading.Timer(timeout, proc.kill)
-watchdog.start()
-
-  stdout, stderr = proc.communicate()
-
-  with lock:
-sys.stdout.write(stdout.decode('utf-8') + '\n')
-if stderr:
-  sys.stderr.write(stderr.decode('utf-8') + '\n')
-except Exception as e:
-  with lock:
-sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
-finally:
-  with lock:
-if (not timeout is None) and (not watchdog is None):
-  if not watchdog.is_alive():
-  sys.stderr.write('Terminated by timeout: ' +
-   ' '.join(command) + '\n')
-  watchdog.cancel()
-  task_queue.task_done()
-
-
-def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
-  for _ in range(max_tasks):
-t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
-t.daemon = True
-t.start()
-
-def merge_replacement_files(tmpdir, mergefile):
-  """Merge all replacement files in a directory into a single file"""
-  # The fixes suggested by clang-tidy >= 4.0.0 are given under
-  # the top level key 'Diagnostics' in the output yaml files
-  mergekey = "Diagnostics"
-  merged = []
-  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
-content = yaml.safe_load(open(replacefile, 'r'))
-if not content:
-  continue # Skip empty files.
-merged.extend(content.get(mergekey, []))
-
-  if merged:
-# MainSourceFile: The key is required by the definition inside
-# include/clang/Tooling/ReplacementsYaml.h, but the value
-# is actually never used inside clang-apply-replacements,
-# so we set it to '' here.
-output = { 'MainSourceFile': '', mergekey: merged }
-with open(mergefile, 'w') as out:
-  yaml.safe_dump(output, out)
-  else:
-# Empty the file:
-open(mergefile, 'w').close()
 
 
 def main():
@@ -126,10 +48,6 @@ def main():
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
-  parser.add_argument('-j', type=int, default=1,
-  help='number of tidy instances to be run in parallel.')
-  parser.add_argument('-timeout', type=int, default=None,
-  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -166,7 +84,7 @@ def main():
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename is None:
+if filename == None:
   continue
 
 if args.regex is not None:
@@ -184,83 +102,44 @@ def main():
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1
+  end_line = start_line + line_count - 1;
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  

[PATCH] D59560: Permit redeclarations of a builtin to specify calling convention.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 191483.
erichkeane added a comment.

Woops!  Look like I left without actually changing the revision *shame*.

Also, I was an idiot about SemaType, please disregard my previous comment.


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

https://reviews.llvm.org/D59560

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaType.cpp
  test/CodeGen/aarch64-vpcs.c
  test/Sema/callingconv-iamcu.c
  test/Sema/callingconv.c
  test/Sema/pr25786.c
  test/Sema/stdcall-fastcall-x64.c
  test/SemaCUDA/cuda-inherits-calling-conv.cu
  test/SemaCXX/borland-extensions.cpp
  test/SemaCXX/cxx11-gnu-attrs.cpp
  test/SemaCXX/virtual-override-x64.cpp
  test/SemaTemplate/instantiate-function-params.cpp

Index: test/SemaTemplate/instantiate-function-params.cpp
===
--- test/SemaTemplate/instantiate-function-params.cpp
+++ test/SemaTemplate/instantiate-function-params.cpp
@@ -88,7 +88,7 @@
 __attribute__((stdcall)) functype stdfunc1;
 stdfunctype stdfunc2;
 
-__attribute__((pcs("aapcs"))) functype pcsfunc; // expected-warning {{calling convention 'pcs' ignored for this target}}
+__attribute__((pcs("aapcs"))) functype pcsfunc; // expected-warning {{'pcs' calling convention ignored for this target}}
   };
 
   void f(X x) {
Index: test/SemaCXX/virtual-override-x64.cpp
===
--- test/SemaCXX/virtual-override-x64.cpp
+++ test/SemaCXX/virtual-override-x64.cpp
@@ -6,7 +6,7 @@
 namespace PR14339 {
   class A {
   public:
-virtual void __attribute__((thiscall)) f();	// expected-warning {{calling convention 'thiscall' ignored for this target}}
+virtual void __attribute__((thiscall)) f();	// expected-warning {{'thiscall' calling convention ignored for this target}}
   };
 
   class B : public A {
@@ -16,7 +16,7 @@
 
   class C : public A {
   public:
-void __attribute__((thiscall)) f();  // expected-warning {{calling convention 'thiscall' ignored for this target}}
+void __attribute__((thiscall)) f();  // expected-warning {{'thiscall' calling convention ignored for this target}}
   };
 
   class D : public A {
@@ -26,7 +26,7 @@
 
   class E {
   public:
-virtual void __attribute__((stdcall)) g();  // expected-warning {{calling convention 'stdcall' ignored for this target}}
+virtual void __attribute__((stdcall)) g();  // expected-warning {{'stdcall' calling convention ignored for this target}}
   };
 
   class F : public E {
Index: test/SemaCXX/cxx11-gnu-attrs.cpp
===
--- test/SemaCXX/cxx11-gnu-attrs.cpp
+++ test/SemaCXX/cxx11-gnu-attrs.cpp
@@ -9,18 +9,18 @@
 int *[[gnu::unused]] attr_on_ptr;
 // expected-warning@-1 {{attribute 'unused' ignored, because it cannot be applied to a type}}
 [[gnu::fastcall]] void pr17424_1();
-// expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
+// expected-warning@-1 {{'fastcall' calling convention ignored for this target}}
 [[gnu::fastcall]] [[gnu::stdcall]] void pr17424_2();
-// expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
-// expected-warning@-2 {{calling convention 'stdcall' ignored for this target}}
+// expected-warning@-1 {{'fastcall' calling convention ignored for this target}}
+// expected-warning@-2 {{'stdcall' calling convention ignored for this target}}
 [[gnu::fastcall]] __stdcall void pr17424_3();
-// expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
-// expected-warning@-2 {{calling convention '__stdcall' ignored for this target}}
+// expected-warning@-1 {{'fastcall' calling convention ignored for this target}}
+// expected-warning@-2 {{'__stdcall' calling convention ignored for this target}}
 [[gnu::fastcall]] void pr17424_4() [[gnu::stdcall]];
-// expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
-// expected-warning@-2 {{calling convention 'stdcall' ignored for this target}}
+// expected-warning@-1 {{'fastcall' calling convention ignored for this target}}
+// expected-warning@-2 {{'stdcall' calling convention ignored for this target}}
 void pr17424_5 [[gnu::fastcall]]();
-// expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
+// expected-warning@-1 {{'fastcall' calling convention ignored for this target}}
 
 // Valid cases.
 
Index: test/SemaCXX/borland-extensions.cpp
===
--- test/SemaCXX/borland-extensions.cpp
+++ test/SemaCXX/borland-extensions.cpp
@@ -7,21 +7,21 @@
 int dummy_function() { return 0; }
 
 // 2. test __pascal
-// expected-warning@+1 {{calling convention '_pascal' ignored for this target}}
+// expected-warning@+1 {{'_pascal' calling convention ignored for this target}}
 int _pascal f2();
 
-// expected

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[Mark Zeren via Phabricator via cfe-commits]

mzeren-vmw added a comment.

In D55170#1436087 , @klimek wrote:

> Oh wow, wasn't aware how popular this is. Sigh. I'd like us to push them to 
> change their style guide, but I guess that's not going to fly.


IDK. If the criteria is "public coding standard for widely used project", 
supporting all such things may be unsustainable. Some projects may need to fork 
or find another formatter. I speak as someone that maintains an internal fork 
with features that I do not deem worthy of foisting on trunk.
(And BTW @MyDeveloperDay, whoever you are, Thanks for all of your recent 
activity!)


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

https://reviews.llvm.org/D55170



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

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[Reuben Thomas via Phabricator via cfe-commits]

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



Comment at: lib/Format/TokenAnnotator.cpp:2546-2560
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
-   tok::kw_new, tok::kw_delete) &&
-  (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
- (Left.is(tok::r_square) && Left.MatchingParen &&
-  Left.MatchingParen->is(TT_LambdaLSquare))) &&
-Line.Type != LT_PreprocessorDirective);
+((Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, 
tok::kw_while,
+   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+   TT_ObjCForIn) ||
+  Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
+  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,

klimek wrote:
> I'm really confused about the changes os parens. It seems like this should 
> just change the spaceRequiredBeforeParens(...), but instead seems to change 
> parens in a way that completely changes the logic? (or alternatively is 
> phabricator showing this wrong?)
I think this looks like a bigger change than it really is because I added an 
open-parens on line 2548, which then affected the formatting of the following 
lines. I also added the `isSimpleTypeSpecifier` check, so that functional-style 
casts (`int (foo)`) are given the correct spacing.


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

https://reviews.llvm.org/D55170



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

[PATCH] D59533: [X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.

[Simon Pilgrim via Phabricator via cfe-commits]

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

LGTM with one minor (doxygen desciptions)




Comment at: lib/Headers/ia32intrin.h:58
 
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)

Please can you copy the doxygen comments from smmintrin.h (_mm_crc32_u8 etc.)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59533



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

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Tolga Mizrak via Phabricator via cfe-commits]

to-miz added a comment.

Is it because the diff was made with the svn repository? It is for the newest 
revision.
Should I make a new diff with the git repo instead?
Or is it not because of the diff, but something in phabricator instead?
Sorry this is my first patch.


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

https://reviews.llvm.org/D52150



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

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:2546-2560
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
-   tok::kw_new, tok::kw_delete) &&
-  (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
- (Left.is(tok::r_square) && Left.MatchingParen &&
-  Left.MatchingParen->is(TT_LambdaLSquare))) &&
-Line.Type != LT_PreprocessorDirective);
+((Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, 
tok::kw_while,
+   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+   TT_ObjCForIn) ||
+  Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
+  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,

reuk wrote:
> klimek wrote:
> > I'm really confused about the changes os parens. It seems like this should 
> > just change the spaceRequiredBeforeParens(...), but instead seems to change 
> > parens in a way that completely changes the logic? (or alternatively is 
> > phabricator showing this wrong?)
> I think this looks like a bigger change than it really is because I added an 
> open-parens on line 2548, which then affected the formatting of the following 
> lines. I also added the `isSimpleTypeSpecifier` check, so that 
> functional-style casts (`int (foo)`) are given the correct spacing.
a) why did you add the one in 2548? you didn't change any of the logic, right?
b) there are 3 extra closing parens in 2560, I see one more new opening in 
2556, but that also seems superfluous?
One question is whether we should pull this apart into bools with names that 
make sense :)


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

https://reviews.llvm.org/D55170



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

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D52150#1436278 , @to-miz wrote:

> Is it because the diff was made with the svn repository? It is for the newest 
> revision.
>  Should I make a new diff with the git repo instead?
>  Or is it not because of the diff, but something in phabricator instead?
>  Sorry this is my first patch.


No worries. It is just because you've made your last update at 2nd of 
march(assuming you've rebased before that, otherwise it might be as old as last 
september) and its 20th today, there are other people making changes, which 
simply conflicts with yours.


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

https://reviews.llvm.org/D52150



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: include/clang/AST/Stmt.h:102
+/// This bit is set only for the Stmt's that are the structured-block
+/// of OpeMP [non-standalone] executable directives.
+/// I.e. those returned by OMPExecutableDirective::getStructuredBlock().

gribozavr wrote:
> "non-standalone OpenMP executable directives"
English isn't my native language; to me the current variant (attempts to?) 
convey
that structured block can only ever exist for non-standalone executable 
directives.
I.e. "structured block of non-standalone executable directives" is a misnomer.

The variant without `[]` does not convey any of that, i think, it just says
that "it's a structured block of non-standalone OpenMP executable directives",
it isn't obvious from that (without knowing OpenMP details) that there 
wouldn't be "a structured block of standalone OpenMP executable directive".

Isn't there an important difference there?



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(

gribozavr wrote:
> lebedev.ri wrote:
> > gribozavr wrote:
> > > Same comment as in the other patch -- I would prefer that the source is 
> > > inlined into the ASSERT_TRUE, with implicit string concatenation and 
> > > "\n"s, if raw strings don't work.
> > I still don't understand the reasoning here.
> > It feels like a change just for the sake of the change.
> These variables don't improve readability.  Also, if there are multiple such 
> variables in one test, it is easy to mistakenly use an incorrect one in the 
> ASSERT_TRUE line.
Can you please quote something about this from
https://llvm.org/docs/CodingStandards.html
https://llvm.org/docs/ProgrammersManual.html
etc?
If not this is entirely subjective/stylistic,
and this was the form that was more readable to me.
(easier to write, too, those "\n" will look horrible)
Also, what about the cases with two assertions? (not just the new ones)

I really don't see how that would be a good change.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:656
+
+TEST(OMPStructuredBlock, DISABLED_TestParallelMaster0XFAIL) {
+  const char *Source =

gribozavr wrote:
> Is it only a pretty-printer problem or something more severe?
[[ https://bugs.llvm.org/show_bug.cgi?id=41022 | PR41022 ]].


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:170
   I.first->second = mergeSymbol(I.first->second, Sym);
+// We re-count number of references while merging refs from scratch.
+I.first->getSecond().References = 0;

ioeric wrote:
> kadircet wrote:
> > ioeric wrote:
> > > kadircet wrote:
> > > > ioeric wrote:
> > > > > note that references might not always exist. SymbolCollector can be 
> > > > > set up to not collect references.
> > > > I thought we wouldn't make any promises about `References` in such a 
> > > > case, do we?
> > > Is this documented somewhere?
> > > 
> > > `References` and `RefSlab` have been two independent things. `References` 
> > > existed before `RefSlab`. Now might be a good time to unify them, but I 
> > > think we should think about how this is reflected in their definitions 
> > > (documentation etc) and other producers and consumers of the structures.
> > Both of them are only produced by SymbolCollector, which has an option 
> > `CountReferences` with a comment saying `// Populate the Symbol.References 
> > field.`.
> > Unfortunately that's not what it does, instead it always sets 
> > `Symol.References` field to `1` for every symbol collected during indexing 
> > of a single TU.
> > Then it is up to the consumers to merge those `1`s. So even though we say:
> > ```
> > /// The number of translation units that reference this symbol from their 
> > main
> > /// file. This number is only meaningful if aggregated in an index.```
> > in the comments of `Symbol::References` it is totally up-to-the consumer 
> > who performs the merging.
> > 
> > The option controls whether Symbol Occurences should be collected or not.
> > 
> > I see two possible options:
> >  - Un-tie SymbolCollector from Symbol::References and rather define it as:
> > ```
> > /// This field is for the convenience of an aggregated symbol index
> > ```
> >  - Rather change Index structure's(consumers of this information) to 
> > store(and build) this information internally if they plan to make use of it 
> > ?
> > 
> > WDYT?
> > Unfortunately that's not what it does, instead it always sets 
> > Symol.References field to 1 for every symbol collected during indexing of a 
> > single TU.
> Looking at the code, I think `Symol.References` is indeed only populated 
> (i.e. set to 1) when `CountReferences` is enabled. Honestly, we should 
> probably get rid of `CountReferences`; I don't see a reason not to count 
> references. 
> 
> But the statement `count one per TU` still stands in SymbolCollector, as it 
> always only handles one TU at a time. 
> 
> Actually, all I'm asking in the initial comment is to define the `References` 
> counting behavior when there is no reference ;) I think a reasonable solution 
> is to keep the `references` untouched when there is no reference, so that 
> `References` aggregation via `mergeSymbol` can still work (for 
> one-TU-per-Slab use cases). 
> 
> And we should probably also document the new behavior e.g. 
> `DuplicateHandling` in FileIndex.h seems to be a good place.
> Honestly, we should probably get rid of CountReferences; I don't see a reason 
> not to count references
We introduced it to avoid getting partial counts in dynamic index. In the 
absence of the static index, they would skew ranking towards symbols used 
inside the files you have open, which is not really a good signal in the first 
place.

I'm not sure if that actually hurts much in practice, though. It could very 
possibly be the case that the added complexity isn't worth it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Nick Renieris via Phabricator via cfe-commits]

VelocityRa updated this revision to Diff 191493.
VelocityRa marked 13 inline comments as done.
VelocityRa added a comment.

Diff updated.
Some comments about the inline changes, since the rest were trivial:

- `AlignMacroSequence` was inlined
- `AlignMacros` was after that
- `AlignMacrosMatches` can't be inlined because it's used both by 
`alignConsecutiveMacros`'s (pre-`AlignMacros`) `AlignCurrentSequence` lambda 
and `alignConsecutiveMacros` itself.
- `AlignCurrentSequence` can't be inlined because it's used three times in 
`alignConsecutiveMacros`.


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

https://reviews.llvm.org/D28462

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9544,8 +9544,104 @@
NoSpaceStyle);
 }
 
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveDeclarations = true;
+  Style.AlignConsecutiveMacros = false;
+
+  verifyFormat("#define a 3\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style);
+
+  verifyFormat("#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y) (x - y)",
+   Style);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+   "#define bar (5, 6)(2 + 2)",
+   Style);
+
+  verifyFormat("#define a 3\n"
+   "#define  4\n"
+   "#define ccc (5)\n"
+   "#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y) (x - y)",
+   Style);
+
+  Style.AlignConsecutiveMacros = true;
+  verifyFormat("#define a3\n"
+   "#define  4\n"
+   "#define ccc  (5)",
+   Style);
+
+  verifyFormat("#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y)   (x - y)",
+   Style);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+   "#define bar   (5, 6)(2 + 2)",
+   Style);
+
+  verifyFormat("#define a3\n"
+   "#define  4\n"
+   "#define ccc  (5)\n"
+   "#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y)   (x - y)",
+   Style);
+
+  verifyFormat("#define a 5\n"
+   "#define foo(x, y) (x + y)\n"
+   "#define CCC   (6)\n"
+   "auto lambda = []() {\n"
+   "  auto  ii = 0;\n"
+   "  float j  = 0;\n"
+   "  return 0;\n"
+   "};\n"
+   "int   i  = 0;\n"
+   "float i2 = 0;\n"
+   "auto  v  = type{\n"
+   "i = 1,   //\n"
+   "(i = 2), //\n"
+   "i = 3//\n"
+   "};",
+   Style);
+
+  Style.AlignConsecutiveMacros = false;
+  Style.ColumnLimit = 20;
+
+  verifyFormat("#define a  \\\n"
+   "  \"aa\"\n"
+   "#define D  \\\n"
+   "  \"aa\" \\\n"
+   "  \"ccdde\"\n"
+   "#define B  \\\n"
+   "  \"Q\"  \\\n"
+   "  \"F\"  \\\n"
+   "  \"\"\n",
+   Style);
+
+  Style.AlignConsecutiveMacros = true;
+  verifyFormat("#define a  \\\n"
+   "  \"aa\"\n"
+   "#define D  \\\n"
+   "  \"aa\" \\\n"
+   "  \"ccdde\"\n"
+   "#define B  \\\n"
+   "  \"Q\"  \\\n"
+   "  \"F\"  \\\n"
+   "  \"\"\n",
+   Style);
+}
+
 TEST_F(FormatTest, AlignConsecutiveAssignments) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveAssignments = false;
   verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -9735,6 +9831,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveDeclarations) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveDeclarations = false;
   verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -10856,6 +10953,7 @@
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignOperands);
   CHECK_PARSE_BOOL(AlignTrailingComments);
+  CHECK_PARSE_BOOL(AlignConsecutiveMacros);
   CH

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Nick Renieris via Phabricator via cfe-commits]

VelocityRa added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:500
+if (Changes[i].NewlinesBefore != 0) {
+  EndOfSequence = i;
+  // If there is a blank line, or if the last line didn't contain any

klimek wrote:
> VelocityRa wrote:
> > klimek wrote:
> > > Why set EndOfSequence outside the if below?
> > It's from `AlignTokens`. I think it's because due to some of the loop 
> > logic, it ends up not checking up to the point of the the last token.
> > Without setting this and calling `AlignCurrentSequence()` once more at the 
> > end, the last line of a macro group does not get properly aligned, the 
> > tests fail.
> I was suggesting to move it inside the if below, did you try that (sounds 
> like you tried to remove it).
I moved it so that the below `if` is:
```
  if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine) {
EndOfSequence = i;
AlignCurrentSequence();
  }
```
and it did not work correctly.


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

https://reviews.llvm.org/D28462



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: include/clang/AST/Stmt.h:102
+/// This bit is set only for the Stmt's that are the structured-block
+/// of OpeMP [non-standalone] executable directives.
+/// I.e. those returned by OMPExecutableDirective::getStructuredBlock().

lebedev.ri wrote:
> gribozavr wrote:
> > "non-standalone OpenMP executable directives"
> English isn't my native language; to me the current variant (attempts to?) 
> convey
> that structured block can only ever exist for non-standalone executable 
> directives.
> I.e. "structured block of non-standalone executable directives" is a misnomer.
> 
> The variant without `[]` does not convey any of that, i think, it just says
> that "it's a structured block of non-standalone OpenMP executable directives",
> it isn't obvious from that (without knowing OpenMP details) that there 
> wouldn't be "a structured block of standalone OpenMP executable directive".
> 
> Isn't there an important difference there?
I didn't catch this difference implied by "[]".

> I.e. "structured block of non-standalone executable directives" is a misnomer.

Why is it a misnomer?  It might be overly verbose, but it is correct.

However, I see that this fact is important to highlight, so I'd suggest to 
spell it out:

"This bit is set only for the Stmts that are the structured-block of OpenMP 
executable directives.  Directives that have a structured block are called 
"non-standalone" directives."

Please also fix the typo "OpeMP" => OpenMP.



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(

lebedev.ri wrote:
> gribozavr wrote:
> > lebedev.ri wrote:
> > > gribozavr wrote:
> > > > Same comment as in the other patch -- I would prefer that the source is 
> > > > inlined into the ASSERT_TRUE, with implicit string concatenation and 
> > > > "\n"s, if raw strings don't work.
> > > I still don't understand the reasoning here.
> > > It feels like a change just for the sake of the change.
> > These variables don't improve readability.  Also, if there are multiple 
> > such variables in one test, it is easy to mistakenly use an incorrect one 
> > in the ASSERT_TRUE line.
> Can you please quote something about this from
> https://llvm.org/docs/CodingStandards.html
> https://llvm.org/docs/ProgrammersManual.html
> etc?
> If not this is entirely subjective/stylistic,
> and this was the form that was more readable to me.
> (easier to write, too, those "\n" will look horrible)
> Also, what about the cases with two assertions? (not just the new ones)
> 
> I really don't see how that would be a good change.
There are lots of readability points that are not formalized in the coding 
style, and yet, are followed in the code.

For cases with two assertions we need to have a variable to avoid code 
duplication, so it is OK there -- the variable has a purpose.

If you want to use raw string literals, could I at least ask you to drop the 
first line onto the same level of indentation as the rest of the lines?

```
  const char *Source =
  R"(void test(int i) {
#pragma omp atomic
++i;
})";
```

change to:

```
  const char *Source = R"(
void test(int i) {
#pragma omp atomic
++i;
})";
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.

LG for the OpenMP part


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Tolga Mizrak via Phabricator via cfe-commits]

to-miz added a comment.

I checked out the git repository and applied the patch to head and reran all 
tests.
The diff produced by git is basically the same, should I upload the new diff?


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

https://reviews.llvm.org/D52150



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

r356564 - [ASTImporter] Remove obsolete function ImportTemplateParameterList.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Wed Mar 20 08:42:42 2019
New Revision: 356564

URL: http://llvm.org/viewvc/llvm-project?rev=356564&view=rev
Log:
[ASTImporter] Remove obsolete function ImportTemplateParameterList.

Summary:
The ASTNodeImporter::ImportTemplateParameterList is replaced by a
template specialization of 'import' that already exists and does
(almost) the same thing.

Reviewers: martong, a.sidorin, shafik, a_sidorin

Reviewed By: martong

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=356564&r1=356563&r2=356564&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Mar 20 08:42:42 2019
@@ -402,8 +402,6 @@ namespace clang {
 Error ImportDefinition(
 ObjCProtocolDecl *From, ObjCProtocolDecl *To,
 ImportDefinitionKind Kind = IDK_Default);
-Expected ImportTemplateParameterList(
-TemplateParameterList *Params);
 Error ImportTemplateArguments(
 const TemplateArgument *FromArgs, unsigned NumFromArgs,
 SmallVectorImpl &ToArgs);
@@ -1899,40 +1897,6 @@ Error ASTNodeImporter::ImportDefinition(
   return Error::success();
 }
 
-// FIXME: Remove this, use `import` instead.
-Expected ASTNodeImporter::ImportTemplateParameterList(
-TemplateParameterList *Params) {
-  SmallVector ToParams(Params->size());
-  if (Error Err = ImportContainerChecked(*Params, ToParams))
-return std::move(Err);
-
-  Expr *ToRequiresClause;
-  if (Expr *const R = Params->getRequiresClause()) {
-if (Error Err = importInto(ToRequiresClause, R))
-  return std::move(Err);
-  } else {
-ToRequiresClause = nullptr;
-  }
-
-  auto ToTemplateLocOrErr = import(Params->getTemplateLoc());
-  if (!ToTemplateLocOrErr)
-return ToTemplateLocOrErr.takeError();
-  auto ToLAngleLocOrErr = import(Params->getLAngleLoc());
-  if (!ToLAngleLocOrErr)
-return ToLAngleLocOrErr.takeError();
-  auto ToRAngleLocOrErr = import(Params->getRAngleLoc());
-  if (!ToRAngleLocOrErr)
-return ToRAngleLocOrErr.takeError();
-
-  return TemplateParameterList::Create(
-  Importer.getToContext(),
-  *ToTemplateLocOrErr,
-  *ToLAngleLocOrErr,
-  ToParams,
-  *ToRAngleLocOrErr,
-  ToRequiresClause);
-}
-
 Error ASTNodeImporter::ImportTemplateArguments(
 const TemplateArgument *FromArgs, unsigned NumFromArgs,
 SmallVectorImpl &ToArgs) {
@@ -3479,7 +3443,7 @@ ExpectedDecl ASTNodeImporter::VisitFrien
   SmallVector ToTPLists(D->NumTPLists);
   auto **FromTPLists = D->getTrailingObjects();
   for (unsigned I = 0; I < D->NumTPLists; I++) {
-if (auto ListOrErr = ImportTemplateParameterList(FromTPLists[I]))
+if (auto ListOrErr = import(FromTPLists[I]))
   ToTPLists[I] = *ListOrErr;
 else
   return ListOrErr.takeError();
@@ -4914,8 +4878,7 @@ ASTNodeImporter::VisitTemplateTemplatePa
 return LocationOrErr.takeError();
 
   // Import template parameters.
-  auto TemplateParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto TemplateParamsOrErr = import(D->getTemplateParameters());
   if (!TemplateParamsOrErr)
 return TemplateParamsOrErr.takeError();
 
@@ -5002,8 +4965,7 @@ ExpectedDecl ASTNodeImporter::VisitClass
 return std::move(Err);
 
   // Create the class template declaration itself.
-  auto TemplateParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto TemplateParamsOrErr = import(D->getTemplateParameters());
   if (!TemplateParamsOrErr)
 return TemplateParamsOrErr.takeError();
 
@@ -5135,8 +5097,7 @@ ExpectedDecl ASTNodeImporter::VisitClass
   return std::move(Err);
 CanonInjType = CanonInjType.getCanonicalType();
 
-auto ToTPListOrErr = ImportTemplateParameterList(
-PartialSpec->getTemplateParameters());
+auto ToTPListOrErr = import(PartialSpec->getTemplateParameters());
 if (!ToTPListOrErr)
   return ToTPListOrErr.takeError();
 
@@ -5290,8 +5251,7 @@ ExpectedDecl ASTNodeImporter::VisitVarTe
 return std::move(Err);
 
   // Create the variable template declaration itself.
-  auto TemplateParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto TemplateParamsOrErr = import(D->getTemplateParameters());
   if (!TemplateParamsOrErr)
 return TemplateParamsOrErr.takeError();
 
@@ -5396,8 +5356,7 @@ ExpectedDecl ASTNodeImporter::VisitVarTe
   *FromTAArgsAsWritten, ArgInfos))
 return std::move(Err);
 
-  auto ToTPListOrErr = ImportTemplateParameterList(
-  FromPartial->getTemplateParameters());
+  auto ToTPListOrErr = import(FromPartial->getTemplateParameters());
   if (!ToTPListOrErr)
 return To

[PATCH] D59134: [ASTImporter] Remove obsolete function ImportTemplateParameterList.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356564: [ASTImporter] Remove obsolete function 
ImportTemplateParameterList. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59134?vs=189855&id=191502#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59134

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -402,8 +402,6 @@
 Error ImportDefinition(
 ObjCProtocolDecl *From, ObjCProtocolDecl *To,
 ImportDefinitionKind Kind = IDK_Default);
-Expected ImportTemplateParameterList(
-TemplateParameterList *Params);
 Error ImportTemplateArguments(
 const TemplateArgument *FromArgs, unsigned NumFromArgs,
 SmallVectorImpl &ToArgs);
@@ -1899,40 +1897,6 @@
   return Error::success();
 }
 
-// FIXME: Remove this, use `import` instead.
-Expected ASTNodeImporter::ImportTemplateParameterList(
-TemplateParameterList *Params) {
-  SmallVector ToParams(Params->size());
-  if (Error Err = ImportContainerChecked(*Params, ToParams))
-return std::move(Err);
-
-  Expr *ToRequiresClause;
-  if (Expr *const R = Params->getRequiresClause()) {
-if (Error Err = importInto(ToRequiresClause, R))
-  return std::move(Err);
-  } else {
-ToRequiresClause = nullptr;
-  }
-
-  auto ToTemplateLocOrErr = import(Params->getTemplateLoc());
-  if (!ToTemplateLocOrErr)
-return ToTemplateLocOrErr.takeError();
-  auto ToLAngleLocOrErr = import(Params->getLAngleLoc());
-  if (!ToLAngleLocOrErr)
-return ToLAngleLocOrErr.takeError();
-  auto ToRAngleLocOrErr = import(Params->getRAngleLoc());
-  if (!ToRAngleLocOrErr)
-return ToRAngleLocOrErr.takeError();
-
-  return TemplateParameterList::Create(
-  Importer.getToContext(),
-  *ToTemplateLocOrErr,
-  *ToLAngleLocOrErr,
-  ToParams,
-  *ToRAngleLocOrErr,
-  ToRequiresClause);
-}
-
 Error ASTNodeImporter::ImportTemplateArguments(
 const TemplateArgument *FromArgs, unsigned NumFromArgs,
 SmallVectorImpl &ToArgs) {
@@ -3479,7 +3443,7 @@
   SmallVector ToTPLists(D->NumTPLists);
   auto **FromTPLists = D->getTrailingObjects();
   for (unsigned I = 0; I < D->NumTPLists; I++) {
-if (auto ListOrErr = ImportTemplateParameterList(FromTPLists[I]))
+if (auto ListOrErr = import(FromTPLists[I]))
   ToTPLists[I] = *ListOrErr;
 else
   return ListOrErr.takeError();
@@ -4914,8 +4878,7 @@
 return LocationOrErr.takeError();
 
   // Import template parameters.
-  auto TemplateParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto TemplateParamsOrErr = import(D->getTemplateParameters());
   if (!TemplateParamsOrErr)
 return TemplateParamsOrErr.takeError();
 
@@ -5002,8 +4965,7 @@
 return std::move(Err);
 
   // Create the class template declaration itself.
-  auto TemplateParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto TemplateParamsOrErr = import(D->getTemplateParameters());
   if (!TemplateParamsOrErr)
 return TemplateParamsOrErr.takeError();
 
@@ -5135,8 +5097,7 @@
   return std::move(Err);
 CanonInjType = CanonInjType.getCanonicalType();
 
-auto ToTPListOrErr = ImportTemplateParameterList(
-PartialSpec->getTemplateParameters());
+auto ToTPListOrErr = import(PartialSpec->getTemplateParameters());
 if (!ToTPListOrErr)
   return ToTPListOrErr.takeError();
 
@@ -5290,8 +5251,7 @@
 return std::move(Err);
 
   // Create the variable template declaration itself.
-  auto TemplateParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto TemplateParamsOrErr = import(D->getTemplateParameters());
   if (!TemplateParamsOrErr)
 return TemplateParamsOrErr.takeError();
 
@@ -5396,8 +5356,7 @@
   *FromTAArgsAsWritten, ArgInfos))
 return std::move(Err);
 
-  auto ToTPListOrErr = ImportTemplateParameterList(
-  FromPartial->getTemplateParameters());
+  auto ToTPListOrErr = import(FromPartial->getTemplateParameters());
   if (!ToTPListOrErr)
 return ToTPListOrErr.takeError();
 
@@ -5505,8 +5464,7 @@
 }
   }
 
-  auto ParamsOrErr = ImportTemplateParameterList(
-  D->getTemplateParameters());
+  auto ParamsOrErr = import(D->getTemplateParameters());
   if (!ParamsOrErr)
 return ParamsOrErr.takeError();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58531: [clang] Specify type of pthread_create builtin

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:9574
 
-  assert(!RequiresICE && "Result of intrinsic cannot be required to be an 
ICE");
+  assert(!RequiresICE && "Result of function cannot be required to be an ICE");
 

I would just remove "of function" here, since it isn't correct when you are not 
decoding a function, and the two words don't make that much more meaningful 
even in the original case, so I don't think it justifies something like "of 
intrinsic/function".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58531



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

[clang-tools-extra] r356565 - Reland r356547 after fixing the tests for Linux.

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Wed Mar 20 08:50:26 2019
New Revision: 356565

URL: http://llvm.org/viewvc/llvm-project?rev=356565&view=rev
Log:
Reland r356547 after fixing the tests for Linux.

[clang-tidy] Parallelize clang-tidy-diff.py

This patch has 2 rationales:

- large patches lead to long command lines and often cause max command line 
length restrictions imposed by OS;
- clang-tidy runs on modified files are independent and can be done in 
parallel, the same as done for run-clang-tidy.

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


Modified:
clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py?rev=356565&r1=356564&r2=356565&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/clang-tidy-diff.py Wed Mar 20 
08:50:26 2019
@@ -24,10 +24,90 @@ Example usage for git/svn users:
 """
 
 import argparse
+import glob
 import json
+import multiprocessing
+import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
+import threading
+import traceback
+import yaml
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if timeout is not None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(stdout.decode('utf-8') + '\n')
+sys.stdout.flush()
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+  sys.stderr.flush()
+except Exception as e:
+  with lock:
+sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' +
+   ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
+
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # The fixes suggested by clang-tidy >= 4.0.0 are given under
+  # the top level key 'Diagnostics' in the output yaml files
+  mergekey = "Diagnostics"
+  merged = []
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+content = yaml.safe_load(open(replacefile, 'r'))
+if not content:
+  continue # Skip empty files.
+merged.extend(content.get(mergekey, []))
+
+  if merged:
+# MainSourceFile: The key is required by the definition inside
+# include/clang/Tooling/ReplacementsYaml.h, but the value
+# is actually never used inside clang-apply-replacements,
+# so we set it to '' here.
+output = { 'MainSourceFile': '', mergekey: merged }
+with open(mergefile, 'w') as out:
+  yaml.safe_dump(output, out)
+  else:
+# Empty the file:
+open(mergefile, 'w').close()
 
 
 def main():
@@ -47,7 +127,10 @@ def main():
   r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hpp|m|mm|inc)',
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
-
+  parser.add_argument('-j', type=int, default=1,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +167,7 @@ def main():
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +185,79 @@ def main():
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No r

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: include/clang/AST/Stmt.h:102
+/// This bit is set only for the Stmt's that are the structured-block
+/// of OpeMP [non-standalone] executable directives.
+/// I.e. those returned by OMPExecutableDirective::getStructuredBlock().

gribozavr wrote:
> lebedev.ri wrote:
> > gribozavr wrote:
> > > "non-standalone OpenMP executable directives"
> > English isn't my native language; to me the current variant (attempts to?) 
> > convey
> > that structured block can only ever exist for non-standalone executable 
> > directives.
> > I.e. "structured block of non-standalone executable directives" is a 
> > misnomer.
> > 
> > The variant without `[]` does not convey any of that, i think, it just says
> > that "it's a structured block of non-standalone OpenMP executable 
> > directives",
> > it isn't obvious from that (without knowing OpenMP details) that there 
> > wouldn't be "a structured block of standalone OpenMP executable directive".
> > 
> > Isn't there an important difference there?
> I didn't catch this difference implied by "[]".
> 
> > I.e. "structured block of non-standalone executable directives" is a 
> > misnomer.
> 
> Why is it a misnomer?  It might be overly verbose, but it is correct.
> 
> However, I see that this fact is important to highlight, so I'd suggest to 
> spell it out:
> 
> "This bit is set only for the Stmts that are the structured-block of OpenMP 
> executable directives.  Directives that have a structured block are called 
> "non-standalone" directives."
> 
> Please also fix the typo "OpeMP" => OpenMP.
>> I.e. "structured block of non-standalone executable directives" is a 
>> misnomer.
> Why is it a misnomer? It might be overly verbose, but it is correct.

Blargh.
I meant to write `"structured block of *standalone* executable directives" 
would make no sense`.

> ".."

Yes, that looks cleaner, thank you!



Comment at: unittests/AST/OMPStructuredBlockTest.cpp:64
+++i;
+})";
+  ASSERT_TRUE(

gribozavr wrote:
> lebedev.ri wrote:
> > gribozavr wrote:
> > > lebedev.ri wrote:
> > > > gribozavr wrote:
> > > > > Same comment as in the other patch -- I would prefer that the source 
> > > > > is inlined into the ASSERT_TRUE, with implicit string concatenation 
> > > > > and "\n"s, if raw strings don't work.
> > > > I still don't understand the reasoning here.
> > > > It feels like a change just for the sake of the change.
> > > These variables don't improve readability.  Also, if there are multiple 
> > > such variables in one test, it is easy to mistakenly use an incorrect one 
> > > in the ASSERT_TRUE line.
> > Can you please quote something about this from
> > https://llvm.org/docs/CodingStandards.html
> > https://llvm.org/docs/ProgrammersManual.html
> > etc?
> > If not this is entirely subjective/stylistic,
> > and this was the form that was more readable to me.
> > (easier to write, too, those "\n" will look horrible)
> > Also, what about the cases with two assertions? (not just the new ones)
> > 
> > I really don't see how that would be a good change.
> There are lots of readability points that are not formalized in the coding 
> style, and yet, are followed in the code.
> 
> For cases with two assertions we need to have a variable to avoid code 
> duplication, so it is OK there -- the variable has a purpose.
> 
> If you want to use raw string literals, could I at least ask you to drop the 
> first line onto the same level of indentation as the rest of the lines?
> 
> ```
>   const char *Source =
>   R"(void test(int i) {
> #pragma omp atomic
> ++i;
> })";
> ```
> 
> change to:
> 
> ```
>   const char *Source = R"(
> void test(int i) {
> #pragma omp atomic
> ++i;
> })";
> ```
> 
> If you want to use raw string literals, could I at least ask you to drop the 
> first line onto the same level of indentation as the rest of the lines?

Absolutely.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59599

Files:
  clangd/AST.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1222,6 +1222,22 @@
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: This should be fixed in AST to point at specialization. Exercised
+  // just to make sure we don't crash.
+  Annotations Header(R"(
+  template  struct [[Foo]];
+  struct Bar {
+  friend class Foo;
+  };
+  template <> struct Foo {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range()),
+ ForCodeCompletion(true;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -82,14 +83,19 @@
   if (auto Args = getTemplateSpecializationArgLocs(ND))
 printTemplateArgumentList(OS, *Args, Policy);
   else if (auto *Cls = llvm::dyn_cast(&ND)) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (auto *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) 
{
+llvm::SmallVector ArgLocs;
+ArgLocs.reserve(STL.getNumArgs());
+for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+  ArgLocs.push_back(STL.getArgLoc(I));
+printTemplateArgumentList(OS, ArgLocs, Policy);
+  }
+} else {
+  // FIXME: Cls->getTypeAsWritten might return null in some cases, e.g.
+  // clang sees first sees a friend declaration and then the 
specialization.
+  // In that case fall back to TemplateArguments without Location info,
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);
 }
   }
   OS.flush();


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1222,6 +1222,22 @@
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: This should be fixed in AST to point at specialization. Exercised
+  // just to make sure we don't crash.
+  Annotations Header(R"(
+  template  struct [[Foo]];
+  struct Bar {
+  friend class Foo;
+  };
+  template <> struct Foo {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range()),
+ ForCodeCompletion(true;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -82,14 +83,19 @@
   if (auto Args = getTemplateSpecializationArgLocs(ND))
 printTemplateArgumentList(OS, *Args, Policy);
   else if (auto *Cls = llvm::dyn_cast(&ND)) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (auto *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) {
+llvm::SmallVector ArgLocs;
+ArgLocs.reserve(STL.getNumArgs());
+for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+  ArgLocs.push_back(STL.getArgLoc(I));
+printTemplateArgumentList(OS, ArgLocs, Policy);
+  }
+} else {
+  // FI

[clang-tools-extra] r356567 - gn build: Add build files for some clang-tools-extra

[Nico Weber via cfe-commits]

Author: nico
Date: Wed Mar 20 09:14:16 2019
New Revision: 356567

URL: http://llvm.org/viewvc/llvm-project?rev=356567&view=rev
Log:
gn build: Add build files for some clang-tools-extra

Adds clang-change-namespace, clang-move, clang-query,
clang-reorder-fields.

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

Modified:
clang-tools-extra/trunk/clang-query/tool/CMakeLists.txt
clang-tools-extra/trunk/clang-reorder-fields/tool/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-query/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-query/tool/CMakeLists.txt?rev=356567&r1=356566&r2=356567&view=diff
==
--- clang-tools-extra/trunk/clang-query/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-query/tool/CMakeLists.txt Wed Mar 20 09:14:16 
2019
@@ -1,6 +1,8 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-query ClangQuery.cpp)
+add_clang_executable(clang-query
+  ClangQuery.cpp
+  )
 target_link_libraries(clang-query
   PRIVATE
   clangAST

Modified: clang-tools-extra/trunk/clang-reorder-fields/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-reorder-fields/tool/CMakeLists.txt?rev=356567&r1=356566&r2=356567&view=diff
==
--- clang-tools-extra/trunk/clang-reorder-fields/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-reorder-fields/tool/CMakeLists.txt Wed Mar 20 
09:14:16 2019
@@ -1,4 +1,6 @@
-add_clang_tool(clang-reorder-fields ClangReorderFields.cpp)
+add_clang_tool(clang-reorder-fields
+  ClangReorderFields.cpp
+  )
 
 target_link_libraries(clang-reorder-fields
   PRIVATE


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

[PATCH] D59595: Remove the unused return value in ASTImporter::Imported [NFC]

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59595



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

[PATCH] D59554: gn build: Add build files for some clang-tools-extra

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE356567: gn build: Add build files for some 
clang-tools-extra (authored by nico, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59554?vs=191486&id=191509#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59554

Files:
  clang-query/tool/CMakeLists.txt
  clang-reorder-fields/tool/CMakeLists.txt


Index: clang-reorder-fields/tool/CMakeLists.txt
===
--- clang-reorder-fields/tool/CMakeLists.txt
+++ clang-reorder-fields/tool/CMakeLists.txt
@@ -1,4 +1,6 @@
-add_clang_tool(clang-reorder-fields ClangReorderFields.cpp)
+add_clang_tool(clang-reorder-fields
+  ClangReorderFields.cpp
+  )
 
 target_link_libraries(clang-reorder-fields
   PRIVATE
Index: clang-query/tool/CMakeLists.txt
===
--- clang-query/tool/CMakeLists.txt
+++ clang-query/tool/CMakeLists.txt
@@ -1,6 +1,8 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-query ClangQuery.cpp)
+add_clang_executable(clang-query
+  ClangQuery.cpp
+  )
 target_link_libraries(clang-query
   PRIVATE
   clangAST


Index: clang-reorder-fields/tool/CMakeLists.txt
===
--- clang-reorder-fields/tool/CMakeLists.txt
+++ clang-reorder-fields/tool/CMakeLists.txt
@@ -1,4 +1,6 @@
-add_clang_tool(clang-reorder-fields ClangReorderFields.cpp)
+add_clang_tool(clang-reorder-fields
+  ClangReorderFields.cpp
+  )
 
 target_link_libraries(clang-reorder-fields
   PRIVATE
Index: clang-query/tool/CMakeLists.txt
===
--- clang-query/tool/CMakeLists.txt
+++ clang-query/tool/CMakeLists.txt
@@ -1,6 +1,8 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clang-query ClangQuery.cpp)
+add_clang_executable(clang-query
+  ClangQuery.cpp
+  )
 target_link_libraries(clang-query
   PRIVATE
   clangAST
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59306: [NFC][clang][astdump] Some baseline tests for OpenMP

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356569: [NFC][clang][astdump] Some baseline tests for OpenMP 
(authored by lebedevri, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59306?vs=190436&id=191511#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59306

Files:
  cfe/trunk/test/AST/ast-dump-openmp-atomic.c
  cfe/trunk/test/AST/ast-dump-openmp-barrier.c
  cfe/trunk/test/AST/ast-dump-openmp-cancel.c
  cfe/trunk/test/AST/ast-dump-openmp-cancellation-point.c
  cfe/trunk/test/AST/ast-dump-openmp-critical.c
  cfe/trunk/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-distribute-parallel-for.c
  cfe/trunk/test/AST/ast-dump-openmp-distribute-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-distribute.c
  cfe/trunk/test/AST/ast-dump-openmp-flush.c
  cfe/trunk/test/AST/ast-dump-openmp-for-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-for.c
  cfe/trunk/test/AST/ast-dump-openmp-master.c
  cfe/trunk/test/AST/ast-dump-openmp-ordered.c
  cfe/trunk/test/AST/ast-dump-openmp-parallel-for-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-parallel-for.c
  cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c
  cfe/trunk/test/AST/ast-dump-openmp-parallel-sections.c
  cfe/trunk/test/AST/ast-dump-openmp-parallel.c
  cfe/trunk/test/AST/ast-dump-openmp-section.c
  cfe/trunk/test/AST/ast-dump-openmp-sections.c
  cfe/trunk/test/AST/ast-dump-openmp-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-single.c
  cfe/trunk/test/AST/ast-dump-openmp-target-data.c
  cfe/trunk/test/AST/ast-dump-openmp-target-enter-data.c
  cfe/trunk/test/AST/ast-dump-openmp-target-exit-data.c
  cfe/trunk/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-target-parallel-for.c
  cfe/trunk/test/AST/ast-dump-openmp-target-parallel.c
  cfe/trunk/test/AST/ast-dump-openmp-target-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-target-teams-distribute.c
  cfe/trunk/test/AST/ast-dump-openmp-target-teams.c
  cfe/trunk/test/AST/ast-dump-openmp-target-update.c
  cfe/trunk/test/AST/ast-dump-openmp-target.c
  cfe/trunk/test/AST/ast-dump-openmp-task.c
  cfe/trunk/test/AST/ast-dump-openmp-taskgroup.c
  cfe/trunk/test/AST/ast-dump-openmp-taskloop-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-taskloop.c
  cfe/trunk/test/AST/ast-dump-openmp-taskwait.c
  cfe/trunk/test/AST/ast-dump-openmp-taskyield.c
  cfe/trunk/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  cfe/trunk/test/AST/ast-dump-openmp-teams-distribute-simd.c
  cfe/trunk/test/AST/ast-dump-openmp-teams-distribute.c
  cfe/trunk/test/AST/ast-dump-openmp-teams.c



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

r356571 - [OpenCL] Generate 'unroll.enable' metadata for __attribute__((opencl_unroll_hint))

[Andrew Savonichev via cfe-commits]

Author: asavonic
Date: Wed Mar 20 09:43:07 2019
New Revision: 356571

URL: http://llvm.org/viewvc/llvm-project?rev=356571&view=rev
Log:
[OpenCL] Generate 'unroll.enable' metadata for  
__attribute__((opencl_unroll_hint))

Summary:
[OpenCL] Generate 'unroll.enable' metadata for  
__attribute__((opencl_unroll_hint))

For both !{!"llvm.loop.unroll.enable"} and !{!"llvm.loop.unroll.full"} the 
unroller
will try to fully unroll a loop unless the trip count is not known at compile 
time.
In that case for '.full' metadata no unrolling will be processed, while for 
'.enable'
the loop will be partially unrolled with a heuristically chosen unroll factor.

See: docs/LanguageExtensions.rst

From 
https://www.khronos.org/registry/OpenCL/sdk/2.0/docs/man/xhtml/attributes-loopUnroll.html

__attribute__((opencl_unroll_hint))
for (int i=0; i<2; i++)
{
...
}

In the example above, the compiler will determine how much to unroll the loop.


Before the patch for  __attribute__((opencl_unroll_hint)) was generated metadata
!{!"llvm.loop.unroll.full"}, which limits ability of loop unroller to decide, 
how
much to unroll the loop.

Reviewers: Anastasia, yaxunl

Reviewed By: Anastasia

Subscribers: zzheng, dmgreen, jdoerfert, cfe-commits, asavonic, AlexeySotkin

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl

Modified: cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.cpp?rev=356571&r1=356570&r2=356571&view=diff
==
--- cfe/trunk/lib/CodeGen/CGLoopInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGLoopInfo.cpp Wed Mar 20 09:43:07 2019
@@ -208,13 +208,13 @@ void LoopInfoStack::push(BasicBlock *Hea
 // Translate opencl_unroll_hint attribute argument to
 // equivalent LoopHintAttr enums.
 // OpenCL v2.0 s6.11.5:
-// 0 - full unroll (no argument).
+// 0 - enable unroll (no argument).
 // 1 - disable unroll.
 // other positive integer n - unroll by n.
 if (OpenCLHint) {
   ValueInt = OpenCLHint->getUnrollHint();
   if (ValueInt == 0) {
-State = LoopHintAttr::Full;
+State = LoopHintAttr::Enable;
   } else if (ValueInt != 1) {
 Option = LoopHintAttr::UnrollCount;
 State = LoopHintAttr::Numeric;

Modified: cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl?rev=356571&r1=356570&r2=356571&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/unroll-hint.cl Wed Mar 20 09:43:07 2019
@@ -18,12 +18,12 @@ void for_disable()
 // CHECK: br label %{{.*}}, !llvm.loop ![[FOR_DISABLE:.*]]
 }
 
-void for_full()
+void for_enable()
 {
-// CHECK-LABEL: for_full
+// CHECK-LABEL: for_enable
 __attribute__((opencl_unroll_hint))
 for( int i = 0; i < 1000; ++i);
-// CHECK: br label %{{.*}}, !llvm.loop ![[FOR_FULL:.*]]
+// CHECK: br label %{{.*}}, !llvm.loop ![[FOR_ENABLE:.*]]
 }
 
 /*** while ***/
@@ -45,13 +45,13 @@ void while_disable()
 // CHECK: br label %{{.*}}, !llvm.loop ![[WHILE_DISABLE:.*]]
 }
 
-void while_full()
+void while_enable()
 {
-// CHECK-LABEL: while_full
+// CHECK-LABEL: while_enable
 int i = 1000;
 __attribute__((opencl_unroll_hint))
 while(i-->0);
-// CHECK: br label %{{.*}}, !llvm.loop ![[WHILE_FULL:.*]]
+// CHECK: br label %{{.*}}, !llvm.loop ![[WHILE_ENABLE:.*]]
 }
 
 /*** do ***/
@@ -73,13 +73,13 @@ void do_disable()
 // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop 
![[DO_DISABLE:.*]]
 }
 
-void do_full()
+void do_enable()
 {
-// CHECK-LABEL: do_full
+// CHECK-LABEL: do_enable
 int i = 1000;
 __attribute__((opencl_unroll_hint))
 do {} while(i--> 0);
-// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop 
![[DO_FULL:.*]]
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop 
![[DO_ENABLE:.*]]
 }
 
 
@@ -87,11 +87,11 @@ void do_full()
 // CHECK: ![[COUNT]] =  !{!"llvm.loop.unroll.count", i32 8}
 // CHECK: ![[FOR_DISABLE]]   =  distinct !{![[FOR_DISABLE]],  ![[DISABLE:.*]]}
 // CHECK: ![[DISABLE]]   =  !{!"llvm.loop.unroll.disable"}
-// CHECK: ![[FOR_FULL]]  =  distinct !{![[FOR_FULL]],  ![[FULL:.*]]}
-// CHECK: ![[FULL]]  =  !{!"llvm.loop.unroll.full"}
+// CHECK: ![[FOR_ENABLE]]  =  distinct !{![[FOR_ENABLE]],  ![[ENABLE:.*]]}
+// CHECK: ![[ENABLE]]  =  !{!"llvm.loop.unroll.enable"}
 // CHECK: ![[WHILE_COUNT]]   =  distinct !{![[WHILE_COUNT]],![[COUNT]]}
 // CHECK: ![[WHILE_DISABLE]] =  distinct !{![[WHILE_DISABLE]],  ![[DISABLE]]}
-// CHECK: ![[WHILE_FULL]]=  distinct !{![[WHILE_FULL]], ![[FULL]]}
+// CHECK: ![[WHILE_ENABLE]]=  distinct !{![[

[PATCH] D59493: [OpenCL] Generate 'unroll.enable' metadata for __attribute__((opencl_unroll_hint))

[Andrew Savonichev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356571: [OpenCL] Generate 'unroll.enable' metadata 
for  __attribute__… (authored by asavonic, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D59493

Files:
  lib/CodeGen/CGLoopInfo.cpp
  test/CodeGenOpenCL/unroll-hint.cl


Index: lib/CodeGen/CGLoopInfo.cpp
===
--- lib/CodeGen/CGLoopInfo.cpp
+++ lib/CodeGen/CGLoopInfo.cpp
@@ -208,13 +208,13 @@
 // Translate opencl_unroll_hint attribute argument to
 // equivalent LoopHintAttr enums.
 // OpenCL v2.0 s6.11.5:
-// 0 - full unroll (no argument).
+// 0 - enable unroll (no argument).
 // 1 - disable unroll.
 // other positive integer n - unroll by n.
 if (OpenCLHint) {
   ValueInt = OpenCLHint->getUnrollHint();
   if (ValueInt == 0) {
-State = LoopHintAttr::Full;
+State = LoopHintAttr::Enable;
   } else if (ValueInt != 1) {
 Option = LoopHintAttr::UnrollCount;
 State = LoopHintAttr::Numeric;
Index: test/CodeGenOpenCL/unroll-hint.cl
===
--- test/CodeGenOpenCL/unroll-hint.cl
+++ test/CodeGenOpenCL/unroll-hint.cl
@@ -18,12 +18,12 @@
 // CHECK: br label %{{.*}}, !llvm.loop ![[FOR_DISABLE:.*]]
 }
 
-void for_full()
+void for_enable()
 {
-// CHECK-LABEL: for_full
+// CHECK-LABEL: for_enable
 __attribute__((opencl_unroll_hint))
 for( int i = 0; i < 1000; ++i);
-// CHECK: br label %{{.*}}, !llvm.loop ![[FOR_FULL:.*]]
+// CHECK: br label %{{.*}}, !llvm.loop ![[FOR_ENABLE:.*]]
 }
 
 /*** while ***/
@@ -45,13 +45,13 @@
 // CHECK: br label %{{.*}}, !llvm.loop ![[WHILE_DISABLE:.*]]
 }
 
-void while_full()
+void while_enable()
 {
-// CHECK-LABEL: while_full
+// CHECK-LABEL: while_enable
 int i = 1000;
 __attribute__((opencl_unroll_hint))
 while(i-->0);
-// CHECK: br label %{{.*}}, !llvm.loop ![[WHILE_FULL:.*]]
+// CHECK: br label %{{.*}}, !llvm.loop ![[WHILE_ENABLE:.*]]
 }
 
 /*** do ***/
@@ -73,13 +73,13 @@
 // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop 
![[DO_DISABLE:.*]]
 }
 
-void do_full()
+void do_enable()
 {
-// CHECK-LABEL: do_full
+// CHECK-LABEL: do_enable
 int i = 1000;
 __attribute__((opencl_unroll_hint))
 do {} while(i--> 0);
-// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop 
![[DO_FULL:.*]]
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !llvm.loop 
![[DO_ENABLE:.*]]
 }
 
 
@@ -87,11 +87,11 @@
 // CHECK: ![[COUNT]] =  !{!"llvm.loop.unroll.count", i32 8}
 // CHECK: ![[FOR_DISABLE]]   =  distinct !{![[FOR_DISABLE]],  ![[DISABLE:.*]]}
 // CHECK: ![[DISABLE]]   =  !{!"llvm.loop.unroll.disable"}
-// CHECK: ![[FOR_FULL]]  =  distinct !{![[FOR_FULL]],  ![[FULL:.*]]}
-// CHECK: ![[FULL]]  =  !{!"llvm.loop.unroll.full"}
+// CHECK: ![[FOR_ENABLE]]  =  distinct !{![[FOR_ENABLE]],  ![[ENABLE:.*]]}
+// CHECK: ![[ENABLE]]  =  !{!"llvm.loop.unroll.enable"}
 // CHECK: ![[WHILE_COUNT]]   =  distinct !{![[WHILE_COUNT]],![[COUNT]]}
 // CHECK: ![[WHILE_DISABLE]] =  distinct !{![[WHILE_DISABLE]],  ![[DISABLE]]}
-// CHECK: ![[WHILE_FULL]]=  distinct !{![[WHILE_FULL]], ![[FULL]]}
+// CHECK: ![[WHILE_ENABLE]]=  distinct !{![[WHILE_ENABLE]], 
![[ENABLE]]}
 // CHECK: ![[DO_COUNT]]  =  distinct !{![[DO_COUNT]],   ![[COUNT]]}
 // CHECK: ![[DO_DISABLE]]=  distinct !{![[DO_DISABLE]], ![[DISABLE]]}
-// CHECK: ![[DO_FULL]]   =  distinct !{![[DO_FULL]],![[FULL]]}
+// CHECK: ![[DO_ENABLE]]   =  distinct !{![[DO_ENABLE]],
![[ENABLE]]}


Index: lib/CodeGen/CGLoopInfo.cpp
===
--- lib/CodeGen/CGLoopInfo.cpp
+++ lib/CodeGen/CGLoopInfo.cpp
@@ -208,13 +208,13 @@
 // Translate opencl_unroll_hint attribute argument to
 // equivalent LoopHintAttr enums.
 // OpenCL v2.0 s6.11.5:
-// 0 - full unroll (no argument).
+// 0 - enable unroll (no argument).
 // 1 - disable unroll.
 // other positive integer n - unroll by n.
 if (OpenCLHint) {
   ValueInt = OpenCLHint->getUnrollHint();
   if (ValueInt == 0) {
-State = LoopHintAttr::Full;
+State = LoopHintAttr::Enable;
   } else if (ValueInt != 1) {
 Option = LoopHintAttr::UnrollCount;
 State = LoopHintAttr::Numeric;
Index: test/CodeGenOpenCL/unroll-hint.cl
===
--- test/CodeGenOpenCL/unroll-hint.cl
+++ test/CodeGenOpenCL/unroll-hint.cl
@@ -18,12 +18,12 @@
 // CHECK: br label %{{.*}}, !llvm.loop ![[FOR_DISABLE:.*]]
 }
 
-void for_full()
+void for_enable()
 {
-// CHECK-LABEL: for_full
+// CHECK-LABEL: for_enable
 __attribute__((opencl_unroll_hint))
 for( int i = 0; i < 1000; ++i);
-// CHECK: br label %{{.*}}, !llvm.l

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:401
+  * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``)
+Allow short if/else if statements even if the else is a compound statement.
+

klimek wrote:
> I'd try to make this either unconditionally what we do, or decide against 
> doing it.
see note before, this is really about maintaining compatibility, I don't want 
to make the assumption that everyone likes


```
if (x) return 1;
else if (x) return 2;
else return 3;

```

There could easily be users who want this.

```
if (x) return 1;
else if (x) return 2;
else 
   return 3;
```

I don't think it over complicates it, but it keeps the flexibility. The name of 
the option may not be the best 'suggestions welcome'



Comment at: clang/include/clang/Format/Format.h:263-264
 SIS_WithoutElse,
-/// Always put short ifs on the same line if
-/// the else is not a compound statement or not.
+/// If Else statements have no braces don't put them
+/// on the same line.
+/// \code

klimek wrote:
> This seems weird - why would we want this?
This is a compatiblity case for the previous "true" as it was before

Before 


```
if (x) 
   return 1;
else
   return 2;
```

would be transformed to


```
if (x) return 1;
else
   return 2;
```

but


```
if (x) 
   return 1;
else {
   return 2;
}
```

would not be transformed at all



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

https://reviews.llvm.org/D59408



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

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/TokenKinds.h:80
  K == tok::utf8_string_literal || K == tok::utf16_string_literal ||
- K == tok::utf32_string_literal;
 }

klimek wrote:
> This change looks pretty good, minus the changes outside Format/ :( Do you 
> see any chance to fix this by extending the token merging? It's unlikely that 
> the community will be convinced to add some minor parts of C# to clang proper 
> just to support clang-format.
I know I'm a little nervous about having this code out where it might break the 
actual compiler

Before I got stuck with how C# does its verbatim and interpolated strings 
especially when \ is in a string just before the "

let me go back and see if I can't do this with merging the tokens now I know a 
little more


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

https://reviews.llvm.org/D58404



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

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

[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




Comment at: clangd/AST.cpp:95
+} else {
+  // FIXME: Cls->getTypeAsWritten might return null in some cases, e.g.
+  // clang sees first sees a friend declaration and then the 
specialization.

NIT: maybe shorten the comment?
Something like `// FIXME: Fix cases when getTypeAsWritten returns null, e.g. 
friend decls.`



Comment at: unittests/clangd/SymbolCollectorTests.cpp:1226
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: This should be fixed in AST to point at specialization. Exercised
+  // just to make sure we don't crash.

NIT: or something like `getTypeAsWritten` is missing for friend declaration, 
this should be fixed in the AST>


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59599



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

r356575 - [clang-format] structured binding in range for detected as Objective C

[Paul Hoad via cfe-commits]

Author: paulhoad
Date: Wed Mar 20 10:10:23 2019
New Revision: 356575

URL: http://llvm.org/viewvc/llvm-project?rev=356575&view=rev
Log:
[clang-format] structured binding in range for detected as Objective C

Summary:
Sometime after 6.0.0 and the current trunk 9.0.0 the following code would be 
considered as objective C and not C++

Reported by: https://twitter.com/mattgodbolt/status/1096188576503644160

$ clang-format.exe test.h
Configuration file(s) do(es) not support Objective-C: 
C:\clang\build\.clang-format

--- test.h --
```

std::vector> C;

void foo()
{
   for (auto && [A,B] : C)
   {
   std::string D = A + B;
   }
}
```
The following code fixes this issue of incorrect detection

Reviewers: djasper, klimek, JonasToth, reuk

Reviewed By: klimek

Subscribers: cfe-commits

Tags: #clang-tools-extra

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=356575&r1=356574&r2=356575&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Mar 20 10:10:23 2019
@@ -410,7 +410,10 @@ private:
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > 
prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as 
ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=356575&r1=356574&r2=356575&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Mar 20 10:10:23 2019
@@ -12926,6 +12926,9 @@ TEST_F(FormatTest, GuessLanguageWithCpp1
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
 "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));


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

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.



> This is, or is very similar to, the problem that the host/device overloading 
> addresses in CUDA.

IIRC the difference was that OpenMP didn't have explicit notion of host/device 
functions which made it hard to apply host/device overloading in practice.

> It is also the problem, or very similar to the problem, that the new OpenMP 5 
> `declare variant` directive is intended to address. Johannes and I discussed 
> this earlier today, and I suggest that we:

Interesting. `declare variant ` sounds (according to openmp-TR7 doc) like a 
`__device__` on steroids. That may indeed make things work. Actually, I would 
like __device__ eventually work like `device variant`, so we can have multiple 
device overloads specialized for particular GPU architecture without relying on 
preprocessor's `__CUDA_ARCH__`.

> 
> 
> 1. Add a math.h wrapper to clang/lib/Headers, which generally just does an 
> include_next of math.h, but provides us with the ability to customize this 
> behavior. Writing a header for OpenMP on NVIDIA GPUs which is essentially 
> identical to the math.h functions in __clang_cuda_device_functions.h would be 
> unfortunate, and as CUDA does provide the underlying execution environment 
> for OpenMP target offload on NVIDIA GPUs, duplicative even in principle. We 
> don't need to alter the default global namespace, however, but can include 
> this file from the wrapper math.h.

Using `__clang_cuda_device_functions.h` in addition to `math.h` wrapper should 
be fine. It gives us a path to provide device-side standard math library 
implementation and math.h wrapper provides convenient point to hook in the 
implementation for platforms other than CUDA.

> 2. We should allow host/device overloading in OpenMP mode. As an extension, 
> we could directly reuse the CUDA host/device overloading capability - this 
> also has the advantage of allowing us to directly reuse 
> __clang_cuda_device_functions.h (and perhaps do a similar thing to pick up 
> the device-side printf, etc. from __clang_cuda_runtime_wrapper.h). In the 
> future, we can extend these to provide overloading using OpenMP declare 
> variant, if desired, when in OpenMP mode.

Is OpenMP is still essentially C-based? host/device overloading relies on C++ 
machinery. I think it should work with `__attribute__((overloadable))` but it's 
not been tested.

We may need to restructure bits and pieces of CUDA-related headers to make them 
reusable by OpenMP.  I guess that with `declare variant` we may be able to 
reuse most of the headers as is by treating `__device__` as if the function was 
a variant for NVPTX back-end.

> Thoughts?

SGTM. Let me know if something in the CUDA-related headers gets in the way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D47849



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

[PATCH] D59546: [clang-format] structured binding in range for detected as Objective C

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356575: [clang-format] structured binding in range for 
detected as Objective C (authored by paulhoad, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D59546?vs=191288&id=191524#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59546

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


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -410,7 +410,10 @@
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > 
prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as 
ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12926,6 +12926,9 @@
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
 "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -410,7 +410,10 @@
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12926,6 +12926,9 @@
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
 "[[clang::callable_when(\"unconsumed\", \"unknown\")]]"));
   EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h", "[[foo::bar, ...]]"));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: svenvh, rjmccall.
Herald added subscribers: jdoerfert, ebevhan, yaxunl.

For backwards compatibility we should allow alternative spelling of address 
spaces - `private`, `local`, `global`, `constant`, `generic`.

Note that in order to accept `private` correctly parsing has been changed to 
understand two different use cases - access specifier vs address space.

This fixes the issues reported in the bugs.

The header from libclc is compiled successfully.


https://reviews.llvm.org/D59603

Files:
  include/clang/Basic/TokenKinds.def
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseTentative.cpp
  test/Parser/opencl-cxx-keywords.cl

Index: test/Parser/opencl-cxx-keywords.cl
===
--- test/Parser/opencl-cxx-keywords.cl
+++ test/Parser/opencl-cxx-keywords.cl
@@ -19,32 +19,34 @@
 // Test that only __-prefixed address space qualifiers are accepted.
 struct test_address_space_qualifiers {
   global int *g;
-  // expected-error@-1 {{unknown type name 'global'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __global int *uug;
-  int global; // should be fine in OpenCL C++
+  int global; // expected-warning{{declaration does not declare anything}}
 
   local int *l;
-  // expected-error@-1 {{unknown type name 'local'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __local int *uul;
-  int local; // should be fine in OpenCL C++
+  int local; // expected-warning{{declaration does not declare anything}}
 
   private int *p;
-  // expected-error@-1 {{expected ':'}}
   __private int *uup;
-  int private; // 'private' is a keyword in C++14 and thus in OpenCL C++
-  // expected-error@-1 {{expected member name or ';' after declaration specifiers}}
+  int private; // expected-warning{{declaration does not declare anything}}
 
   constant int *c;
-  // expected-error@-1 {{unknown type name 'constant'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __constant int *uuc;
-  int constant; // should be fine in OpenCL C++
+  int constant; // expected-warning{{declaration does not declare anything}}
 
   generic int *ge;
-  // expected-error@-1 {{unknown type name 'generic'}}
-  // expected-error@-2 {{expected member name or ';' after declaration specifiers}}
   __generic int *uuge;
-  int generic; // should be fine in OpenCL C++
+  int generic; // expected-warning{{declaration does not declare anything}}
 };
+
+// Test that 'private' can be parsed as an access qualifier and an address space too.
+class A{
+  private:
+  private int i; //expected-error{{field may not be qualified with an address space}}
+};
+
+private int i; //expected-error{{program scope variable must reside in global or constant address space}}
+
+void foo(private int i);
+
+private int bar(); //expected-error{{return value cannot be qualified with address space}}
Index: lib/Parse/ParseTentative.cpp
===
--- lib/Parse/ParseTentative.cpp
+++ lib/Parse/ParseTentative.cpp
@@ -1411,6 +1411,7 @@
   case tok::kw_const:
   case tok::kw_volatile:
 // OpenCL address space qualifiers
+  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3047,9 +3047,14 @@
 DiagnoseUnexpectedNamespace(cast(TagDecl));
 return nullptr;
 
+  case tok::kw_private:
+// FIXME: We don't accept GNU attributes on access specifiers in OpenCL mode
+// yet.
+if (getLangOpts().OpenCL && !NextToken().is(tok::colon))
+  return ParseCXXClassMemberDeclaration(AS, AccessAttrs);
+LLVM_FALLTHROUGH;
   case tok::kw_public:
-  case tok::kw_protected:
-  case tok::kw_private: {
+  case tok::kw_protected: {
 AccessSpecifier NewAS = getAccessSpecifierIfPresent();
 assert(NewAS != AS_none);
 // Current token is a C++ access specifier.
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3824,6 +3824,7 @@
 break;
   };
   LLVM_FALLTHROUGH;
+case tok::kw_private:
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:
@@ -4790,6 +4791,7 @@
 
   case tok::kw___kindof:
 
+  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -4980,6 +4982,7 @@
 
   case tok::kw___kindof:
 
+  case tok::kw_private:
   case tok::kw___private:
   case tok::kw___local:
   case tok::kw___global:
@@ -5192,6 +5195,7 @@
   break;
 
 // OpenCL qualifiers:
+case tok::kw_private:
 case tok::kw___private:
 case tok::kw___global:
 case tok::k

r356577 - [AST] Disable ast-dump-openmp-parallel-master-XFAIL.c test

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Wed Mar 20 10:14:49 2019
New Revision: 356577

URL: http://llvm.org/viewvc/llvm-project?rev=356577&view=rev
Log:
[AST] Disable ast-dump-openmp-parallel-master-XFAIL.c test

Fails on MSVC buildbot (but not locally).
Not important as it is 'testing' something that isn't supported yet anyway:
https://bugs.llvm.org/show_bug.cgi?id=41022

Modified:
cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c

Modified: cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c?rev=356577&r1=356576&r2=356577&view=diff
==
--- cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c (original)
+++ cfe/trunk/test/AST/ast-dump-openmp-parallel-master-XFAIL.c Wed Mar 20 
10:14:49 2019
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -fopenmp-version=50 
-ast-dump %s 2>&1 | FileCheck --match-full-lines 
-implicit-check-not=openmp_structured_block %s
 
+// REQUIRES: broken-PR41022
+// https://bugs.llvm.org/show_bug.cgi?id=41022
+
 void test_zero() {
 #pragma omp parallel master
   ;


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

r356578 - [NFC][ASTMatchers] Alphabetically sort REGISTER_MATCHER() macros in RegistryMaps::RegistryMaps()

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Wed Mar 20 10:15:47 2019
New Revision: 356578

URL: http://llvm.org/viewvc/llvm-project?rev=356578&view=rev
Log:
[NFC][ASTMatchers] Alphabetically sort REGISTER_MATCHER() macros in 
RegistryMaps::RegistryMaps()

As noted in https://reviews.llvm.org/D59453#inline-526253

Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=356578&r1=356577&r2=356578&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Wed Mar 20 10:15:47 2019
@@ -90,6 +90,7 @@ void RegistryMaps::registerMatcher(
   } while (false)
 
 /// Generate a registry map with all the known matchers.
+/// Please keep sorted alphabetically!
 RegistryMaps::RegistryMaps() {
   // TODO: Here is the list of the missing matchers, grouped by reason.
   //
@@ -129,12 +130,12 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(argumentCountIs);
   REGISTER_MATCHER(arraySubscriptExpr);
   REGISTER_MATCHER(arrayType);
-  REGISTER_MATCHER(asmStmt);
   REGISTER_MATCHER(asString);
+  REGISTER_MATCHER(asmStmt);
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
-  REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(autoType);
+  REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(blockDecl);
@@ -143,6 +144,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(booleanType);
   REGISTER_MATCHER(breakStmt);
   REGISTER_MATCHER(builtinType);
+  REGISTER_MATCHER(cStyleCastExpr);
   REGISTER_MATCHER(callExpr);
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
@@ -158,7 +160,6 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(constantExpr);
   REGISTER_MATCHER(containsDeclaration);
   REGISTER_MATCHER(continueStmt);
-  REGISTER_MATCHER(cStyleCastExpr);
   REGISTER_MATCHER(cudaKernelCallExpr);
   REGISTER_MATCHER(cxxBindTemporaryExpr);
   REGISTER_MATCHER(cxxBoolLiteral);
@@ -191,10 +192,10 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(cxxUnresolvedConstructExpr);
   REGISTER_MATCHER(decayedType);
   REGISTER_MATCHER(decl);
-  REGISTER_MATCHER(declaratorDecl);
   REGISTER_MATCHER(declCountIs);
   REGISTER_MATCHER(declRefExpr);
   REGISTER_MATCHER(declStmt);
+  REGISTER_MATCHER(declaratorDecl);
   REGISTER_MATCHER(decltypeType);
   REGISTER_MATCHER(defaultStmt);
   REGISTER_MATCHER(dependentSizedArrayType);
@@ -212,7 +213,6 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(expr);
   REGISTER_MATCHER(exprWithCleanups);
   REGISTER_MATCHER(fieldDecl);
-  REGISTER_MATCHER(indirectFieldDecl);
   REGISTER_MATCHER(floatLiteral);
   REGISTER_MATCHER(forEach);
   REGISTER_MATCHER(forEachArgumentWithParam);
@@ -255,8 +255,8 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasCondition);
   REGISTER_MATCHER(hasConditionVariableStatement);
   REGISTER_MATCHER(hasDecayedType);
-  REGISTER_MATCHER(hasDeclaration);
   REGISTER_MATCHER(hasDeclContext);
+  REGISTER_MATCHER(hasDeclaration);
   REGISTER_MATCHER(hasDeducedType);
   REGISTER_MATCHER(hasDefaultArgument);
   REGISTER_MATCHER(hasDefinition);
@@ -290,12 +290,12 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasParameter);
   REGISTER_MATCHER(hasParent);
   REGISTER_MATCHER(hasQualifier);
+  REGISTER_MATCHER(hasRHS);
   REGISTER_MATCHER(hasRangeInit);
   REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
-  REGISTER_MATCHER(hasRHS);
   REGISTER_MATCHER(hasSelector);
   REGISTER_MATCHER(hasSingleDecl);
   REGISTER_MATCHER(hasSize);
@@ -326,6 +326,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
   REGISTER_MATCHER(incompleteArrayType);
+  REGISTER_MATCHER(indirectFieldDecl);
   REGISTER_MATCHER(initListExpr);
   REGISTER_MATCHER(injectedClassNameType);
   REGISTER_MATCHER(innerType);
@@ -341,15 +342,15 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isCatchAll);
   REGISTER_MATCHER(isClass);
   REGISTER_MATCHER(isConst);
-  REGISTER_MATCHER(isConstexpr);
   REGISTER_MATCHER(isConstQualified);
+  REGISTER_MATCHER(isConstexpr);
   REGISTER_MATCHER(isCopyAssignmentOperator);
   REGISTER_MATCHER(isCopyConstructor);
   REGISTER_MATCHER(isDefaultConstructor);
   REGISTER_MATCHER(isDefaulted);
   REGISTER_MATCHER(isDefinition);
-  REGISTER_MATCHER(isDeleted);
   REGISTER_MATCHER(isDelegatingConstructor);
+  REGISTER_MATCHER(isDeleted);
   REGISTER_MATCHER(isExceptionVariable);
   REGISTER_MATCHER(isExpansionInFileMatching);
   REGISTER_MATCHER(isExpansionInMainFile);
@@ -360,13 +361,13 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isExternC);
   REGISTER_MATCHER(isFinal);
   REGISTER_M

[PATCH] D53705: [OpenCL] Postpone PSV address space diagnostic

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.
Herald added a subscriber: ebevhan.
Herald added a project: clang.

Btw, I have created a patch that allows non-underscore prefixed spelling for 
address spaces in C++ for OpenCL:
https://reviews.llvm.org/D59603

I assume the error becomes less critical however it remains. I think it still 
makes sense to fix it considering that it isn't very costly?


Repository:
  rC Clang

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

https://reviews.llvm.org/D53705



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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:

ymandel wrote:
> ilya-biryukov wrote:
> > Maybe consider separating the fluent API to build the rewrite rule from the 
> > rewrite rule itself?
> > 
> > Not opposed to having the fluent builder API, this does look nice and seems 
> > to nicely align with the matcher APIs.
> > However, it is somewhat hard to figure out what can `RewriteRule` do 
> > **after** it was built when looking at the code right now.
> > ```
> > class RewriteRule {
> > public:
> >   RewriteRule(DynTypedMatcher, TextGenerator Replacement, TextGenerator 
> > Explanation);
> > 
> >   DynTypedMatcher matcher() const;
> >   Expected replacement() const;
> >   Expected explanation() const;
> > };
> > 
> > struct RewriteRuleBuilder { // Having a better name than 'Builder' would be 
> > nice.
> >   RewriteRule finish() &&; // produce the final RewriteRule.
> > 
> >   template 
> >   RewriteRuleBuilder &change(const TypedNodeId &Target,
> >   NodePart Part = NodePart::Node) &;
> >   RewriteRuleBuilder &replaceWith(TextGenerator Replacement) &;
> >   RewriteRuleBuilder &because(TextGenerator Explanation) &;
> > private:
> >   RewriteRule RuleInProgress;
> > };
> > RewriteRuleBuilder makeRewriteRule();
> > ```
> I see your point, but do you think it might be enough to improve the comments 
> on the class? My concern with a builder is the mental burden on the user of 
> another concept (the builder) and the need for an extra `.build()` 
> everywhere. To a lesser extent, I also don't love the cost of an extra copy, 
> although I doubt it matters and I suppose we could support moves in the build 
> method.
The issues with the builder interface is that it requires lots of boilerplate, 
which is hard to throw away when reading the code of the class. I agree that 
having a separate builder class is also annoying (more concepts, etc).

Keeping them separate would be my personal preference, but if you'd prefer to 
keep it in the same class than maybe move the non-builder pieces to the top of 
the class and separate the builder methods with a comment. 
WDYT? 

> To a lesser extent, I also don't love the cost of an extra copy, although I 
> doubt it matters and I suppose we could support moves in the build method.
I believe we can be as efficient (up to an extra move) with builders as without 
them. If most usages are of the form `RewriteRule R = 
rewriteRule(...).change(...).replaceWith(...).because(...);`
Then we could make all builder functions return r-value reference to a builder 
and have an implicit conversion operator that would consume the builder, 
producing the final `RewriteRule`:
```
class RewriteRuleBuilder {
  operator RewriteRule () &&;
  /// ...
};
RewriteRuleBuilder rewriteRule();

void addRule(RewriteRule R);
void clientCode() {
  addRule(rewriteRule().change(Matcher).replaceWith("foo").because("bar"));
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

r356580 - [X86] Remove getCPUKindCanonicalName which is unused.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed Mar 20 10:26:51 2019
New Revision: 356580

URL: http://llvm.org/viewvc/llvm-project?rev=356580&view=rev
Log:
[X86] Remove getCPUKindCanonicalName which is unused.

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

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=356580&r1=356579&r2=356580&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Mar 20 10:26:51 2019
@@ -1540,18 +1540,6 @@ void X86TargetInfo::getCPUSpecificCPUDis
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)
\
-  case CK_##ENUM:  
\
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=356580&r1=356579&r2=356580&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Wed Mar 20 10:26:51 2019
@@ -121,8 +121,6 @@ protected:
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:


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

[PATCH] D59578: [X86] Remove getCPUKindCanonicalName which seems to be unused.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356580: [X86] Remove getCPUKindCanonicalName which is 
unused. (authored by ctopper, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59578

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/lib/Basic/Targets/X86.h


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1540,18 +1540,6 @@
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)
\
-  case CK_##ENUM:  
\
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently
Index: cfe/trunk/lib/Basic/Targets/X86.h
===
--- cfe/trunk/lib/Basic/Targets/X86.h
+++ cfe/trunk/lib/Basic/Targets/X86.h
@@ -121,8 +121,6 @@
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -1540,18 +1540,6 @@
   WholeList.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 }
 
-std::string X86TargetInfo::getCPUKindCanonicalName(CPUKind Kind) const {
-  switch (Kind) {
-  case CK_Generic:
-return "";
-#define PROC(ENUM, STRING, IS64BIT)\
-  case CK_##ENUM:  \
-return STRING;
-#include "clang/Basic/X86Target.def"
-  }
-  llvm_unreachable("Invalid CPUKind");
-}
-
 // We can't use a generic validation scheme for the cpus accepted here
 // versus subtarget cpus accepted in the target attribute because the
 // variables intitialized by the runtime only support the below currently
Index: cfe/trunk/lib/Basic/Targets/X86.h
===
--- cfe/trunk/lib/Basic/Targets/X86.h
+++ cfe/trunk/lib/Basic/Targets/X86.h
@@ -121,8 +121,6 @@
 
   CPUKind getCPUKind(StringRef CPU) const;
 
-  std::string getCPUKindCanonicalName(CPUKind Kind) const;
-
   enum FPMathKind { FP_Default, FP_SSE, FP_387 } FPMath = FP_Default;
 
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D52150#1436423 , @to-miz wrote:

> I checked out the git repository and applied the patch to head and reran all 
> tests.
>  The diff produced by git is basically the same, should I upload the new diff?


I think you might need a new diff, I tried to merge this and it rejected some 
of the patch


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

https://reviews.llvm.org/D52150



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

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 191530.
MyDeveloperDay added a comment.

Addressing code review comment

  @klimek I think is what you meant, a much smaller and neater version of what 
I had before.


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

https://reviews.llvm.org/D59309

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
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T &c);\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2049,6 +2049,10 @@
   Tok = Tok->MatchingParen;
   continue;
 }
+if (Tok->is(TT_TemplateOpener) && Tok->MatchingParen) {
+  Tok = Tok->MatchingParen;
+  continue;
+}
 if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||
 Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis))
   return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T &c);\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2049,6 +2049,10 @@
   Tok = Tok->MatchingParen;
   continue;
 }
+if (Tok->is(TT_TemplateOpener) && Tok->MatchingParen) {
+  Tok = Tok->MatchingParen;
+  continue;
+}
 if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||
 Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59605: [clangd] Introduce background-indexer

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jdoerfert, erik.pilkington, arphaman, 
jkorous, MaskRay, ioeric, mgorny.
Herald added a project: clang.

Sample output:
I[18:37:50.083] BackgroundIndex: build symbol index periodically every 8640 
ms.
I[18:37:50.115] Enqueueing 3742 commands for indexing
I[18:37:50.280] [0/3742] Loaded shards from storage
I[18:37:51.026] [1/3742] Indexed 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
 (6422 symbols, 18236 refs, 161 files)
I[18:37:51.581] [2/3742] Indexed 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/MD5.cpp (10204 
symbols, 28897 refs, 232 files)
I[18:37:51.607] [3/3742] Indexed 
/usr/local/google/home/kadircet/repos/llvm/llvm/lib/MC/MCSchedule.cpp (11461 
symbols, 32420 refs, 247 files)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59605

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp
  clang-tools-extra/clangd/background-indexer/CMakeLists.txt
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h

Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -144,6 +144,11 @@
   std::deque> Queue;
   std::vector ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
+
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;
+  std::mutex TUCounterMutex;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -29,6 +29,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -215,6 +216,10 @@
 // We're doing this asynchronously, because we'll read shards here too.
 log("Enqueueing {0} commands for indexing", ChangedFiles.size());
 SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
+{
+  std::lock_guard Lock(TUCounterMutex);
+  EnqueuedTUs += ChangedFiles.size();
+}
 
 auto NeedsReIndexing = loadShards(std::move(ChangedFiles));
 // Run indexing for files that need to be updated.
@@ -447,9 +452,13 @@
   assert(Index.Symbols && Index.Refs && Index.Sources &&
  "Symbols, Refs and Sources must be set.");
 
-  log("Indexed {0} ({1} symbols, {2} refs, {3} files)",
-  Inputs.CompileCommand.Filename, Index.Symbols->size(),
-  Index.Refs->numRefs(), Index.Sources->size());
+  {
+std::lock_guard Lock(TUCounterMutex);
+++IndexedTUs;
+log("[{0}/{1}] Indexed {2} ({3} symbols, {4} refs, {5} files)", IndexedTUs,
+EnqueuedTUs, Inputs.CompileCommand.Filename, Index.Symbols->size(),
+Index.Refs->numRefs(), Index.Sources->size());
+  }
   SPAN_ATTACH(Tracer, "symbols", int(Index.Symbols->size()));
   SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs()));
   SPAN_ATTACH(Tracer, "sources", int(Index.Sources->size()));
@@ -581,20 +590,23 @@
   // Keeps track of the loaded shards to make sure we don't perform redundant
   // disk IO. Keys are absolute paths.
   llvm::StringSet<> LoadedShards;
+  size_t UpToDateTUs = 0;
   for (const auto &File : ChangedFiles) {
 ProjectInfo PI;
 auto Cmd = CDB.getCompileCommand(File, &PI);
 if (!Cmd)
   continue;
 BackgroundIndexStorage *IndexStorage = IndexStorageFactory(PI.SourceRoot);
+++UpToDateTUs;
 auto Dependencies = loadShard(*Cmd, IndexStorage, LoadedShards);
 for (const auto &Dependency : Dependencies) {
   if (!Dependency.NeedsReIndexing || FilesToIndex.count(Dependency.Path))
 continue;
+  --UpToDateTUs;
   // FIXME: Currently, we simply schedule indexing on a TU whenever any of
   // its dependencies needs re-indexing. We might do it smarter by figuring
   // out a minimal set of TUs that will cover all the stale dependencies.
-  vlog("Enqueueing TU {0} because its dependency {1} needs re-indexing.",
+  dlog("Enqueueing TU {0} because its dependency {1} needs re-indexing.",
Cmd->Filename, Dependency.Path);
   NeedsReIndexing.push_back({std::move(*Cmd), IndexStorage});
   // Mark all of this TU's dependencies as to-be-indexed so that we won't
@@ -604,7 +616,11 @@
   break;
 }
   }
-  vlog("Loaded all shards");
+  {
+std::lock_guard Lock(TUCounterMutex);
+IndexedTUs += UpToDateTUs;
+log("[{0}/{1}] Loaded shards from storage", IndexedTUs, EnqueuedTUs);
+  }
   reset(IndexedSymbols.buildIndex(IndexType::Heavy, DuplicateHandling::Merge));
   vlog("BackgroundIndex: built symbol index with estimated memory {0} "
   

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 191536.
MyDeveloperDay added a comment.

Sorry for the spam:

- write this a little better


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

https://reviews.llvm.org/D59309

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
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T &c);\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2045,7 +2045,7 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
-if (Tok->is(tok::l_paren) && Tok->MatchingParen) {
+if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5419,6 +5419,42 @@
"}\n"
"template  T *f(T &c);\n", // No break here.
Style);
+  verifyFormat("int\n"
+   "foo(A a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A<8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, bool> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
+  verifyFormat("int\n"
+   "foo(A, 8> a)\n"
+   "{\n"
+   "  return a;\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2045,7 +2045,7 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
-if (Tok->is(tok::l_paren) && Tok->MatchingParen) {
+if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59605: [clangd] Introduce background-indexer

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:13
+
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Context.h"
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Logger.h"

Please use relative path. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for changes. A few more comments.




Comment at: clangd/ClangdUnit.cpp:259
+// Generates a mapping from start of an include directive to its range.
+std::map
+positionToRangeMap(const std::vector &MainFileIncludes) {

Could we binary-search in a sorted `vector` with a custom comparator 
instead?



Comment at: clangd/ClangdUnit.cpp:411
+MaxDiagsPerIncludeDirective) {
+  ++NumberOfDiagsAtLine[D.IncludeDirective->line];
+  return true;

The function name suggests it does not have side-effects.
Maybe handle the update and query of `NumberOfDiagstAtLine` outside this 
function instead?



Comment at: clangd/Diagnostics.cpp:396
+  for (llvm::StringRef Inc : IncludeStack)
+OS << "In file included from: " << Inc << ":\n";
+}

NIT: could we reuse the function from clang that does the same thing?

The code here is pretty simple, though, so writing it here is fine. Just wanted 
to make sure we considered this option and found it's easier to redo this work 
ourselves.



Comment at: clangd/Diagnostics.cpp:419
-  else
-vlog("Dropped diagnostic outside main file: {0}: {1}", LastDiag->File,
- LastDiag->Message);

We can still drop diagnostics at some point, right?
Could we move the log statement in a different point?




Comment at: clangd/Diagnostics.h:87
+  /// directive, otherwise None.
+  llvm::Optional IncludeDirective = llvm::None;
 };

Could we avoid changing this interface?
We have enough information to fill the corresponding range and message when 
collecting diagnostics inside our implementation of clang's 
`DiagnosticConsumer`, there's really no point in carrying this information in 
this struct.

If that means intermediate structures stored in our consumer, so be it, it's a 
private implementation detail. `Diag` is a public interface, so keeping it 
simpler is valuable.



Comment at: unittests/clangd/DiagnosticsTests.cpp:773
+  UnorderedElementsAre(
+  Diag(Main.range(), "/clangd-test/a.h:2:44: C++ requires a "
+ "type specifier for all declarations")));

NIT: maybe use raw string literals to keep the message a one-liner?



Comment at: unittests/clangd/DiagnosticsTests.cpp:779
 } // namespace clangd
 } // namespace clang

Could we add a test for reaching the limit?
Could we mention that some diagnostics were dropped in that case? Something 
like:
```
In file included from a.h:20:1:
error: unresolved identifier 'foo'.

And 10 more diagnostics...
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clangd/AST.cpp:86
   else if (auto *Cls = llvm::dyn_cast(&ND)) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (auto *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) 
{

Return type is not obvious, so auto should not be used.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59599



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

[PATCH] D59605: [clangd] Introduce background-indexer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:13
+
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Context.h"
+#include 
"/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Logger.h"

Eugene.Zelenko wrote:
> Please use relative path. Same below.
Heh, where do these come from?
Does our include insertion prefer to add global paths in some cases? Dynamic 
index?



Comment at: clang-tools-extra/clangd/index/Background.cpp:622
+IndexedTUs += UpToDateTUs;
+log("[{0}/{1}] Loaded shards from storage", IndexedTUs, EnqueuedTUs);
+  }

Everything else is `vlog` (or even `dlog`), what's the rationale of using `log` 
here?



Comment at: clang-tools-extra/clangd/index/Background.h:149
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;

Maybe use `std::atomic` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:

ilya-biryukov wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > Maybe consider separating the fluent API to build the rewrite rule from 
> > > the rewrite rule itself?
> > > 
> > > Not opposed to having the fluent builder API, this does look nice and 
> > > seems to nicely align with the matcher APIs.
> > > However, it is somewhat hard to figure out what can `RewriteRule` do 
> > > **after** it was built when looking at the code right now.
> > > ```
> > > class RewriteRule {
> > > public:
> > >   RewriteRule(DynTypedMatcher, TextGenerator Replacement, TextGenerator 
> > > Explanation);
> > > 
> > >   DynTypedMatcher matcher() const;
> > >   Expected replacement() const;
> > >   Expected explanation() const;
> > > };
> > > 
> > > struct RewriteRuleBuilder { // Having a better name than 'Builder' would 
> > > be nice.
> > >   RewriteRule finish() &&; // produce the final RewriteRule.
> > > 
> > >   template 
> > >   RewriteRuleBuilder &change(const TypedNodeId &Target,
> > >   NodePart Part = NodePart::Node) &;
> > >   RewriteRuleBuilder &replaceWith(TextGenerator Replacement) &;
> > >   RewriteRuleBuilder &because(TextGenerator Explanation) &;
> > > private:
> > >   RewriteRule RuleInProgress;
> > > };
> > > RewriteRuleBuilder makeRewriteRule();
> > > ```
> > I see your point, but do you think it might be enough to improve the 
> > comments on the class? My concern with a builder is the mental burden on 
> > the user of another concept (the builder) and the need for an extra 
> > `.build()` everywhere. To a lesser extent, I also don't love the cost of an 
> > extra copy, although I doubt it matters and I suppose we could support 
> > moves in the build method.
> The issues with the builder interface is that it requires lots of 
> boilerplate, which is hard to throw away when reading the code of the class. 
> I agree that having a separate builder class is also annoying (more concepts, 
> etc).
> 
> Keeping them separate would be my personal preference, but if you'd prefer to 
> keep it in the same class than maybe move the non-builder pieces to the top 
> of the class and separate the builder methods with a comment. 
> WDYT? 
> 
> > To a lesser extent, I also don't love the cost of an extra copy, although I 
> > doubt it matters and I suppose we could support moves in the build method.
> I believe we can be as efficient (up to an extra move) with builders as 
> without them. If most usages are of the form `RewriteRule R = 
> rewriteRule(...).change(...).replaceWith(...).because(...);`
> Then we could make all builder functions return r-value reference to a 
> builder and have an implicit conversion operator that would consume the 
> builder, producing the final `RewriteRule`:
> ```
> class RewriteRuleBuilder {
>   operator RewriteRule () &&;
>   /// ...
> };
> RewriteRuleBuilder rewriteRule();
> 
> void addRule(RewriteRule R);
> void clientCode() {
>   addRule(rewriteRule().change(Matcher).replaceWith("foo").because("bar"));
> }
> ```
I didn't realize that implicit conversion functions are allowed. With that, I'm 
fine w/ splitting.   Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

[PATCH] D59605: [clangd] Introduce background-indexer

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clangd/index/Background.cpp:29
 #include "llvm/Support/SHA1.h"
 
 #include 

Unnecessary empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping for reviews please.


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

https://reviews.llvm.org/D59279



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

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

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood marked 9 inline comments as done.
LegalizeAdulthood added inline comments.



Comment at: test/clang-tidy/check_clang_tidy.py:112
+process_output = e.output.decode()
+print('%s failed:\n%s' % (' '.join(args), process_output))
+if raise_error:

JonasToth wrote:
> Its better to use `.format()` instead of `%` syntax
That can be done in a later commit; it is not the point of this review.



Comment at: test/clang-tidy/check_clang_tidy.py:141
+   '-check-prefixes=' + ','.join(check_notes_prefixes),
+   '-implicit-check-not={{note|warning|error}}:'])
+  return

serge-sans-paille wrote:
> These three `check_*` function all use the same `'FileCheck', '-']` 
> pattern. Maybe it's worth syndicating that to a 
> try_run_filecheck(input_file0, input_file1, *extra_args)` function.
That can be done in a later commit; it is not the point of this review.



Comment at: test/clang-tidy/check_clang_tidy.py:178
+check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes = \
+get_prefixes(args.check_suffix, input_text)
+

serge-sans-paille wrote:
> This is the verbose call site I was pointing to above.
Addressed by Extract Class instead of Extract Function



Comment at: test/clang-tidy/check_clang_tidy.py:206
 
-  if has_check_fixes:
-try:
-  subprocess.check_output(
-  ['FileCheck', '-input-file=' + temp_file_name, input_file_name,
-   '-check-prefixes=' + ','.join(check_fixes_prefixes),
-   '-strict-whitespace'],
-  stderr=subprocess.STDOUT)
-except subprocess.CalledProcessError as e:
-  print('FileCheck failed:\n' + e.output.decode())
-  raise
-
-  if has_check_messages:
-messages_file = temp_file_name + '.msg'
-write_file(messages_file, clang_tidy_output)
-try:
-  subprocess.check_output(
-  ['FileCheck', '-input-file=' + messages_file, input_file_name,
-   '-check-prefixes=' + ','.join(check_messages_prefixes),
-   '-implicit-check-not={{warning|error}}:'],
-  stderr=subprocess.STDOUT)
-except subprocess.CalledProcessError as e:
-  print('FileCheck failed:\n' + e.output.decode())
-  raise
-
-  if has_check_notes:
-notes_file = temp_file_name + '.notes'
-filtered_output = [line for line in clang_tidy_output.splitlines()
-   if not "note: FIX-IT applied" in line]
-write_file(notes_file, '\n'.join(filtered_output))
-try:
-  subprocess.check_output(
-  ['FileCheck', '-input-file=' + notes_file, input_file_name,
-   '-check-prefixes=' + ','.join(check_notes_prefixes),
-   '-implicit-check-not={{note|warning|error}}:'],
-  stderr=subprocess.STDOUT)
-except subprocess.CalledProcessError as e:
-  print('FileCheck failed:\n' + e.output.decode())
-  raise
+  check_fixes(check_fixes_prefixes, has_check_fixes, input_file_name, 
temp_file_name)
+  check_messages(check_messages_prefixes, has_check_messages, 
clang_tidy_output, input_file_name, temp_file_name)

JonasToth wrote:
> If would prefer keeping the `if check_notes` outside of the function call and 
> remove that one argument. Same for the other `check_...` functions
It's the wrong level of abstraction for the `if` check to be inside `run()`


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

https://reviews.llvm.org/D56343



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

[PATCH] D59605: [clangd] Introduce background-indexer

[Jan Korous via Phabricator via cfe-commits]

jkorous added inline comments.



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:56
+  // non-interactive tools like this one.
+  24 * 60 * 60 * 1000);
+  llvm::SmallString<128> DummyFile(CompileCommandsDir);

Nit: maybe we should give this constant a name? Or maybe create a command line 
option for this?



Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:60
+  llvm::sys::path::remove_dots(DummyFile, true);
+  llvm::sys::path::append(DummyFile, "dummy.cpp");
+  CDB.getCompileCommand(DummyFile);

Maybe we should add a comment about why we do this?



Comment at: clang-tools-extra/clangd/index/Background.cpp:605
 continue;
+  --UpToDateTUs;
   // FIXME: Currently, we simply schedule indexing on a TU whenever any of

It's not obvious to me that we don't underflow here given we are using 
`size_t`. Maybe add an `assert` or some other sanity check?



Comment at: clang-tools-extra/clangd/index/Background.h:149
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;

ilya-biryukov wrote:
> Maybe use `std::atomic` instead?
+1 for atomic


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59605



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

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> The cost is financial, as it's developer time, which costs real money to 
> companies. In the end, to support this, people like myself who are doing this 
> as part of their job spend time that they'd otherwise spend to make other 
> things better that might be more important.

Don't get me wrong I totally appreciate what you do,

But what you mean is it costs your employer.  That I understand, but not all of 
us are doing this on behalf of a company (more specially not yours), so you 
could also say that your employer benefits the other way from those 
contributors who give their time without them having to spend a dime.

I guess my question would be, should the cost to your employer really come into 
the decision about whether something goes in or not? Other than of course we 
are totally grateful to them for giving us so much of your time, but that 
shouldn't have impact on what is worthy to go in should it? or am I wrong?




Comment at: lib/Format/TokenAnnotator.cpp:2546-2560
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
-(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
-  tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
- Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
- (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
-   tok::kw_new, tok::kw_delete) &&
-  (!Left.Previous || Left.Previous->isNot(tok::period) ||
-   (Style.SpaceBeforeParens == FormatStyle::SBPO_Always &&
-(Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
- Left.is(tok::r_paren) ||
- (Left.is(tok::r_square) && Left.MatchingParen &&
-  Left.MatchingParen->is(TT_LambdaLSquare))) &&
-Line.Type != LT_PreprocessorDirective);
+((Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, 
tok::kw_while,
+   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
+   TT_ObjCForIn) ||
+  Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
+  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,

klimek wrote:
> reuk wrote:
> > klimek wrote:
> > > I'm really confused about the changes os parens. It seems like this 
> > > should just change the spaceRequiredBeforeParens(...), but instead seems 
> > > to change parens in a way that completely changes the logic? (or 
> > > alternatively is phabricator showing this wrong?)
> > I think this looks like a bigger change than it really is because I added 
> > an open-parens on line 2548, which then affected the formatting of the 
> > following lines. I also added the `isSimpleTypeSpecifier` check, so that 
> > functional-style casts (`int (foo)`) are given the correct spacing.
> a) why did you add the one in 2548? you didn't change any of the logic, right?
> b) there are 3 extra closing parens in 2560, I see one more new opening in 
> 2556, but that also seems superfluous?
> One question is whether we should pull this apart into bools with names that 
> make sense :)
Isn't this just a classic example of where rewriting this as a series of static 
functions  e.g.

```
static bool someBehavior(Line, Left)
{
 if  (Line.Type == LT_ObjCDecl)
  return true;
 if  (Left.is(tok::semi)
  return true;
..

return false; 
}
```

would be so much easier to understand?


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

https://reviews.llvm.org/D55170



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

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Tolga Mizrak via Phabricator via cfe-commits]

to-miz updated this revision to Diff 191540.
to-miz added a comment.

Made a new diff on the git repository, since the docs now recommend using git.
Rebased and created a new diff with `git show HEAD -U99`.
The patch applies cleanly on windows after `git reset --hard` so I hope there 
shouldn't be any merge conflicts now.


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

https://reviews.llvm.org/D52150

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2993,22 +2993,25 @@
 EXPECT_EQ(Expected, format(ToFormat, Style));
 EXPECT_EQ(Expected, format(Expected, Style));
   }
-  // Test with tabs.
-  Style.UseTab = FormatStyle::UT_Always;
-  Style.IndentWidth = 8;
-  Style.TabWidth = 8;
-  verifyFormat("#ifdef _WIN32\n"
-   "#\tdefine A 0\n"
-   "#\tifdef VAR2\n"
-   "#\t\tdefine B 1\n"
-   "#\t\tinclude \n"
-   "#\t\tdefine MACRO  \\\n"
-   "\t\t\tsome_very_long_func_aa();\n"
-   "#\tendif\n"
-   "#else\n"
-   "#\tdefine A 1\n"
-   "#endif",
-   Style);
+  // Test AfterHash with tabs.
+  {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO  \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+  }
 
   // Regression test: Multiline-macro inside include guards.
   verifyFormat("#ifndef HEADER_H\n"
@@ -3018,6 +3021,102 @@
"  int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+  Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+  // Basic before hash indent tests
+  verifyFormat("#ifdef _WIN32\n"
+   "  #define A 0\n"
+   "  #ifdef VAR2\n"
+   "#define B 1\n"
+   "#include \n"
+   "#define MACRO  \\\n"
+   "  some_very_long_func_aa();\n"
+   "  #endif\n"
+   "#else\n"
+   "  #define A 1\n"
+   "#endif",
+   Style);
+  verifyFormat("#if A\n"
+   "  #define MACRO\\\n"
+   "void a(int x) {\\\n"
+   "  b(); \\\n"
+   "  c(); \\\n"
+   "  d(); \\\n"
+   "  e(); \\\n"
+   "  f(); \\\n"
+   "}\n"
+   "#endif",
+   Style);
+  // Keep comments aligned with indented directives. These
+  // tests cannot use verifyFormat because messUp manipulates leading
+  // whitespace.
+  {
+const char *Expected = "void f() {\n"
+   "// Aligned to preprocessor.\n"
+   "#if 1\n"
+   "  // Aligned to code.\n"
+   "  int a;\n"
+   "  #if 1\n"
+   "// Aligned to preprocessor.\n"
+   "#define A 0\n"
+   "  // Aligned to code.\n"
+   "  int b;\n"
+   "  #endif\n"
+   "#endif\n"
+   "}";
+const char *ToFormat = "void f() {\n"
+   "// Aligned to preprocessor.\n"
+   "#if 1\n"
+   "// Aligned to code.\n"
+   "int a;\n"
+   "#if 1\n"
+   "// Aligned to preprocessor.\n"
+   "#define A 0\n"
+   "// Aligned to code.\n"
+   "int b;\n"
+   "#endif\n"
+   "#endif\n"
+   "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+  }
+  {
+const char *Expected = 

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: include/clang/Basic/DiagnosticLexKinds.td:397
+def ext_pp_opencl_variadic_macros : Extension<
+  "variadic macros not supported in OpenCL">;
 

Maybe rephrase the message now to say it's an extension? The other similar 
warnings seem to mention "GNU extension" or "Microsoft extensions" or  extension.

None of them seem to mention "clang" extensions however.


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

https://reviews.llvm.org/D59492



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

[clang-tools-extra] r356589 - [clang-tidy] Fix redundant check breaking the test on many platforms.

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Wed Mar 20 11:37:04 2019
New Revision: 356589

URL: http://llvm.org/viewvc/llvm-project?rev=356589&view=rev
Log:
[clang-tidy] Fix redundant check breaking the test on many platforms.

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


Modified:
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp?rev=356589&r1=356588&r2=356589&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp Wed Mar 20 
11:37:04 2019
@@ -23,5 +23,4 @@ struct B : public A {
 // CHECK-QUIET-NOT: warning:
 };
 // CHECK-SANITY-NOT: Suppressed
-// CHECK: Suppressed 1 warnings (1 due to line filter).
 // CHECK-QUIET-NOT: Suppressed


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

[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a comment.

I was a little bit worried about

  struct top_level { int i; };
  
  private ::top_level a;

but it should be fine because in that case we have a `tok::coloncolon`


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

https://reviews.llvm.org/D59603



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

[PATCH] D59595: Remove the unused return value in ASTImporter::Imported [NFC]

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356592: Remove the unused return value in 
ASTImporter::Imported [NFC] (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59595?vs=191469&id=191551#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59595

Files:
  cfe/trunk/include/clang/AST/ASTImporter.h
  cfe/trunk/lib/AST/ExternalASTMerger.cpp
  lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
  lldb/trunk/source/Symbol/ClangASTImporter.cpp


Index: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
@@ -262,7 +262,7 @@
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
Index: lldb/trunk/source/Symbol/ClangASTImporter.cpp
===
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp
@@ -937,7 +937,7 @@
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -425,7 +425,7 @@
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);
Index: cfe/trunk/lib/AST/ExternalASTMerger.cpp
===
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp
@@ -110,7 +110,7 @@
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's 
origin
   /// map is kept up to date.  Also set the appropriate flags.
-  Decl *Imported(Decl *From, Decl *To) override {
+  void Imported(Decl *From, Decl *To) override {
 if (auto *ToDC = dyn_cast(To)) {
   const bool LoggingEnabled = Parent.LoggingEnabled();
   if (LoggingEnabled)
@@ -153,7 +153,6 @@
   ToContainer->getPrimaryContext()->setMustBuildLookupTable();
   assert(Parent.CanComplete(ToContainer));
 }
-return To;
   }
   ASTImporter &GetReverse() { return Reverse; }
 };


Index: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
===
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
@@ -262,7 +262,7 @@
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 
Index: lldb/trunk/source/Symbol/ClangASTImporter.cpp
===
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp
@@ -937,7 +937,7 @@
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {
Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -425,7 +425,7 @@
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign 

r356592 - Remove the unused return value in ASTImporter::Imported [NFC]

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Wed Mar 20 12:00:25 2019
New Revision: 356592

URL: http://llvm.org/viewvc/llvm-project?rev=356592&view=rev
Log:
Remove the unused return value in ASTImporter::Imported [NFC]

Summary:
`ASTImporter::Imported` currently returns a Decl, but that return value is not 
used by the ASTImporter (or anywhere else)
nor is it documented.

Reviewers: balazske, martong, a.sidorin, shafik

Reviewed By: balazske, martong

Subscribers: rnkovacs, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ExternalASTMerger.cpp

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=356592&r1=356591&r2=356592&view=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Wed Mar 20 12:00:25 2019
@@ -425,7 +425,7 @@ class TypeSourceInfo;
 
 /// Subclasses can override this function to observe all of the \c From ->
 /// \c To declaration mappings as they are imported.
-virtual Decl *Imported(Decl *From, Decl *To) { return To; }
+virtual void Imported(Decl *From, Decl *To) {}
 
 /// Store and assign the imported declaration to its counterpart.
 Decl *MapImported(Decl *From, Decl *To);

Modified: cfe/trunk/lib/AST/ExternalASTMerger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTMerger.cpp?rev=356592&r1=356591&r2=356592&view=diff
==
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp (original)
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp Wed Mar 20 12:00:25 2019
@@ -110,7 +110,7 @@ public:
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's 
origin
   /// map is kept up to date.  Also set the appropriate flags.
-  Decl *Imported(Decl *From, Decl *To) override {
+  void Imported(Decl *From, Decl *To) override {
 if (auto *ToDC = dyn_cast(To)) {
   const bool LoggingEnabled = Parent.LoggingEnabled();
   if (LoggingEnabled)
@@ -153,7 +153,6 @@ public:
   ToContainer->getPrimaryContext()->setMustBuildLookupTable();
   assert(Parent.CanComplete(ToContainer));
 }
-return To;
   }
   ASTImporter &GetReverse() { return Reverse; }
 };


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

RE: [clang-tools-extra] r356565 - Reland r356547 after fixing the tests for Linux.

[via cfe-commits]

Hi Zinovy,

The test you modified is still failing on the PS4 linux bot after your change:

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/45557

FAIL: Clang Tools :: clang-tidy/clang-tidy-diff.cpp (14410 of 47824)
 TEST 'Clang Tools :: clang-tidy/clang-tidy-diff.cpp' 
FAILED 
Script:
--
: 'RUN: at line 2';   sed 's/placeholder_for_f/f/' 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 > 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
: 'RUN: at line 3';   clang-tidy -checks=-*,modernize-use-override 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
: 'RUN: at line 4';   not diff -U0 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 | /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/../clang-tidy/tool/clang-tidy-diff.py
 -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
: 'RUN: at line 5';   not diff -U0 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 | /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/../clang-tidy/tool/clang-tidy-diff.py
 -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck 
-check-prefix=CHECK-QUIET 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
: 'RUN: at line 6';   mkdir -p 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/compilation-database-test/
: 'RUN: at line 7';   echo '[{"directory": 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output",
 "command": "clang++ -o test.o -std=c++11 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp",
 "file": 
"/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp"}]'
 > 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/compilation-database-test/compile_commands.json
: 'RUN: at line 8';   not diff -U0 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/clang-tidy-diff.cpp.tmp.cpp
 | /usr/bin/python2.7 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/../test/../clang-tidy/tool/clang-tidy-diff.py
 -checks=-*,modernize-use-override -path 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/tools/extra/test/clang-tidy/Output/compilation-database-test
 2>&1 | FileCheck -check-prefix=CHECK 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp
--
Exit Code: 1

Command Output (stderr):
--
2 warnings generated.
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/test/clang-tidy/clang-tidy-diff.cpp:18:11:
 error: CHECK: expected string not found in input
// CHECK: [[@LINE-2]]:8: warning: annotate this
  ^
:1:1: note: scanning from here
Traceback (most recent call last):
^
:1:1: note: with expression "@LINE-2" equal to "16"
Traceback (most recent call last):
^
:2:1

r356598 - Recommit "Support attribute used in member funcs of class templates"

[Rafael Auler via cfe-commits]

Author: rafauler
Date: Wed Mar 20 12:22:24 2019
New Revision: 356598

URL: http://llvm.org/viewvc/llvm-project?rev=356598&view=rev
Log:
Recommit "Support attribute used in member funcs of class templates"

This diff previously exposed a bug in LLVM's IRLinker, breaking
buildbots that tried to self-host LLVM with monolithic LTO.
The bug is now in LLVM by D59552

Original commit message:
As PR17480 describes, clang does not support the used attribute
for member functions of class templates. This means that if the member
function is not used, its definition is never instantiated. This patch
changes clang to emit the definition if it has the used attribute.

Test Plan: Added a testcase

Reviewed By: aaron.ballman

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

Added:

cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=356598&r1=356597&r2=356598&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 20 12:22:24 2019
@@ -2232,6 +2232,20 @@ TemplateDeclInstantiator::VisitCXXMethod
 Owner->addDecl(Method);
   }
 
+  // PR17480: Honor the used attribute to instantiate member function
+  // definitions
+  if (Method->hasAttr()) {
+if (const auto *A = dyn_cast(Owner)) {
+  SourceLocation Loc;
+  if (const MemberSpecializationInfo *MSInfo =
+  A->getMemberSpecializationInfo())
+Loc = MSInfo->getPointOfInstantiation();
+  else if (const auto *Spec = dyn_cast(A))
+Loc = Spec->getPointOfInstantiation();
+  SemaRef.MarkFunctionReferenced(Loc, Method);
+}
+  }
+
   return Method;
 }
 

Added: 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp?rev=356598&view=auto
==
--- 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
(added)
+++ 
cfe/trunk/test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp 
Wed Mar 20 12:22:24 2019
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+// Check that PR17480 is fixed: __attribute__((used)) ignored in templated
+// classes
+namespace InstantiateUsedMemberDefinition {
+template 
+struct S {
+  int __attribute__((used)) f() {
+return 0;
+  }
+};
+
+void test() {
+  // Check that InstantiateUsedMemberDefinition::S::f() is defined
+  // as a result of the S class template implicit instantiation
+  // CHECK: define linkonce_odr i32 
@_ZN31InstantiateUsedMemberDefinition1SIiE1fEv
+  S inst;
+}
+} // namespace InstantiateUsedMemberDefinition


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

r356599 - [Sema] Deduplicate some availability checking logic

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Mar 20 12:26:33 2019
New Revision: 356599

URL: http://llvm.org/viewvc/llvm-project?rev=356599&view=rev
Log:
[Sema] Deduplicate some availability checking logic

Before this commit, we emit unavailable errors for calls to functions during
overload resolution, and for references to all other declarations in
DiagnoseUseOfDecl. The early checks during overload resolution aren't as good as
the DiagnoseAvailabilityOfDecl based checks, as they error on the code from
PR40991. This commit fixes this by removing the early checking.

llvm.org/PR40991
rdar://48564179

Differential revision: https://reviews.llvm.org/D59394

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/Sema/enable_if.c
cfe/trunk/test/Sema/overloadable.c
cfe/trunk/test/SemaCXX/attr-unavailable.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
cfe/trunk/test/SemaObjCXX/overload.mm
cfe/trunk/test/SemaTemplate/instantiate-expr-4.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356599&r1=356598&r2=356599&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Mar 20 12:26:33 
2019
@@ -3605,12 +3605,11 @@ def err_ovl_no_viable_member_function_in
   "no matching member function for call to %0">;
 def err_ovl_ambiguous_call : Error<
   "call to %0 is ambiguous">;
-def err_ovl_deleted_call : Error<
-  "call to %select{unavailable|deleted}0 function %1%2">;
+def err_ovl_deleted_call : Error<"call to deleted function %0">;
 def err_ovl_ambiguous_member_call : Error<
   "call to member function %0 is ambiguous">;
 def err_ovl_deleted_member_call : Error<
-  "call to %select{unavailable|deleted}0 member function %1%2">;
+  "call to deleted member function %0">;
 def note_ovl_too_many_candidates : Note<
 "remaining %0 candidate%s0 omitted; "
 "pass -fshow-overloads=all to show them">;
@@ -3824,7 +3823,7 @@ def err_ovl_ambiguous_init : Error<"call
 def err_ref_init_ambiguous : Error<
   "reference initialization of type %0 with initializer of type %1 is 
ambiguous">;
 def err_ovl_deleted_init : Error<
-  "call to %select{unavailable|deleted}0 constructor of %1">;
+  "call to deleted constructor of %0">;
 def err_ovl_deleted_special_init : Error<
   "call to implicitly-deleted %select{default constructor|copy constructor|"
   "move constructor|copy assignment operator|move assignment operator|"
@@ -3836,7 +3835,7 @@ def err_ovl_ambiguous_oper_binary : Erro
 def err_ovl_no_viable_oper : Error<"no viable overloaded '%0'">;
 def note_assign_lhs_incomplete : Note<"type %0 is incomplete">;
 def err_ovl_deleted_oper : Error<
-  "overload resolution selected %select{unavailable|deleted}0 operator 
'%1'%2">;
+  "overload resolution selected deleted operator '%0'">;
 def err_ovl_deleted_special_oper : Error<
   "object of type %0 cannot be %select{constructed|copied|moved|assigned|"
   "assigned|destroyed}1 because its %sub{select_special_member_kind}1 is 
implicitly deleted">;
@@ -3857,7 +3856,7 @@ def err_ovl_no_viable_object_call : Erro
 def err_ovl_ambiguous_object_call : Error<
   "call to object of type %0 is ambiguous">;
 def err_ovl_deleted_object_call : Error<
-  "call to %select{unavailable|deleted}0 function call operator in type %1%2">;
+  "call to deleted function call operator in type %0">;
 def note_ovl_surrogate_cand : Note<"conversion candidate of type %0">;
 def err_member_call_without_object : Error<
   "call to non-static member function without an object argument">;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=356599&r1=356598&r2=356599&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 20 12:26:33 2019
@@ -2634,13 +2634,6 @@ public:
   bool IsOverload(FunctionDecl *New, FunctionDecl *Old, bool IsForUsingDecl,
   bool ConsiderCudaAttrs = true);
 
-  /// Checks availability of the function depending on the current
-  /// function context.Inside an unavailable function,unavailability is 
ignored.
-  ///
-  /// \returns true if \p FD is unavailable and current context is inside
-  /// an available function, false otherwise.
-  bool isFunctionConsideredUnavailable(FunctionDecl *FD);
-
   ImplicitConversionSequence
   TryImplicitConversion(Expr *From, QualType ToType,
 bool SuppressUserConversions,
@@ -4102,7 +4095,6 @@ public:
  ObjCIn

r356600 - Add a __has_extension check for '#pragma clang attribute' as an external-declaration

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Mar 20 12:26:37 2019
New Revision: 356600

URL: http://llvm.org/viewvc/llvm-project?rev=356600&view=rev
Log:
Add a __has_extension check for '#pragma clang attribute' as an 
external-declaration

This was added in r356075.

Modified:
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/test/Parser/pragma-attribute-context.cpp

Modified: cfe/trunk/include/clang/Basic/Features.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=356600&r1=356599&r2=356600&view=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Wed Mar 20 12:26:37 2019
@@ -247,6 +247,7 @@ EXTENSION(cxx_variable_templates, LangOp
 // Miscellaneous language extensions
 EXTENSION(overloadable_unmarked, true)
 EXTENSION(pragma_clang_attribute_namespaces, true)
+EXTENSION(pragma_clang_attribute_external_declaration, true)
 
 #undef EXTENSION
 #undef FEATURE

Modified: cfe/trunk/test/Parser/pragma-attribute-context.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/pragma-attribute-context.cpp?rev=356600&r1=356599&r2=356600&view=diff
==
--- cfe/trunk/test/Parser/pragma-attribute-context.cpp (original)
+++ cfe/trunk/test/Parser/pragma-attribute-context.cpp Wed Mar 20 12:26:37 2019
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9.0.0 -verify -std=c++11 %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9.0.0 -xobjective-c++ -verify 
-std=c++11 %s
 
+#if !__has_extension(pragma_clang_attribute_external_declaration)
+#error
+#endif
+
 #define BEGIN_PRAGMA _Pragma("clang attribute push 
(__attribute__((availability(macos, introduced=1000))), apply_to=function)")
 #define END_PRAGMA _Pragma("clang attribute pop")
 


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

[PATCH] D59394: [Sema] De-duplicate some availability checking logic

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356599: [Sema] Deduplicate some availability checking logic 
(authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59394?vs=190747&id=191557#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59394

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/Sema/enable_if.c
  cfe/trunk/test/Sema/overloadable.c
  cfe/trunk/test/SemaCXX/attr-unavailable.cpp
  cfe/trunk/test/SemaCXX/coroutines.cpp
  cfe/trunk/test/SemaObjCXX/overload.mm
  cfe/trunk/test/SemaTemplate/instantiate-expr-4.cpp

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -2634,13 +2634,6 @@
   bool IsOverload(FunctionDecl *New, FunctionDecl *Old, bool IsForUsingDecl,
   bool ConsiderCudaAttrs = true);
 
-  /// Checks availability of the function depending on the current
-  /// function context.Inside an unavailable function,unavailability is ignored.
-  ///
-  /// \returns true if \p FD is unavailable and current context is inside
-  /// an available function, false otherwise.
-  bool isFunctionConsideredUnavailable(FunctionDecl *FD);
-
   ImplicitConversionSequence
   TryImplicitConversion(Expr *From, QualType ToType,
 bool SuppressUserConversions,
@@ -4102,7 +4095,6 @@
  ObjCInterfaceDecl *ClassReciever = nullptr);
   void NoteDeletedFunction(FunctionDecl *FD);
   void NoteDeletedInheritingConstructor(CXXConstructorDecl *CD);
-  std::string getDeletedOrUnavailableSuffix(const FunctionDecl *FD);
   bool DiagnosePropertyAccessorMismatch(ObjCPropertyDecl *PD,
 ObjCMethodDecl *Getter,
 SourceLocation Loc);
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3605,12 +3605,11 @@
   "no matching member function for call to %0">;
 def err_ovl_ambiguous_call : Error<
   "call to %0 is ambiguous">;
-def err_ovl_deleted_call : Error<
-  "call to %select{unavailable|deleted}0 function %1%2">;
+def err_ovl_deleted_call : Error<"call to deleted function %0">;
 def err_ovl_ambiguous_member_call : Error<
   "call to member function %0 is ambiguous">;
 def err_ovl_deleted_member_call : Error<
-  "call to %select{unavailable|deleted}0 member function %1%2">;
+  "call to deleted member function %0">;
 def note_ovl_too_many_candidates : Note<
 "remaining %0 candidate%s0 omitted; "
 "pass -fshow-overloads=all to show them">;
@@ -3824,7 +3823,7 @@
 def err_ref_init_ambiguous : Error<
   "reference initialization of type %0 with initializer of type %1 is ambiguous">;
 def err_ovl_deleted_init : Error<
-  "call to %select{unavailable|deleted}0 constructor of %1">;
+  "call to deleted constructor of %0">;
 def err_ovl_deleted_special_init : Error<
   "call to implicitly-deleted %select{default constructor|copy constructor|"
   "move constructor|copy assignment operator|move assignment operator|"
@@ -3836,7 +3835,7 @@
 def err_ovl_no_viable_oper : Error<"no viable overloaded '%0'">;
 def note_assign_lhs_incomplete : Note<"type %0 is incomplete">;
 def err_ovl_deleted_oper : Error<
-  "overload resolution selected %select{unavailable|deleted}0 operator '%1'%2">;
+  "overload resolution selected deleted operator '%0'">;
 def err_ovl_deleted_special_oper : Error<
   "object of type %0 cannot be %select{constructed|copied|moved|assigned|"
   "assigned|destroyed}1 because its %sub{select_special_member_kind}1 is implicitly deleted">;
@@ -3857,7 +3856,7 @@
 def err_ovl_ambiguous_object_call : Error<
   "call to object of type %0 is ambiguous">;
 def err_ovl_deleted_object_call : Error<
-  "call to %select{unavailable|deleted}0 function call operator in type %1%2">;
+  "call to deleted function call operator in type %0">;
 def note_ovl_surrogate_cand : Note<"conversion candidate of type %0">;
 def err_member_call_without_object : Error<
   "call to non-static member function without an object argument">;
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -649,26 +649,26 @@
 
 struct bad_promise_base {
 private:
-  void return_void();
+  void return_void(); // expected-note 2 {{declare