[PATCH] D75323: Support relative dest paths in headermap files

[Xiaoyi Shi via Phabricator via cfe-commits]

ashi009 added a comment.

Any update on this?


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

https://reviews.llvm.org/D75323

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

[clang] 2482648 - thinlto_embed_bitcode.ll: clarify grep should treat input as text

[Mircea Trofin via cfe-commits]

Author: Mircea Trofin
Date: 2020-11-21T21:46:53-08:00
New Revision: 2482648a795afbe12774168bbbf70dc14c031267

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

LOG: thinlto_embed_bitcode.ll: clarify grep should treat input as text

The input to the test's use of grep should be treated as text, and
that's not the case on certain Linux distros. Added --text.

Added: 


Modified: 
clang/test/CodeGen/thinlto_embed_bitcode.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto_embed_bitcode.ll 
b/clang/test/CodeGen/thinlto_embed_bitcode.ll
index 6c7e36e7226b..971d4005435d 100644
--- a/clang/test/CodeGen/thinlto_embed_bitcode.ll
+++ b/clang/test/CodeGen/thinlto_embed_bitcode.ll
@@ -18,7 +18,7 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c 
-fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s 
--check-prefixes=CHECK-ELF,CHECK-ELF-CMD
 ; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
-; RUN: grep x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
+; RUN: grep --text x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 



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

[clang] 1756d67 - [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

[Fangrui Song via cfe-commits]

Author: Ella Ma
Date: 2020-11-21T21:04:12-08:00
New Revision: 1756d67934bb5fe3b12bdb5fa55d61f61bd70bc5

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

LOG: [llvm][clang][mlir] Add checks for the return values from 
Target::createXXX to prevent protential null deref

All these potential null pointer dereferences are reported by my static 
analyzer for null smart pointer dereferences, which has a different 
implementation from `alpha.cplusplus.SmartPtr`.

The checked pointers in this patch are initialized by Target::createXXX 
functions. When the creator function pointer is not correctly set, a null 
pointer will be returned, or the creator function may originally return a null 
pointer.

Some of them may not make sense as they may be checked before entering the 
function, but I fixed them all in this patch. I submit this fix because 1) 
similar checks are found in some other places in the LLVM codebase for the same 
return value of the function; and, 2) some of the pointers are dereferenced 
before they are checked, which may definitely trigger a null pointer 
dereference if the return value is nullptr.

Reviewed By: tejohnson, MaskRay, jpienaar

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

Added: 


Modified: 
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
clang/tools/driver/cc1as_main.cpp
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
llvm/lib/CodeGen/LLVMTargetMachine.cpp
llvm/lib/CodeGen/ParallelCG.cpp
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/LTO/LTOCodeGenerator.cpp
llvm/lib/LTO/LTOModule.cpp
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
llvm/tools/llvm-exegesis/lib/LlvmState.cpp
llvm/tools/llvm-exegesis/llvm-exegesis.cpp
llvm/tools/llvm-mc/llvm-mc.cpp
llvm/tools/llvm-mca/llvm-mca.cpp
llvm/tools/llvm-ml/llvm-ml.cpp
llvm/tools/llvm-objdump/MachODump.cpp
llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
mlir/lib/Conversion/GPUCommon/ConvertKernelFuncToBlob.cpp
mlir/lib/ExecutionEngine/ExecutionEngine.cpp

Removed: 




diff  --git a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp 
b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
index 17ce40b5b1b6..4adb6eb39d09 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -107,11 +107,16 @@ static std::string OptLLVM(const std::string , 
CodeGenOpt::Level OLvl) {
   std::string E;
   const Target *TheTarget =
   TargetRegistry::lookupTarget(codegen::getMArch(), ModuleTriple, E);
-  TargetMachine *Machine = TheTarget->createTargetMachine(
+  if (!TheTarget)
+ErrorAndExit(E);
+
+  std::unique_ptr TM(TheTarget->createTargetMachine(
   M->getTargetTriple(), codegen::getCPUStr(), codegen::getFeaturesStr(),
   Options, codegen::getExplicitRelocModel(),
-  codegen::getExplicitCodeModel(), OLvl);
-  std::unique_ptr TM(Machine);
+  codegen::getExplicitCodeModel(), OLvl));
+  if (!TM)
+ErrorAndExit("Could not create target machine");
+
   codegen::setFunctionAttributes(codegen::getCPUStr(),
  codegen::getFeaturesStr(), *M);
 

diff  --git a/clang/tools/driver/cc1as_main.cpp 
b/clang/tools/driver/cc1as_main.cpp
index 07dddc6dc85b..de71026fbffe 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -428,8 +428,11 @@ static bool ExecuteAssembler(AssemblerInvocation ,
   std::unique_ptr Str;
 
   std::unique_ptr MCII(TheTarget->createMCInstrInfo());
+  assert(MCII && "Unable to create instruction info!");
+
   std::unique_ptr STI(
   TheTarget->createMCSubtargetInfo(Opts.Triple, Opts.CPU, FS));
+  assert(STI && "Unable to create subtarget info!");
 
   raw_pwrite_stream *Out = FDOS.get();
   std::unique_ptr BOS;
@@ -468,6 +471,8 @@ static bool ExecuteAssembler(AssemblerInvocation ,
 TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
+assert(MAB && "Unable to create asm backend!");
+
 std::unique_ptr OW =
 DwoOS ? MAB->createDwoObjectWriter(*Out, *DwoOS)
   : MAB->createObjectWriter(*Out);

diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp 
b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 8287a45e6722..4d7f17aa65bb 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -307,6 +307,7 @@ bool AsmPrinter::doInitialization(Module ) {
 std::unique_ptr STI(TM.getTarget().createMCSubtargetInfo(
 TM.getTargetTriple().str(), TM.getTargetCPU(),
 TM.getTargetFeatureString()));
+assert(STI && "Unable to 

[PATCH] D91223: Support struct annotations in FuchsiaHandleChecker.

[Haowei Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG914f6c4ff8a4: [StaticAnalyzer] Support struct annotations in 
FuchsiaHandleChecker (authored by haowei).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D91223?vs=306778=306890#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91223

Files:
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.cpp

Index: clang/test/Analysis/fuchsia_handle.cpp
===
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -9,9 +9,9 @@
 #define ZX_HANDLE_INVALID 0
 
 #if defined(__clang__)
-#define ZX_HANDLE_ACQUIRE  __attribute__((acquire_handle("Fuchsia")))
-#define ZX_HANDLE_RELEASE  __attribute__((release_handle("Fuchsia")))
-#define ZX_HANDLE_USE  __attribute__((use_handle("Fuchsia")))
+#define ZX_HANDLE_ACQUIRE __attribute__((acquire_handle("Fuchsia")))
+#define ZX_HANDLE_RELEASE __attribute__((release_handle("Fuchsia")))
+#define ZX_HANDLE_USE __attribute__((use_handle("Fuchsia")))
 #else
 #define ZX_HANDLE_ACQUIRE
 #define ZX_HANDLE_RELEASE
@@ -31,7 +31,7 @@
 
 void escape1(zx_handle_t *in);
 void escape2(zx_handle_t in);
-void (*escape3)(zx_handle_t) = escape2; 
+void (*escape3)(zx_handle_t) = escape2;
 
 void use1(const zx_handle_t *in ZX_HANDLE_USE);
 void use2(zx_handle_t in ZX_HANDLE_USE);
@@ -82,8 +82,8 @@
   // expected-note-re@-3 {{Handle allocated through {{(2nd|3rd)}} parameter}}
   if (status == 0) { // expected-note {{Assuming 'status' is equal to 0}}
  // expected-note@-1 {{Taking true branch}}
-return; // expected-warning {{Potential leak of handle}}
-// expected-note@-1 {{Potential leak of handle}}
+return;  // expected-warning {{Potential leak of handle}}
+ // expected-note@-1 {{Potential leak of handle}}
   }
   __builtin_trap();
 }
@@ -138,7 +138,7 @@
 return;
   zx_handle_close(sa);
   zx_handle_close(sb);
-} 
+}
 
 void checkLeak01(int tag) {
   zx_handle_t sa, sb;
@@ -158,7 +158,7 @@
   zx_handle_t sa = return_handle(); // expected-note {{Function 'return_handle' returns an open handle}}
   (void)sa;
 } // expected-note {{Potential leak of handle}}
-  // expected-warning@-1 {{Potential leak of handle}}
+// expected-warning@-1 {{Potential leak of handle}}
 
 void checkReportLeakOnOnePath(int tag) {
   zx_handle_t sa, sb;
@@ -166,26 +166,26 @@
 return;   // expected-note@-1 {{Assuming the condition is false}}
   // expected-note@-2 {{Taking false branch}}
   zx_handle_close(sb);
-  switch(tag) { // expected-note {{Control jumps to the 'default' case at line}} 
-case 0:
-  use2(sa);
-  return;
-case 1:
-  use2(sa);
-  return;
-case 2:
-  use2(sa);
-  return;
-case 3:
-  use2(sa);
-  return;
-case 4:
-  use2(sa);
-  return;
-default:
-  use2(sa);
-  return; // expected-warning {{Potential leak of handle}}
-  // expected-note@-1 {{Potential leak of handle}}
+  switch (tag) { // expected-note {{Control jumps to the 'default' case at line}}
+  case 0:
+use2(sa);
+return;
+  case 1:
+use2(sa);
+return;
+  case 2:
+use2(sa);
+return;
+  case 3:
+use2(sa);
+return;
+  case 4:
+use2(sa);
+return;
+  default:
+use2(sa);
+return; // expected-warning {{Potential leak of handle}}
+// expected-note@-1 {{Potential leak of handle}}
   }
 }
 
@@ -193,7 +193,7 @@
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
   // expected-note@-1 {{Handle allocated through 2nd parameter}}
-  if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
+  if (tag)   // expected-note {{Assuming 'tag' is not equal to 0}}
 zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
   // expected-note@-2 {{Taking true branch}}
   zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
@@ -211,12 +211,12 @@
   if (tag) {
 // expected-note@-1 {{Assuming 'tag' is not equal to 0}}
 zx_handle_close(sa); // expected-note {{Handle released through 1st parameter}}
-use1(); // expected-warning {{Using a previously released handle}}
+use1();   // expected-warning {{Using a previously released handle}}
 // expected-note@-1 {{Using a previously released handle}}
   }
   // expected-note@-6 {{Assuming 'tag' is 0}}
   zx_handle_close(sb); // expected-note {{Handle released through 1st parameter}}
-  use2(sb); // expected-warning {{Using a previously released handle}}
+  use2(sb);// expected-warning {{Using a previously released handle}}
   // expected-note@-1 {{Using a 

[clang] 914f6c4 - [StaticAnalyzer] Support struct annotations in FuchsiaHandleChecker

[Haowei Wu via cfe-commits]

Author: Haowei Wu
Date: 2020-11-21T19:59:51-08:00
New Revision: 914f6c4ff8a42d384cad0bbb07de4dd1a96c78d4

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

LOG: [StaticAnalyzer] Support struct annotations in FuchsiaHandleChecker

Support adding handle annotations to sturucture that contains
handles. All the handles referenced by the structure (direct
value or ptr) would be treated as containing the
release/use/acquire annotations directly.

Patch by Yu Shan

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/test/Analysis/fuchsia_handle.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
index b2822e5307f3..d4901eb0abbb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -53,7 +53,7 @@
 //
 // Note that, the analyzer does not always know for sure if a function failed
 // or succeeded. In those cases we use the state MaybeAllocated.
-// Thus, the diagramm above captures the intent, not implementation details.
+// Thus, the diagram above captures the intent, not implementation details.
 //
 // Due to the fact that the number of handle related syscalls in Fuchsia
 // is large, we adopt the annotation attributes to descript syscalls'
@@ -226,32 +226,70 @@ static const ExplodedNode *getAcquireSite(const 
ExplodedNode *N, SymbolRef Sym,
   return nullptr;
 }
 
-/// Returns the symbols extracted from the argument or null if it cannot be
-/// found.
-static SymbolRef getFuchsiaHandleSymbol(QualType QT, SVal Arg,
-ProgramStateRef State) {
+namespace {
+class FuchsiaHandleSymbolVisitor final : public SymbolVisitor {
+public:
+  FuchsiaHandleSymbolVisitor(ProgramStateRef State) : State(std::move(State)) 
{}
+  ProgramStateRef getState() const { return State; }
+
+  bool VisitSymbol(SymbolRef S) override {
+if (const auto *HandleType = S->getType()->getAs())
+  if (HandleType->getDecl()->getName() == HandleTypeName)
+Symbols.push_back(S);
+return true;
+  }
+
+  SmallVector GetSymbols() { return Symbols; }
+
+private:
+  SmallVector Symbols;
+  ProgramStateRef State;
+};
+} // end anonymous namespace
+
+/// Returns the symbols extracted from the argument or empty vector if it 
cannot
+/// be found. It is unlikely to have over 1024 symbols in one argument.
+static SmallVector
+getFuchsiaHandleSymbols(QualType QT, SVal Arg, ProgramStateRef State) {
   int PtrToHandleLevel = 0;
   while (QT->isAnyPointerType() || QT->isReferenceType()) {
 ++PtrToHandleLevel;
 QT = QT->getPointeeType();
   }
+  if (QT->isStructureType()) {
+// If we see a structure, see if there is any handle referenced by the
+// structure.
+FuchsiaHandleSymbolVisitor Visitor(State);
+State->scanReachableSymbols(Arg, Visitor);
+return Visitor.GetSymbols();
+  }
   if (const auto *HandleType = QT->getAs()) {
 if (HandleType->getDecl()->getName() != HandleTypeName)
-  return nullptr;
-if (PtrToHandleLevel > 1) {
+  return {};
+if (PtrToHandleLevel > 1)
   // Not supported yet.
-  return nullptr;
-}
+  return {};
 
 if (PtrToHandleLevel == 0) {
-  return Arg.getAsSymbol();
+  SymbolRef Sym = Arg.getAsSymbol();
+  if (Sym) {
+return {Sym};
+  } else {
+return {};
+  }
 } else {
   assert(PtrToHandleLevel == 1);
-  if (Optional ArgLoc = Arg.getAs())
-return State->getSVal(*ArgLoc).getAsSymbol();
+  if (Optional ArgLoc = Arg.getAs()) {
+SymbolRef Sym = State->getSVal(*ArgLoc).getAsSymbol();
+if (Sym) {
+  return {Sym};
+} else {
+  return {};
+}
+  }
 }
   }
-  return nullptr;
+  return {};
 }
 
 void FuchsiaHandleChecker::checkPreCall(const CallEvent ,
@@ -273,30 +311,31 @@ void FuchsiaHandleChecker::checkPreCall(const CallEvent 
,
 if (Arg >= FuncDecl->getNumParams())
   break;
 const ParmVarDecl *PVD = FuncDecl->getParamDecl(Arg);
-SymbolRef Handle =
-getFuchsiaHandleSymbol(PVD->getType(), Call.getArgSVal(Arg), State);
-if (!Handle)
-  continue;
+SmallVector Handles =
+getFuchsiaHandleSymbols(PVD->getType(), Call.getArgSVal(Arg), State);
 
 // Handled in checkPostCall.
 if (hasFuchsiaAttr(PVD) ||
 hasFuchsiaAttr(PVD))
   continue;
 
-const HandleState *HState = State->get(Handle);
-if (!HState || HState->isEscaped())
-  continue;
+for (SymbolRef Handle : Handles) {
+  const HandleState *HState = State->get(Handle);
+  if 

[PATCH] D87981: [X86] AMX programming model.

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added a comment.

@craig.topper. Thank you for the good idea. I create another patch 
(https://reviews.llvm.org/D91927) to fix the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

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

[PATCH] D91927: [X86] Add x86_amx type for intel AMX. The x86_amx is used for AMX intrisics. <256 x i32> is bitcast to x86_amx when it is used by AMX intrinsics, and x86_amx is bitcast to <256 x i32>

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, pengfei, 
hiraditya, qcolombet.
Herald added a reviewer: deadalnix.
Herald added projects: clang, LLVM.
LuoYuanke requested review of this revision.
Herald added a subscriber: jdoerfert.

...only operate on type x86_amx. It can separate amx intrinsics from llvm IR 
instructions (+-*/). Thank Craig for the idea. This patch depend on 
https://reviews.llvm.org/D87981.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91927

Files:
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/CodeGen/ValueTypes.td
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/Type.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -248,7 +248,8 @@
   IIT_V128 = 47,
   IIT_BF16 = 48,
   IIT_STRUCT9 = 49,
-  IIT_V256 = 50
+  IIT_V256 = 50,
+  IIT_AMX  = 51,
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -276,6 +277,7 @@
   case MVT::token: return Sig.push_back(IIT_TOKEN);
   case MVT::Metadata: return Sig.push_back(IIT_METADATA);
   case MVT::x86mmx: return Sig.push_back(IIT_MMX);
+  case MVT::x86amx: return Sig.push_back(IIT_AMX);
   // MVT::OtherVT is used to mean the empty struct type here.
   case MVT::Other: return Sig.push_back(IIT_EMPTYSTRUCT);
   // MVT::isVoid is used to represent varargs here.
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -76,6 +76,7 @@
   case MVT::f128: return "MVT::f128";
   case MVT::ppcf128:  return "MVT::ppcf128";
   case MVT::x86mmx:   return "MVT::x86mmx";
+  case MVT::x86amx:   return "MVT::x86amx";
   case MVT::Glue: return "MVT::Glue";
   case MVT::isVoid:   return "MVT::isVoid";
   case MVT::v1i1: return "MVT::v1i1";
Index: llvm/test/CodeGen/X86/AMX/amx-type.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-type.ll
+++ llvm/test/CodeGen/X86/AMX/amx-type.ll
@@ -12,14 +12,8 @@
 ; CHECK-LABEL: @test_load(
 ; CHECK-NEXT:[[TMP1:%.*]] = bitcast i8* [[IN:%.*]] to <256 x i32>*
 ; CHECK-NEXT:[[TMP2:%.*]] = bitcast i8* [[OUT:%.*]] to <256 x i32>*
-; CHECK-NEXT:[[TMP3:%.*]] = bitcast <256 x i32>* [[TMP1]] to <128 x i32>*
-; CHECK-NEXT:[[TMP4:%.*]] = load <128 x i32>, <128 x i32>* [[TMP3]], align 64
-; CHECK-NEXT:[[TMP5:%.*]] = getelementptr <128 x i32>, <128 x i32>* [[TMP3]], i32 1
-; CHECK-NEXT:[[TMP6:%.*]] = load <128 x i32>, <128 x i32>* [[TMP5]], align 64
-; CHECK-NEXT:[[TMP7:%.*]] = bitcast <256 x i32>* [[TMP2]] to <128 x i32>*
-; CHECK-NEXT:store <128 x i32> [[TMP4]], <128 x i32>* [[TMP7]], align 64
-; CHECK-NEXT:[[TMP8:%.*]] = getelementptr <128 x i32>, <128 x i32>* [[TMP7]], i32 1
-; CHECK-NEXT:store <128 x i32> [[TMP6]], <128 x i32>* [[TMP8]], align 64
+; CHECK-NEXT:[[TMP3:%.*]] = load <256 x i32>, <256 x i32>* [[TMP1]], align 64, [[TBAA2:!tbaa !.*]]
+; CHECK-NEXT:store <256 x i32> [[TMP3]], <256 x i32>* [[TMP2]], align 64, [[TBAA2]]
 ; CHECK-NEXT:ret void
 ;
   %1 = bitcast i8* %in to <256 x i32>*
@@ -29,18 +23,33 @@
   ret void
 }
 
+define dso_local <256 x i32> @foo(<256 x i32>* nocapture readonly byval(<256 x i32>) align 1024 %0, <256 x i32>* nocapture readonly byval(<256 x i32>) align 1024 %1) local_unnamed_addr #0 {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[X:%.*]] = load <256 x i32>, <256 x i32>* [[TMP0:%.*]], align 1024, [[TBAA5:!tbaa !.*]]
+; CHECK-NEXT:[[Y:%.*]] = load <256 x i32>, <256 x i32>* [[TMP1:%.*]], align 1024, [[TBAA5]]
+; CHECK-NEXT:[[ADD:%.*]] = add <256 x i32> [[Y]], [[X]]
+; CHECK-NEXT:ret <256 x i32> [[ADD]]
+;
+entry:
+  %x = load <256 x i32>, <256 x i32>* 

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 306884.
njames93 added a comment.

Fix unittest failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91885

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -166,6 +166,10 @@
   ClangTidyOptions Options;
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
+  ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
+   , false);
+  Context.setDiagnosticsEngine();
   TestCheck TestCheck();
   CHECK_ERROR(TestCheck.getLocal("Opt"), MissingOptionError,
   "option not found 'test.Opt'");
@@ -191,6 +195,10 @@
 
   ClangTidyContext Context(std::make_unique(
   ClangTidyGlobalOptions(), Options));
+  ClangTidyDiagnosticConsumer DiagConsumer(Context);
+  DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions,
+   , false);
+  Context.setDiagnosticsEngine();
   TestCheck TestCheck();
 
 #define CHECK_ERROR_INT(Name, Expected)\
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -10,7 +10,9 @@
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context) {
+diag("DiagWithNoLoc");
+  }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override {
 Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
   }
@@ -26,9 +28,10 @@
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", );
-  EXPECT_EQ(2ul, Errors.size());
-  EXPECT_EQ("type specifier", Errors[0].Message.Message);
-  EXPECT_EQ("variable", Errors[1].Message.Message);
+  EXPECT_EQ(3ul, Errors.size());
+  EXPECT_EQ("DiagWithNoLoc", Errors[0].Message.Message);
+  EXPECT_EQ("type specifier", Errors[1].Message.Message);
+  EXPECT_EQ("variable", Errors[2].Message.Message);
 }
 
 } // namespace test
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-case-violation.cpp
@@ -5,11 +5,11 @@
 // RUN:   {key: readability-identifier-naming.ClassCase, value: UUPER_CASE}, \
 // RUN:   {key: readability-identifier-naming.StructCase, value: CAMEL}, \
 // RUN:   {key: readability-identifier-naming.EnumCase, value: AnY_cASe}, \
-// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not warning
+// RUN:   ]}" 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
 
-// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'?{{$}}
-// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'?{{$}}
+// CHECK-DAG: warning: invalid configuration value 'camelback' for option 'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'? [clang-tidy-config]
+// CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'? 

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

[Brooks Moses via Phabricator via cfe-commits]

brooksmoses added a comment.

Aha, okay.  I hadn't realized that this optimization had a 
`-fno-delete-null-pointer-checks` option to disable it.  I agree that since 
that's available there's no call for a rollback.

(I'll also make sure upstream bugs are filed for the cases in OpenJDK and 
Verilator that I know about, though I imagine those are likely to take a little 
while to make it out into the ecosystem.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17993

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

[PATCH] D91925: [clangd][NFC] Small tweak to combined provider

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: sammccall, Bigcheese.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This should address the FIXME about clang3.9 dervied to base unique_ptr 
constructor not working.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91925

Files:
  clang-tools-extra/clangd/ConfigProvider.cpp


Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -206,8 +206,8 @@
 
 std::unique_ptr
 Provider::combine(std::vector Providers) {
-  struct CombinedProvider : Provider {
-std::vector Providers;
+  class CombinedProvider : public Provider {
+const std::vector Providers;
 
 std::vector
 getFragments(const Params , DiagnosticCallback DC) const override {
@@ -218,14 +218,13 @@
   }
   return Result;
 }
+
+  public:
+CombinedProvider(std::vector Providers)
+: Providers(std::move(Providers)) {}
   };
-  auto Result = std::make_unique();
-  Result->Providers = std::move(Providers);
-  // FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
-  //   The constructor that is supposed to allow for Derived to Base
-  //   conversion does not work. Remove this if we drop support for such
-  //   configurations.
-  return std::unique_ptr(Result.release());
+
+  return std::make_unique(std::move(Providers));
 }
 
 Config Provider::getConfig(const Params , DiagnosticCallback DC) const {


Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -206,8 +206,8 @@
 
 std::unique_ptr
 Provider::combine(std::vector Providers) {
-  struct CombinedProvider : Provider {
-std::vector Providers;
+  class CombinedProvider : public Provider {
+const std::vector Providers;
 
 std::vector
 getFragments(const Params , DiagnosticCallback DC) const override {
@@ -218,14 +218,13 @@
   }
   return Result;
 }
+
+  public:
+CombinedProvider(std::vector Providers)
+: Providers(std::move(Providers)) {}
   };
-  auto Result = std::make_unique();
-  Result->Providers = std::move(Providers);
-  // FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
-  //   The constructor that is supposed to allow for Derived to Base
-  //   conversion does not work. Remove this if we drop support for such
-  //   configurations.
-  return std::unique_ptr(Result.release());
+
+  return std::make_unique(std::move(Providers));
 }
 
 Config Provider::getConfig(const Params , DiagnosticCallback DC) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@jwakely It looks like `UnsizedTail` causes a crash.

Other than that all the tests in this PR pass. It also looks like there's (at 
least) some support for unions and bitfields. This patch doesn't support those 
but I'm planning to add support as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[libunwind] 3324fd8 - [libunwind] Delete unused handlerNotFound in unwind_phase1

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-11-21T12:38:00-08:00
New Revision: 3324fd8a7b1ab011513017ed8fd81e06928526d5

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

LOG: [libunwind] Delete unused handlerNotFound in unwind_phase1

Added: 


Modified: 
libunwind/src/UnwindLevel1.c

Removed: 




diff  --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 3e75b5f13cd6..68e5e48b8c05 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libunwind/src/UnwindLevel1.c
@@ -39,8 +39,7 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   __unw_init_local(cursor, uc);
 
   // Walk each frame looking for a place to stop.
-  bool handlerNotFound = true;
-  while (handlerNotFound) {
+  while (true) {
 // Ask libunwind to get next frame (skip over first which is
 // _Unwind_RaiseException).
 int stepResult = __unw_step(cursor);
@@ -102,7 +101,6 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   case _URC_HANDLER_FOUND:
 // found a catch clause or locals that need destructing in this frame
 // stop search and remember stack pointer at the frame
-handlerNotFound = false;
 __unw_get_reg(cursor, UNW_REG_SP, );
 exception_object->private_2 = (uintptr_t)sp;
 _LIBUNWIND_TRACE_UNWINDING(



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

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

[Shane via Phabricator via cfe-commits]

smhc updated this revision to Diff 306870.
smhc added a comment.

Thanks - I had it like that originally but was not sure how often 
MacroUsageCallbacks would be instantiated or whether there was a many-to-one 
relationship. I've now modified it to create and maintain the regex object as a 
member of MacroUsageCallbacks.


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

https://reviews.llvm.org/D91908

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -32,8 +32,8 @@
 class MacroUsageCallbacks : public PPCallbacks {
 public:
   MacroUsageCallbacks(MacroUsageCheck *Check, const SourceManager ,
-  StringRef RegExp, bool CapsOnly, bool IgnoreCommandLine)
-  : Check(Check), SM(SM), RegExp(RegExp), CheckCapsOnly(CapsOnly),
+  StringRef RegExpStr, bool CapsOnly, bool 
IgnoreCommandLine)
+  : Check(Check), SM(SM), RegExp(RegExpStr), CheckCapsOnly(CapsOnly),
 IgnoreCommandLineMacros(IgnoreCommandLine) {}
   void MacroDefined(const Token ,
 const MacroDirective *MD) override {
@@ -47,7 +47,7 @@
   return;
 
 StringRef MacroName = MacroNameTok.getIdentifierInfo()->getName();
-if (!CheckCapsOnly && !llvm::Regex(RegExp).match(MacroName))
+if (!CheckCapsOnly && !RegExp.match(MacroName))
   Check->warnMacro(MD, MacroName);
 
 if (CheckCapsOnly && !isCapsOnly(MacroName))
@@ -57,7 +57,7 @@
 private:
   MacroUsageCheck *Check;
   const SourceManager 
-  StringRef RegExp;
+  const llvm::Regex RegExp;
   bool CheckCapsOnly;
   bool IgnoreCommandLineMacros;
 };


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -32,8 +32,8 @@
 class MacroUsageCallbacks : public PPCallbacks {
 public:
   MacroUsageCallbacks(MacroUsageCheck *Check, const SourceManager ,
-  StringRef RegExp, bool CapsOnly, bool IgnoreCommandLine)
-  : Check(Check), SM(SM), RegExp(RegExp), CheckCapsOnly(CapsOnly),
+  StringRef RegExpStr, bool CapsOnly, bool IgnoreCommandLine)
+  : Check(Check), SM(SM), RegExp(RegExpStr), CheckCapsOnly(CapsOnly),
 IgnoreCommandLineMacros(IgnoreCommandLine) {}
   void MacroDefined(const Token ,
 const MacroDirective *MD) override {
@@ -47,7 +47,7 @@
   return;
 
 StringRef MacroName = MacroNameTok.getIdentifierInfo()->getName();
-if (!CheckCapsOnly && !llvm::Regex(RegExp).match(MacroName))
+if (!CheckCapsOnly && !RegExp.match(MacroName))
   Check->warnMacro(MD, MacroName);
 
 if (CheckCapsOnly && !isCapsOnly(MacroName))
@@ -57,7 +57,7 @@
 private:
   MacroUsageCheck *Check;
   const SourceManager 
-  StringRef RegExp;
+  const llvm::Regex RegExp;
   bool CheckCapsOnly;
   bool IgnoreCommandLineMacros;
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90329: [PowerPC] Fix va_arg in C++, Objective-C on 32-bit ELF targets

[Brad Smith via Phabricator via cfe-commits]

brad added a comment.

@efriedma Eli?


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

https://reviews.llvm.org/D90329

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

I applied the patch locally and it fixes most of the linker errors but I'm 
still seeing one:

  [6/393] Linking CXX executable bin/clangd-index-server
  FAILED: bin/clangd-index-server
  : && /usr/bin/clang++-10 -fPIC -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections 
-fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG 
-fuse-ld=gold -Wl,-O3 -Wl,--gc-sections 
tools/clang/tools/extra/clangd/index/remote/server/CMakeFiles/clangd-index-server.dir/Server.cpp.o
 -o bin/clangd-index-server  -Wl,-rpath,"\$ORIGIN/../lib"  -lpthread  
lib/libRemoteIndexServiceProto.so.12git  
lib/libclangdRemoteMarshalling.so.12git  -lgrpc++  lib/libclangDaemon.so.12git  
lib/libclangdSupport.so.12git  lib/libRemoteIndexProto.so.12git  
lib/libLLVMSupport.so.12git  -Wl,-rpath-link,llvm/prod-build/lib && :
  
tools/clang/tools/extra/clangd/index/remote/server/CMakeFiles/clangd-index-server.dir/Server.cpp.o:Server.cpp:function
 llvm::detail::stream_operator_format_adapter::format(llvm::raw_ostream&, 
llvm::StringRef): error: undefined reference to 
'google::protobuf::Message::DebugString[abi:cxx11]() const'
  clang: error: linker command failed with exit code 1 (use -v to see 
invocation)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91918: Remove the IgnoreImplicitCastsAndParentheses traversal kind

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
steveire requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91918

Files:
  clang-tools-extra/clang-query/QueryParser.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ParentMapContext.cpp


Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -36,8 +36,6 @@
   switch (Traversal) {
   case TK_AsIs:
 return E;
-  case TK_IgnoreImplicitCastsAndParentheses:
-return E->IgnoreParenImpCasts();
   case TK_IgnoreUnlessSpelledInSource:
 return E->IgnoreUnlessSpelledInSource();
   }
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -40,10 +40,6 @@
   /// Will traverse all child nodes.
   TK_AsIs,
 
-  /// Will not traverse implicit casts and parentheses.
-  /// Corresponds to Expr::IgnoreParenImpCasts()
-  TK_IgnoreImplicitCastsAndParentheses,
-
   /// Ignore AST nodes not written in the source
   TK_IgnoreUnlessSpelledInSource
 };
@@ -542,8 +538,6 @@
 using TraversalKind = ::clang::TraversalKind;
 
 constexpr TraversalKind TK_AsIs = ::clang::TK_AsIs;
-constexpr TraversalKind TK_IgnoreImplicitCastsAndParentheses =
-::clang::TK_IgnoreImplicitCastsAndParentheses;
 constexpr TraversalKind TK_IgnoreUnlessSpelledInSource =
 ::clang::TK_IgnoreUnlessSpelledInSource;
 } // namespace ast_type_traits
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -126,9 +126,6 @@
 switch (Traversal) {
 case TK_AsIs:
   break;
-case TK_IgnoreImplicitCastsAndParentheses:
-  S = E->IgnoreParenImpCasts();
-  break;
 case TK_IgnoreUnlessSpelledInSource:
   S = E->IgnoreUnlessSpelledInSource();
   break;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -234,6 +234,9 @@
   has been changed to no longer match on template instantiations or on
   implicit nodes which are not spelled in the source.
 
+- The TK_IgnoreImplicitCastsAndParentheses traversal kind was removed. It
+  is recommended to use TK_IgnoreUnlessSpelledInSource instead.
+
 - The behavior of the forEach() matcher was changed to not internally ignore
   implicit and parenthesis nodes.
 
Index: clang-tools-extra/clang-query/QueryParser.cpp
===
--- clang-tools-extra/clang-query/QueryParser.cpp
+++ clang-tools-extra/clang-query/QueryParser.cpp
@@ -134,8 +134,6 @@
   unsigned Value =
   LexOrCompleteWord(this, ValStr)
   .Case("AsIs", ast_type_traits::TK_AsIs)
-  .Case("IgnoreImplicitCastsAndParentheses",
-ast_type_traits::TK_IgnoreImplicitCastsAndParentheses)
   .Case("IgnoreUnlessSpelledInSource",
 ast_type_traits::TK_IgnoreUnlessSpelledInSource)
   .Default(~0u);


Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -36,8 +36,6 @@
   switch (Traversal) {
   case TK_AsIs:
 return E;
-  case TK_IgnoreImplicitCastsAndParentheses:
-return E->IgnoreParenImpCasts();
   case TK_IgnoreUnlessSpelledInSource:
 return E->IgnoreUnlessSpelledInSource();
   }
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -40,10 +40,6 @@
   /// Will traverse all child nodes.
   TK_AsIs,
 
-  /// Will not traverse implicit casts and parentheses.
-  /// Corresponds to Expr::IgnoreParenImpCasts()
-  TK_IgnoreImplicitCastsAndParentheses,
-
   /// Ignore AST nodes not written in the source
   TK_IgnoreUnlessSpelledInSource
 };
@@ -542,8 +538,6 @@
 using TraversalKind = ::clang::TraversalKind;
 
 constexpr TraversalKind TK_AsIs = ::clang::TK_AsIs;
-constexpr TraversalKind TK_IgnoreImplicitCastsAndParentheses =
-::clang::TK_IgnoreImplicitCastsAndParentheses;
 constexpr TraversalKind TK_IgnoreUnlessSpelledInSource =
 ::clang::TK_IgnoreUnlessSpelledInSource;
 } // namespace ast_type_traits
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- 

[PATCH] D91916: Remove automatic traversal from forEach matcher

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 306858.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91916

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3370,6 +3370,38 @@
 std::make_unique>("f", 4)));
 }
 
+TEST(ForEach, DoesNotIgnoreImplicit) {
+  StringRef Code = R"cpp(
+void foo()
+{
+int i = 0;
+int b = 4;
+i < b;
+}
+)cpp";
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+  Code, binaryOperator(forEach(declRefExpr().bind("dre"))),
+  std::make_unique>("dre", 0)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  binaryOperator(forEach(
+  implicitCastExpr(hasSourceExpression(declRefExpr().bind("dre"),
+  std::make_unique>("dre", 2)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  binaryOperator(
+  forEach(expr(ignoringImplicit(declRefExpr().bind("dre"),
+  std::make_unique>("dre", 2)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   binaryOperator(forEach(declRefExpr().bind("dre",
+  std::make_unique>("dre", 2)));
+}
+
 TEST(ForEachDescendant, BindsOneNode) {
   EXPECT_TRUE(matchAndVerifyResultTrue("class C { class D { int x; }; };",
recordDecl(hasName("C"),
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -70,7 +70,6 @@
   internal::Matcher, AMatcher) {
   return Finder->matchesChildOf(
   Node, AMatcher, Builder,
-  TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
   ASTMatchFinder::BK_First);
 }
 
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -95,12 +95,11 @@
   // matching the descendants.
   MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder, int MaxDepth,
-   TraversalKind Traversal, bool IgnoreImplicitChildren,
+   bool IgnoreImplicitChildren,
ASTMatchFinder::BindKind Bind)
   : Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
-MaxDepth(MaxDepth), Traversal(Traversal),
-IgnoreImplicitChildren(IgnoreImplicitChildren), Bind(Bind),
-Matches(false) {}
+MaxDepth(MaxDepth), IgnoreImplicitChildren(IgnoreImplicitChildren),
+Bind(Bind), Matches(false) {}
 
   // Returns true if a match is found in the subtree rooted at the
   // given AST node. This is done via a set of mutually recursive
@@ -168,10 +167,6 @@
 Finder->getASTContext().getParentMapContext().traverseIgnored(
 ExprNode);
 }
-if (Traversal == TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
-  if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
-StmtToTraverse = ExprNode->IgnoreParenImpCasts();
-}
 return StmtToTraverse;
   }
 
@@ -371,7 +366,6 @@
   BoundNodesTreeBuilder ResultBindings;
   int CurrentDepth;
   const int MaxDepth;
-  const TraversalKind Traversal;
   const bool IgnoreImplicitChildren;
   const ASTMatchFinder::BindKind Bind;
   bool Matches;
@@ -473,11 +467,10 @@
   bool memoizedMatchesRecursively(const DynTypedNode , ASTContext ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  BindKind Bind) {
 // For AST-nodes that don't have an identity, we can't memoize.
 if (!Node.getMemoizationData() || !Builder->isComparable())
-  return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
-Bind);
+  return matchesRecursively(Node, Matcher, Builder, MaxDepth, Bind);
 
 MatchKey Key;
 Key.MatcherID = Matcher.getID();
@@ -495,8 +488,8 @@
 
 MemoizedMatchResult Result;
 Result.Nodes = *Builder;
-Result.ResultOfMatch = matchesRecursively(Node, Matcher, ,
-  MaxDepth, Traversal, Bind);
+ 

[PATCH] D91917: Update mode used in traverse() examples

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
steveire requested review of this revision.

traverse() predates the IgnoreUnlessSpelledInSource mode. Update example
and test code to use the newer mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91917

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1873,8 +1873,8 @@
   auto Matcher = varDecl(hasInitializer(floatLiteral()));
 
   EXPECT_TRUE(notMatches(VarDeclCode, traverse(TK_AsIs, Matcher)));
-  EXPECT_TRUE(matches(VarDeclCode,
-  traverse(TK_IgnoreImplicitCastsAndParentheses, 
Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode, traverse(TK_IgnoreUnlessSpelledInSource, Matcher)));
 
   auto ParentMatcher = floatLiteral(hasParent(varDecl(hasName("i";
 
@@ -2715,14 +2715,14 @@
 )cpp";
 
   EXPECT_TRUE(
-  matches(Code, traverse(TK_IgnoreImplicitCastsAndParentheses,
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
  callExpr(has(callExpr(traverse(
  TK_AsIs, callExpr(has(implicitCastExpr(
   has(floatLiteral(;
 
   EXPECT_TRUE(matches(
   Code,
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
traverse(TK_AsIs, implicitCastExpr(has(floatLiteral()));
 }
 
@@ -2738,8 +2738,7 @@
 }
   )cpp";
 
-  auto Matcher =
-  traverse(TK_IgnoreImplicitCastsAndParentheses, implicitCastExpr());
+  auto Matcher = traverse(TK_IgnoreUnlessSpelledInSource, implicitCastExpr());
 
   // Verfiy that it does not segfault
   EXPECT_FALSE(matches(Code, Matcher));
@@ -2766,7 +2765,7 @@
   EXPECT_TRUE(matches(
   Code,
   functionDecl(anyOf(hasDescendant(Matcher),
- traverse(TK_IgnoreImplicitCastsAndParentheses,
+ traverse(TK_IgnoreUnlessSpelledInSource,
   functionDecl(hasDescendant(Matcher)));
 }
 
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -782,7 +782,7 @@
 /// \endcode
 /// The matcher
 /// \code
-///   traverse(TK_IgnoreImplicitCastsAndParentheses,
+///   traverse(TK_IgnoreUnlessSpelledInSource,
 /// varDecl(hasInitializer(floatLiteral().bind("init")))
 ///   )
 /// \endcode
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -5567,7 +5567,7 @@
   int i = 3.0;
   }
 The matcher
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
 varDecl(hasInitializer(floatLiteral().bind("init")))
   )
 matches the variable declaration with "init" bound to the "3.0".


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1873,8 +1873,8 @@
   auto Matcher = varDecl(hasInitializer(floatLiteral()));
 
   EXPECT_TRUE(notMatches(VarDeclCode, traverse(TK_AsIs, Matcher)));
-  EXPECT_TRUE(matches(VarDeclCode,
-  traverse(TK_IgnoreImplicitCastsAndParentheses, Matcher)));
+  EXPECT_TRUE(
+  matches(VarDeclCode, traverse(TK_IgnoreUnlessSpelledInSource, Matcher)));
 
   auto ParentMatcher = floatLiteral(hasParent(varDecl(hasName("i";
 
@@ -2715,14 +2715,14 @@
 )cpp";
 
   EXPECT_TRUE(
-  matches(Code, traverse(TK_IgnoreImplicitCastsAndParentheses,
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
  callExpr(has(callExpr(traverse(
  TK_AsIs, callExpr(has(implicitCastExpr(
   has(floatLiteral(;
 
   EXPECT_TRUE(matches(
   Code,
-  traverse(TK_IgnoreImplicitCastsAndParentheses,
+  traverse(TK_IgnoreUnlessSpelledInSource,
traverse(TK_AsIs, implicitCastExpr(has(floatLiteral()));
 }
 
@@ -2738,8 +2738,7 @@
 }
   )cpp";
 
-  auto Matcher =
-  traverse(TK_IgnoreImplicitCastsAndParentheses, implicitCastExpr());
+  auto Matcher = traverse(TK_IgnoreUnlessSpelledInSource, implicitCastExpr());
 
   // Verfiy that it does not segfault
   

[PATCH] D91916: Remove automatic traversal from forEach matcher

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
steveire requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91916

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3370,6 +3370,38 @@
 std::make_unique>("f", 4)));
 }
 
+TEST(ForEach, DoesNotIgnoreImplicit) {
+  StringRef Code = R"cpp(
+void foo()
+{
+int i = 0;
+int b = 4;
+i < b;
+}
+)cpp";
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+  Code, binaryOperator(forEach(declRefExpr().bind("dre"))),
+  std::make_unique>("dre", 0)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  binaryOperator(forEach(
+  implicitCastExpr(hasSourceExpression(declRefExpr().bind("dre"),
+  std::make_unique>("dre", 2)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  binaryOperator(
+  forEach(expr(ignoringImplicit(declRefExpr().bind("dre"),
+  std::make_unique>("dre", 2)));
+
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   binaryOperator(forEach(declRefExpr().bind("dre",
+  std::make_unique>("dre", 2)));
+}
+
 TEST(ForEachDescendant, BindsOneNode) {
   EXPECT_TRUE(matchAndVerifyResultTrue("class C { class D { int x; }; };",
recordDecl(hasName("C"),
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -70,7 +70,6 @@
   internal::Matcher, AMatcher) {
   return Finder->matchesChildOf(
   Node, AMatcher, Builder,
-  TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
   ASTMatchFinder::BK_First);
 }
 
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -95,12 +95,11 @@
   // matching the descendants.
   MatchChildASTVisitor(const DynTypedMatcher *Matcher, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder, int MaxDepth,
-   TraversalKind Traversal, bool IgnoreImplicitChildren,
+   bool IgnoreImplicitChildren,
ASTMatchFinder::BindKind Bind)
   : Matcher(Matcher), Finder(Finder), Builder(Builder), CurrentDepth(0),
-MaxDepth(MaxDepth), Traversal(Traversal),
-IgnoreImplicitChildren(IgnoreImplicitChildren), Bind(Bind),
-Matches(false) {}
+MaxDepth(MaxDepth), IgnoreImplicitChildren(IgnoreImplicitChildren),
+Bind(Bind), Matches(false) {}
 
   // Returns true if a match is found in the subtree rooted at the
   // given AST node. This is done via a set of mutually recursive
@@ -168,10 +167,6 @@
 Finder->getASTContext().getParentMapContext().traverseIgnored(
 ExprNode);
 }
-if (Traversal == TraversalKind::TK_IgnoreImplicitCastsAndParentheses) {
-  if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
-StmtToTraverse = ExprNode->IgnoreParenImpCasts();
-}
 return StmtToTraverse;
   }
 
@@ -371,7 +366,6 @@
   BoundNodesTreeBuilder ResultBindings;
   int CurrentDepth;
   const int MaxDepth;
-  const TraversalKind Traversal;
   const bool IgnoreImplicitChildren;
   const ASTMatchFinder::BindKind Bind;
   bool Matches;
@@ -473,11 +467,10 @@
   bool memoizedMatchesRecursively(const DynTypedNode , ASTContext ,
   const DynTypedMatcher ,
   BoundNodesTreeBuilder *Builder, int MaxDepth,
-  TraversalKind Traversal, BindKind Bind) {
+  BindKind Bind) {
 // For AST-nodes that don't have an identity, we can't memoize.
 if (!Node.getMemoizationData() || !Builder->isComparable())
-  return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
-Bind);
+  return matchesRecursively(Node, Matcher, Builder, MaxDepth, Bind);
 
 MatchKey Key;
 Key.MatcherID = Matcher.getID();
@@ -495,8 +488,8 @@
 
 MemoizedMatchResult Result;
 Result.Nodes = *Builder;
-Result.ResultOfMatch = matchesRecursively(Node, Matcher, ,
-  MaxDepth, 

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.

Addresses https://bugs.llvm.org/show_bug.cgi?id=48230.
Handle the case when the Fixup suggested isn't a valid c/c++ identifer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91915

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
@@ -25,3 +25,13 @@
 
   return 0;
 }
+
+int func3(int _0Bad) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for 
parameter '_0Bad'; cannot be fixed because '0_bad' is not a valid identifier 
[readability-identifier-naming]
+  // CHECK-FIXES: {{^}}int func3(int _0Bad) {{{$}}
+  if (_0Bad == 1) {
+// CHECK-FIXES: {{^}}  if (_0Bad == 1) {{{$}}
+return 2;
+  }
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -59,6 +59,9 @@
 /// automatically.
 ConflictsWithMacroDefinition,
 
+/// The fixup results in an identifier that is not a valid c/c++ 
identifier.
+FixInvalidIdentifier,
+
 /// Values pass this threshold will be ignored completely
 /// i.e no message, no fixup.
 IgnoreFailureThreshold,
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -10,6 +10,7 @@
 #include "ASTUtils.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
@@ -463,6 +464,8 @@
 Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;
   else if (Ident->hasMacroDefinition())
 Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition;
+} else if (!isValidIdentifier(Info.Fixup)) {
+  Failure.FixStatus = ShouldFixStatus::FixInvalidIdentifier;
 }
 
 Failure.Info = std::move(Info);
@@ -517,6 +520,9 @@
   RenamerClangTidyCheck::ShouldFixStatus::ConflictsWithMacroDefinition)
 return "; cannot be fixed because '" + Fixup +
"' would conflict with a macro definition";
+  if (FixStatus == 
RenamerClangTidyCheck::ShouldFixStatus::FixInvalidIdentifier)
+return "; cannot be fixed because '" + Fixup +
+   "' is not a valid identifier";
 
   llvm_unreachable("invalid ShouldFixStatus");
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-bugfix-name-conflicts.cpp
@@ -25,3 +25,13 @@
 
   return 0;
 }
+
+int func3(int _0Bad) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for parameter '_0Bad'; cannot be fixed because '0_bad' is not a valid identifier [readability-identifier-naming]
+  // CHECK-FIXES: {{^}}int func3(int _0Bad) {{{$}}
+  if (_0Bad == 1) {
+// CHECK-FIXES: {{^}}  if (_0Bad == 1) {{{$}}
+return 2;
+  }
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -59,6 +59,9 @@
 /// automatically.
 ConflictsWithMacroDefinition,
 
+/// The fixup results in an identifier that is not a valid c/c++ identifier.
+FixInvalidIdentifier,
+
 /// Values pass this threshold will be ignored completely
 /// i.e no message, no fixup.
 IgnoreFailureThreshold,
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ 

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D91913#2409688 , @lebedev.ri wrote:

> Then the comment needs to be fixed too i would think?

It's the comment right above the code that I changed; I did fix that too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

> The associated comment suggested that GCC keeps this extension enabled in 
> C90/C++03 standard-conforming mode, but it actually does not, so rather than 
> adding a check for C++ language version, this change simply removes the check 
> for C language version.

Then the comment needs to be fixed too i would think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91913

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

[PATCH] D91410: [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

[Jacques Pienaar via Phabricator via cfe-commits]

jpienaar accepted this revision.
jpienaar added a comment.

LG MLIR changes


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

https://reviews.llvm.org/D91410

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

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

[Gabor Bencze via Phabricator via cfe-commits]

gbencze added a comment.

Thanks for the review! I'll push this as soon as the updated version of D89649 
 is also accepted.


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

https://reviews.llvm.org/D89651

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

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

[Gabor Bencze via Phabricator via cfe-commits]

gbencze added a comment.

Pinging @rsmith - could you please check the updates to this patch? Thanks!


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

https://reviews.llvm.org/D89649

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

[PATCH] D91029: [clangd] Implement clang-tidy options from config

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 306850.
njames93 added a comment.

Fix ChecksToDisable to actually disable the checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91029

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/TidyProvider.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -17,6 +17,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
+#include "../TidyProvider.h"
 #include "Compiler.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
@@ -58,8 +59,7 @@
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
-  llvm::Optional ClangTidyChecks;
-  llvm::Optional ClangTidyWarningsAsErrors;
+  mutable TidyProvider ClangTidyProvider = {};
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -59,8 +59,8 @@
 FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = 
   Inputs.Opts = ParseOptions();
-  Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
-  Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
+  if (ClangTidyProvider)
+Inputs.ClangTidyProvider = ClangTidyProvider;
   Inputs.Index = ExternalIndex;
   if (Inputs.Index)
 Inputs.Opts.SuggestMissingIncludes = true;
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -24,6 +24,7 @@
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -250,7 +251,8 @@
   TestTU TU;
   // this check runs the preprocessor, we need to make sure it does not break
   // our recording logic.
-  TU.ClangTidyChecks = "modernize-use-trailing-return-type";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("modernize-use-trailing-return-type");
   TU.Code = "inline int foo() {}";
 
   auto AST = TU.build();
@@ -406,7 +408,7 @@
   "replay-preamble-module", "");
   TestTU TU;
   // This check records inclusion directives replayed by clangd.
-  TU.ClangTidyChecks = "replay-preamble-check";
+  TU.ClangTidyProvider = fixedTidyProvider("replay-preamble-check");
   llvm::Annotations Test(R"cpp(
 $hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
 $hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -14,6 +14,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "index/MemIndex.h"
 #include "support/Path.h"
 #include "clang/Basic/Diagnostic.h"
@@ -129,7 +130,7 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ClangTidyChecks = "-*,google-explicit-constructor";
+  TU.ClangTidyProvider = fixedTidyProvider("-*,google-explicit-constructor");
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   ElementsAre(
@@ -201,8 +202,9 @@
   auto TU = TestTU::withCode(Test.code());
   // Enable alias clang-tidy checks, these check emits the same diagnostics
   // (except the check name).
-  TU.ClangTidyChecks = "-*, readability-uppercase-literal-suffix, "
-   "hicpp-uppercase-literal-suffix";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("-*, readability-uppercase-literal-suffix, "
+"hicpp-uppercase-literal-suffix");
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
   TU.build().getDiagnostics(),
@@ -245,9 +247,9 @@
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   TU.HeaderFilename = 

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

While I'm a fan of this change, I think this is the wrong way to do it. Leave 
the check the same, but build the regex in the pp callback constructor. That 
should only get called once for the lifetime of the check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91908

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

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk created this revision.
hvdijk added a reviewer: rsmith.
hvdijk added a project: clang.
Herald added a subscriber: cfe-commits.
hvdijk requested review of this revision.

The GNU token paste extension that removes the comma in , ## __VA_ARGS__ 
conflicts with C99/C++11's requirements when a variadic macro has no named 
parameters: according to the standard, an invocation as FOO() gives it a single 
empty argument, and concatenation of anything with an empty argument is 
well-defined. For this reason, the GNU extension was already disabled in C99 
standard-conforming mode. It was not yet disabled in C++11 standard-conforming 
mode.

The associated comment suggested that GCC keeps this extension enabled in 
C90/C++03 standard-conforming mode, but it actually does not, so rather than 
adding a check for C++ language version, this change simply removes the check 
for C language version.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91913

Files:
  clang/lib/Lex/TokenLexer.cpp
  clang/test/Preprocessor/macro_fn_comma_swallow2.c


Index: clang/test/Preprocessor/macro_fn_comma_swallow2.c
===
--- clang/test/Preprocessor/macro_fn_comma_swallow2.c
+++ clang/test/Preprocessor/macro_fn_comma_swallow2.c
@@ -1,9 +1,12 @@
 // Test the __VA_ARGS__ comma swallowing extensions of various compiler 
dialects.
 
 // RUN: %clang_cc1 -E %s | FileCheck -check-prefix=GCC -strict-whitespace %s
+// RUN: %clang_cc1 -E -std=c90 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c99 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c11 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -x c++ %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++03 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++11 %s | FileCheck -check-prefix=C99 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -std=gnu99 %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -fms-compatibility %s | FileCheck -check-prefix=MS 
-strict-whitespace %s
 // RUN: %clang_cc1 -E -DNAMED %s | FileCheck -check-prefix=GCC 
-strict-whitespace %s
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -148,12 +148,11 @@
 return false;
 
   // GCC removes the comma in the expansion of " ... , ## __VA_ARGS__ " if
-  // __VA_ARGS__ is empty, but not in strict C99 mode where there are no
-  // named arguments, where it remains.  In all other modes, including C99
-  // with GNU extensions, it is removed regardless of named arguments.
+  // __VA_ARGS__ is empty, but not in strict mode where there are no
+  // named arguments, where it remains.  With GNU extensions, it is removed
+  // regardless of named arguments.
   // Microsoft also appears to support this extension, unofficially.
-  if (PP.getLangOpts().C99 && !PP.getLangOpts().GNUMode
-&& Macro->getNumParams() < 2)
+  if (!PP.getLangOpts().GNUMode && Macro->getNumParams() < 2)
 return false;
 
   // Is a comma available to be removed?


Index: clang/test/Preprocessor/macro_fn_comma_swallow2.c
===
--- clang/test/Preprocessor/macro_fn_comma_swallow2.c
+++ clang/test/Preprocessor/macro_fn_comma_swallow2.c
@@ -1,9 +1,12 @@
 // Test the __VA_ARGS__ comma swallowing extensions of various compiler dialects.
 
 // RUN: %clang_cc1 -E %s | FileCheck -check-prefix=GCC -strict-whitespace %s
+// RUN: %clang_cc1 -E -std=c90 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c99 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=c11 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 // RUN: %clang_cc1 -E -x c++ %s | FileCheck -check-prefix=GCC -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++03 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
+// RUN: %clang_cc1 -E -x c++ -std=c++11 %s | FileCheck -check-prefix=C99 -strict-whitespace %s
 // RUN: %clang_cc1 -E -std=gnu99 %s | FileCheck -check-prefix=GCC -strict-whitespace %s
 // RUN: %clang_cc1 -E -fms-compatibility %s | FileCheck -check-prefix=MS -strict-whitespace %s
 // RUN: %clang_cc1 -E -DNAMED %s | FileCheck -check-prefix=GCC -strict-whitespace %s
Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -148,12 +148,11 @@
 return false;
 
   // GCC removes the comma in the expansion of " ... , ## __VA_ARGS__ " if
-  // __VA_ARGS__ is empty, but not in strict C99 mode where there are no
-  // named arguments, where it remains.  In all other modes, including C99
-  // with GNU extensions, it is removed 

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

committed here: 
https://github.com/llvm/llvm-project/commit/9930d4dff31a130890f21a64f43d530a83ae3d0a

Thank you Aaron, Richard and Wyat!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[clang] 9930d4d - [NFC, Refactor] Modernize enum FunctionDefinitionKind (DeclSpech.h) into a scoped enum

[Faisal Vali via cfe-commits]

Author: Faisal Vali
Date: 2020-11-21T09:49:52-06:00
New Revision: 9930d4dff31a130890f21a64f43d530a83ae3d0a

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

LOG: [NFC, Refactor] Modernize enum FunctionDefinitionKind (DeclSpech.h) into a 
scoped enum

Reviewed by aaron.ballman, rsmith, wchilders
Highlights of review:
- avoid specifying an underlying type (unless such an enum is stored (or part 
of an abi?))
- avoid using enums as bit-fields, preferring unsigned bit-fields that we 
static_cast enumerators to. (MS's abi laysout enum bit-fields differently).
- clang-format, clang-format, clang-format.

https://reviews.llvm.org/D91035

Thank you!

Added: 


Modified: 
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseCXXInlineMethods.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/DeclSpec.h 
b/clang/include/clang/Sema/DeclSpec.h
index d2acafc2e4b3..afcbbaa5cfa7 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -1748,11 +1748,11 @@ class DecompositionDeclarator {
 
 /// Described the kind of function definition (if any) provided for
 /// a function.
-enum FunctionDefinitionKind {
-  FDK_Declaration,
-  FDK_Definition,
-  FDK_Defaulted,
-  FDK_Deleted
+enum class FunctionDefinitionKind {
+  Declaration,
+  Definition,
+  Defaulted,
+  Deleted
 };
 
 enum class DeclaratorContext {
@@ -1888,7 +1888,8 @@ class Declarator {
   Declarator(const DeclSpec , DeclaratorContext C)
   : DS(ds), Range(ds.getSourceRange()), Context(C),
 InvalidType(DS.getTypeSpecType() == DeclSpec::TST_error),
-GroupingParens(false), FunctionDefinition(FDK_Declaration),
+GroupingParens(false), FunctionDefinition(static_cast(
+   FunctionDefinitionKind::Declaration)),
 Redeclaration(false), Extension(false), ObjCIvar(false),
 ObjCWeakProperty(false), InlineStorageUsed(false),
 Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr),
@@ -2562,11 +2563,11 @@ class Declarator {
   void setEllipsisLoc(SourceLocation EL) { EllipsisLoc = EL; }
 
   void setFunctionDefinitionKind(FunctionDefinitionKind Val) {
-FunctionDefinition = Val;
+FunctionDefinition = static_cast(Val);
   }
 
   bool isFunctionDefinition() const {
-return getFunctionDefinitionKind() != FDK_Declaration;
+return getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration;
   }
 
   FunctionDefinitionKind getFunctionDefinitionKind() const {

diff  --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp 
b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 12941f214cbc..b0335905b6f8 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -108,7 +108,7 @@ NamedDecl *Parser::ParseCXXInlineMethodDef(
   // or if we are about to parse function member template then consume
   // the tokens and store them for parsing at the end of the translation unit.
   if (getLangOpts().DelayedTemplateParsing &&
-  D.getFunctionDefinitionKind() == FDK_Definition &&
+  D.getFunctionDefinitionKind() == FunctionDefinitionKind::Definition &&
   !D.getDeclSpec().hasConstexprSpecifier() &&
   !(FnD && FnD->getAsFunction() &&
 FnD->getAsFunction()->getReturnType()->getContainedAutoType()) &&

diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp 
b/clang/lib/Parse/ParseDeclCXX.cpp
index 0a810fc393a6..9525c0222b9f 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -1045,8 +1045,16 @@ void Parser::AnnotateExistingDecltypeSpecifier(const 
DeclSpec& DS,
SourceLocation StartLoc,
SourceLocation EndLoc) {
   // make sure we have a token we can turn into an annotation token
-  if (PP.isBacktrackEnabled())
+  if (PP.isBacktrackEnabled()) {
 PP.RevertCachedTokens(1);
+if (DS.getTypeSpecType() == TST_error) {
+  // We encountered an error in parsing 'decltype(...)' so lets annotate 
all
+  // the tokens in the backtracking cache - that we likely had to skip over
+  // to get to a token that allows us to resume parsing, such as a
+  // semi-colon.
+  EndLoc = PP.getLastCachedTokenLocation();
+}
+  }
   else
 PP.EnterToken(Tok, /*IsReinject*/true);
 
@@ -2707,23 +2715,23 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier 
AS,
 if (getLangOpts().MicrosoftExt && DeclaratorInfo.isDeclarationOfFunction())
   TryConsumePureSpecifier(/*AllowDefinition*/ true);
 
-FunctionDefinitionKind 

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please add test case for new command-line option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91908

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

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.h:109
+/// Get call hierarchy information at \p Pos.
+llvm::Optional>
+prepareCallHierarchy(ParsedAST , Position Pos, const SymbolIndex *Index,

nridge wrote:
> kadircet wrote:
> > nridge wrote:
> > > kadircet wrote:
> > > > what's the distinction between empty and none return values ? (same for 
> > > > others)
> > > Generally speaking, a `None` result means "the input was not recognized 
> > > in some way", while an empty vector result means "there are no results 
> > > for this input".
> > > 
> > > For `prepareCallHierarchy`, the inputs encode a source location, and the 
> > > operation asks "give me `CallHierarchyItem`s for suitable declarations 
> > > (i.e. functions) at this location. So, `None` means "the source location 
> > > could not be recognized" (`sourceLocationInMainFile` failed), while an 
> > > empty result means "there are no declarations of functions at this 
> > > location".
> > > 
> > > For `incomingCalls` and `outgoingCalls`, the inputs encode a declaration 
> > > of a function, and the operation asks "give me its callers / callees". 
> > > So, a `None` means "could not locate the function (i.e. symbol)", while 
> > > an empty result means "this function has no callers / callees".
> > > Generally speaking, a None result means "the input was not recognized in 
> > > some way", while an empty vector result means "there are no results for 
> > > this input".
> > 
> > Makes sense, but that sounds like an implementation detail. I don't see any 
> > difference between the two from caller's point of view. Even the logging 
> > happens inside the implementation. As for interpreting llvm::None by the 
> > caller, i believe it is subject to change, so unless we return an Expected, 
> > there's not much value in returning an Optional, and even in such a case, 
> > caller probably can't do much but propagate the error (maybe also try with 
> > Pos+/-1, but usually that's handled within the XRefs as well).
> > 
> > Returning an empty container is also coherent with rest of the APIs we have 
> > in XRefs.h.
> > 
> > So I believe we should drop the optionals here.
> The protocol 
> [specifies](https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textDocument_prepareCallHierarchy)
>  return types of `CallHierarchyItem[] | null` for `prepareCallHierarchy` and 
> `CallHierarchyIncomingCall[] | null` for `incomingCalls`.
> 
> The `| null` in there suggests that the protocol intends for there to be a 
> semantic difference  between an empty array and null, and that clients may 
> want to do things differently in the two caes (e.g. show an "unable to 
> compute call hierarchy" error dialog vs. show an emty tree).
yes, that's indeed a possibility, and the same goes for other features like 
textDocument/{definition,declaration,references}. AFAICT, we don't signal the 
difference between "no results" and "something went wrong" for those, and 
haven't received any complaints yet. So I'd rather keep them coherent, and 
replace all of them if we see that there are clients taking different actions 
for real.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D90751: [clangd] Use ProjectAwareIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet abandoned this revision.
kadircet added a comment.

Merged with https://reviews.llvm.org/D91860


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90751

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

[PATCH] D91860: [clangd] Move remote-index dependency from clangDaemon to ClangdMain

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 306840.
kadircet added a comment.

- Merge with https://reviews.llvm.org/D90751


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91860

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -13,6 +13,9 @@
 #include "Protocol.h"
 #include "Transport.h"
 #include "index/Background.h"
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/ProjectAware.h"
 #include "index/Serialization.h"
 #include "index/remote/Client.h"
 #include "refactor/Rename.h"
@@ -40,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifndef _WIN32
 #include 
@@ -551,6 +555,27 @@
 const char TestScheme::TestDir[] = "/clangd-test";
 #endif
 
+std::unique_ptr
+loadExternalIndex(const Config::ExternalIndexSpec ,
+  AsyncTaskRunner ) {
+  switch (External.Kind) {
+  case Config::ExternalIndexSpec::Server:
+log("Associating {0} with remote index at {1}.", External.MountPoint,
+External.Location);
+return remote::getClient(External.Location, External.MountPoint);
+  case Config::ExternalIndexSpec::File:
+log("Associating {0} with monolithic index at {1}.", External.MountPoint,
+External.Location);
+auto NewIndex = std::make_unique(std::make_unique());
+Tasks.runAsync("Load-index:" + External.Location,
+   [File = External.Location, PlaceHolder = NewIndex.get()] {
+ if (auto Idx = loadIndex(File, /*UseDex=*/true))
+   PlaceHolder->reset(std::move(Idx));
+   });
+return std::move(NewIndex);
+  }
+  llvm_unreachable("Invalid ExternalIndexKind.");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
@@ -726,6 +751,7 @@
 Opts.ResourceDir = ResourceDir;
   Opts.BuildDynamicSymbolIndex = EnableIndex;
   Opts.CollectMainFileRefs = CollectMainFileRefs;
+  std::vector> IdxStack;
   std::unique_ptr StaticIdx;
   std::future AsyncIndexLoad; // Block exit while loading the index.
   if (EnableIndex && !IndexFile.empty()) {
@@ -757,7 +783,15 @@
   }
 #endif
   Opts.BackgroundIndex = EnableBackgroundIndex;
-  Opts.StaticIndex = StaticIdx.get();
+  auto PAI = createProjectAwareIndex(loadExternalIndex);
+  if (StaticIdx) {
+IdxStack.emplace_back(std::move(StaticIdx));
+IdxStack.emplace_back(
+std::make_unique(PAI.get(), IdxStack.back().get()));
+Opts.StaticIndex = IdxStack.back().get();
+  } else {
+Opts.StaticIndex = PAI.get();
+  }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
Index: clang-tools-extra/clangd/index/ProjectAware.h
===
--- clang-tools-extra/clangd/index/ProjectAware.h
+++ clang-tools-extra/clangd/index/ProjectAware.h
@@ -23,11 +23,11 @@
 using IndexFactory = std::function(
 const Config::ExternalIndexSpec &, AsyncTaskRunner &)>;
 
-/// Returns an index that answers queries using external indices. IndexGenerator
-/// can be used to customize how to generate an index from an external source.
-/// The default implementation loads the index asynchronously on the
-/// AsyncTaskRunner. The index will appear empty until loaded.
-std::unique_ptr createProjectAwareIndex(IndexFactory = nullptr);
+/// Returns an index that answers queries using external indices. IndexFactory
+/// specifies how to generate an index from an external source.
+/// IndexFactory must be injected because this code cannot depend on the remote
+/// index client.
+std::unique_ptr createProjectAwareIndex(IndexFactory);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/index/ProjectAware.cpp
===
--- clang-tools-extra/clangd/index/ProjectAware.cpp
+++ clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -15,7 +15,6 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
-#include "index/remote/Client.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
 #include "support/Trace.h"
@@ -124,33 +123,11 @@
 Entry.first->getSecond() = Gen(External, Tasks);
   return Entry.first->second.get();
 }
-
-std::unique_ptr
-defaultFactory(const Config::ExternalIndexSpec ,
-   AsyncTaskRunner ) {
-  switch (External.Kind) {
-  case Config::ExternalIndexSpec::Server:
-log("Associating {0} with remote index at {1}.", External.MountPoint,
-External.Location);
-return remote::getClient(External.Location, 

[PATCH] D91860: [clangd] Move remote-index dependency from clangDaemon to ClangdMain

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 306839.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91860

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -555,6 +555,27 @@
 const char TestScheme::TestDir[] = "/clangd-test";
 #endif
 
+std::unique_ptr
+loadExternalIndex(const Config::ExternalIndexSpec ,
+  AsyncTaskRunner ) {
+  switch (External.Kind) {
+  case Config::ExternalIndexSpec::Server:
+log("Associating {0} with remote index at {1}.", External.MountPoint,
+External.Location);
+return remote::getClient(External.Location, External.MountPoint);
+  case Config::ExternalIndexSpec::File:
+log("Associating {0} with monolithic index at {1}.", External.MountPoint,
+External.Location);
+auto NewIndex = std::make_unique(std::make_unique());
+Tasks.runAsync("Load-index:" + External.Location,
+   [File = External.Location, PlaceHolder = NewIndex.get()] {
+ if (auto Idx = loadIndex(File, /*UseDex=*/true))
+   PlaceHolder->reset(std::move(Idx));
+   });
+return std::move(NewIndex);
+  }
+  llvm_unreachable("Invalid ExternalIndexKind.");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
@@ -762,7 +783,7 @@
   }
 #endif
   Opts.BackgroundIndex = EnableBackgroundIndex;
-  auto PAI = createProjectAwareIndex();
+  auto PAI = createProjectAwareIndex(loadExternalIndex);
   if (StaticIdx) {
 IdxStack.emplace_back(std::move(StaticIdx));
 IdxStack.emplace_back(
Index: clang-tools-extra/clangd/index/ProjectAware.h
===
--- clang-tools-extra/clangd/index/ProjectAware.h
+++ clang-tools-extra/clangd/index/ProjectAware.h
@@ -23,11 +23,11 @@
 using IndexFactory = std::function(
 const Config::ExternalIndexSpec &, AsyncTaskRunner &)>;
 
-/// Returns an index that answers queries using external indices. IndexGenerator
-/// can be used to customize how to generate an index from an external source.
-/// The default implementation loads the index asynchronously on the
-/// AsyncTaskRunner. The index will appear empty until loaded.
-std::unique_ptr createProjectAwareIndex(IndexFactory = nullptr);
+/// Returns an index that answers queries using external indices. IndexFactory
+/// specifies how to generate an index from an external source.
+/// IndexFactory must be injected because this code cannot depend on the remote
+/// index client.
+std::unique_ptr createProjectAwareIndex(IndexFactory);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/index/ProjectAware.cpp
===
--- clang-tools-extra/clangd/index/ProjectAware.cpp
+++ clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -15,7 +15,6 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
-#include "index/remote/Client.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
 #include "support/Trace.h"
@@ -124,33 +123,11 @@
 Entry.first->getSecond() = Gen(External, Tasks);
   return Entry.first->second.get();
 }
-
-std::unique_ptr
-defaultFactory(const Config::ExternalIndexSpec ,
-   AsyncTaskRunner ) {
-  switch (External.Kind) {
-  case Config::ExternalIndexSpec::Server:
-log("Associating {0} with remote index at {1}.", External.MountPoint,
-External.Location);
-return remote::getClient(External.Location, External.MountPoint);
-  case Config::ExternalIndexSpec::File:
-log("Associating {0} with monolithic index at {1}.", External.MountPoint,
-External.Location);
-auto NewIndex = std::make_unique(std::make_unique());
-Tasks.runAsync("Load-index:" + External.Location,
-   [File = External.Location, PlaceHolder = NewIndex.get()] {
- if (auto Idx = loadIndex(File, /*UseDex=*/true))
-   PlaceHolder->reset(std::move(Idx));
-   });
-return std::move(NewIndex);
-  }
-  llvm_unreachable("Invalid ExternalIndexKind.");
-}
 } // namespace
 
 std::unique_ptr createProjectAwareIndex(IndexFactory Gen) {
-  return std::make_unique(Gen ? std::move(Gen)
- : defaultFactory);
+  assert(Gen);
+  return std::make_unique(std::move(Gen));
 }
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/CMakeLists.txt

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

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

xbolva00 added a comment.

In that code there is a comment:

>> These flags are required for GCC 6 builds as undefined behaviour in OpenJDK 
>> code

So you should really fix UB...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17993

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added a comment.

In D17993#2409401 , @brooksmoses wrote:

> So, I have bad news: This causes OpenJDK to segfault.  The relevant code is 
> here:
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/arena.cpp#L311
>
>   void Arena::destruct_contents() {
> if (UseMallocOnly && _first != NULL) {
>   char* end = _first->next() ? _first->top() : _hwm;
>   free_malloced_objects(_first, _first->bottom(), end, _hwm);
> }
> // reset size before chop to avoid a rare racing condition
> // that can have total arena memory exceed total chunk memory
> set_size_in_bytes(0);
> _first->chop();
> reset();
>   }
>
> I've also seen a segfault in Verilator that root-causes to this patch, though 
> I haven't yet tracked that down to the source code.
>
> I hate to say it, but is this a significant enough problem to call for a 
> (temporary, I hope) rollback?

I don't see why this would be enough for a rollback, jdk is supposed to build 
with `-fno-delete-null-pointer-checks`, which disables this optimization:
https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L842

Is the build system not setting this when using Clang?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17993

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

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:695
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}

nridge wrote:
> kadircet wrote:
> > why do we run this on the mainthread ? I suppose we should just do 
> > `WorkScheduler.run`
> I was following what `resolveTypeHierarchy` did. Should I change that too?
yes, i think we should. preferably on a different patch though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91123

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

[PATCH] D91124: [clangd] Call hierarchy (ClangdLSPServer layer)

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91124

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

[PATCH] D91868: [clangd] Mention when CXXThis is implicit in exposed AST.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/DumpAST.cpp:233
   return CCO->getConstructor()->getNameAsString();
+if (const auto *CTE = dyn_cast(S)) {
+  bool Const = CTE->getType()->getPointeeType().isLocalConstQualified();

sammccall wrote:
> kadircet wrote:
> > should we ensure we always return within the branch (as we do within the 
> > rest of the branches to make sure we don't accumulate details by mistake)? 
> > e.g:
> > ```
> > if(CXXThisExpr) {
> >   details = {}
> >   if (const) details += "const";
> >   if (implicit) details += "implicit";
> >   return join(",", details);
> > }
> > ```
> Actually I was trying to *avoid* returning `""` in several places.
> I think of these functions as a collection of special cases where we have 
> something to say - if we don't find anything, we fall off the end.
> (`getDetail` for DeducedType is the other example)
> 
> Can change it if you think it's important though.
> Actually I was trying to *avoid* returning "" in several places.

I agree that this is bad, but it feels acceptable when the code doesn't read 
that way directly :P

> I think of these functions as a collection of special cases where we have 
> something to say - if we don't find anything, we fall off the end.

I was afraid of entering other branches, before falling off the end (not 
applicable here, but assume one day we have a children of `CXXThisExpr`, this 
code will prefer the children when thisexpr is neither const nor implicit, 
which might be surprising).

> Can change it if you think it's important though.

As mentioned not an immediate threat, but might become surprising in the future 
and require some digging. So would be nice if you can.



Comment at: clang-tools-extra/clangd/unittests/DumpASTTests.cpp:79
   {R"cpp(
-template  int root() {
-  (void)root();
+namespace root {
+template  int tmpl() {

sammccall wrote:
> kadircet wrote:
> > is this change intentional ?
> Yeah, the problem is that I wanted the root in the new test to be a member 
> function, so I needed to switch from findDecl("root") to 
> findUnqualifiedDecl("root"). But that fails if there are two decls with that 
> name, as there were here: the primary template root and the specialization 
> root. Thus the change to wrap the whole thing in a root namespace. 
> Maybe there's a neater way to solve this, I'm not sure it matters much.
ah makes sense. sorry i missed the change from finddecl to unqualifieddecl 
within the test body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91868

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt


Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -143,7 +143,8 @@
 if (CLANGD_ENABLE_REMOTE)
   target_link_libraries(ClangdTests
 PRIVATE
-clangdRemoteMarshalling)
+clangdRemoteMarshalling
+RemoteIndexProto)
 endif()
 
 if (CLANGD_BUILD_XPC)
Index: clang-tools-extra/clangd/index/remote/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/CMakeLists.txt
@@ -3,6 +3,12 @@
   generate_protos(RemoteIndexServiceProto "Service.proto"
 DEPENDS "Index.proto"
 GRPC)
+  # FIXME: Move this into generate_protos. Currently we only mention proto
+  # filename as a dependency, but linking requires target name.
+  target_link_libraries(RemoteIndexServiceProto
+PRIVATE
+RemoteIndexProto
+)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../)
 
@@ -20,6 +26,8 @@
 clangdRemoteMarshalling
 protobuf
 grpc++
+clangBasic
+clangDaemon
 clangdSupport
 
 DEPENDS
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -167,17 +167,18 @@
   add_subdirectory(xpc)
 endif ()
 
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+
 if(CLANG_INCLUDE_TESTS)
-add_subdirectory(test)
-add_subdirectory(unittests)
+  add_subdirectory(test)
+  add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is 
stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support 
for Clangd" OFF)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual 
installation.")
 
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
 add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)


Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -143,7 +143,8 @@
 if (CLANGD_ENABLE_REMOTE)
   target_link_libraries(ClangdTests
 PRIVATE
-clangdRemoteMarshalling)
+clangdRemoteMarshalling
+RemoteIndexProto)
 endif()
 
 if (CLANGD_BUILD_XPC)
Index: clang-tools-extra/clangd/index/remote/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/CMakeLists.txt
@@ -3,6 +3,12 @@
   generate_protos(RemoteIndexServiceProto "Service.proto"
 DEPENDS "Index.proto"
 GRPC)
+  # FIXME: Move this into generate_protos. Currently we only mention proto
+  # filename as a dependency, but linking requires target name.
+  target_link_libraries(RemoteIndexServiceProto
+PRIVATE
+RemoteIndexProto
+)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../)
 
@@ -20,6 +26,8 @@
 clangdRemoteMarshalling
 protobuf
 grpc++
+clangBasic
+clangDaemon
 clangdSupport
 
 DEPENDS
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -167,17 +167,18 @@
   add_subdirectory(xpc)
 endif ()
 
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+
 if(CLANG_INCLUDE_TESTS)
-add_subdirectory(test)
-add_subdirectory(unittests)
+  add_subdirectory(test)
+  add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
 
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
 add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a subscriber: nridge.
kadircet added a comment.

> I don't think that's how CMake works, the whole CMakeLists tree is parsed 
> before anything is compiled, so it shouldn't race like that.

That was also my mental model, but it looks like `include_directories` 
statement within the `FindGRPC` submodule only affects targets created 
afterwards (lexicographically). Even though cmake docs say that it should 
affect everything within that cmakelists file 
(https://cmake.org/cmake/help/latest/command/include_directories.html).

I've tried to migrate into target_include_directories instead(i.e. only 
introduce the include header to targets generated via `generate_protos` and 
propagate into their dependents), and it works great with targets introduced 
via `add_clang_executable` or `add_unittest`, but for some reason I could not 
figure out; propagation doesn't work for targets introduced via 
`add_clang_library` :(.

And indeed my compile commands also didn't contain the necessary `-isystem`, 
but I had an acceptable version of protobuf installed in /usr/include even 
though i tried really hard to delete all the remaining packages, so it was 
working on my machine :/

Hopefully it should be good now. It is quite nice that @nridge has also noticed 
this while we are working on a fix, so I hope he can try this patch on his 
setup as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

(also please add "Fixes https://github.com/clangd/clangd/issues/598; to the 
patch description to close the related issue on GH)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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