[PATCH] D144321: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

[Brad Smith via Phabricator via cfe-commits]

brad updated this revision to Diff 498748.
Herald added a subscriber: MaskRay.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144321

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ppc-abi.c
  llvm/include/llvm/TargetParser/Triple.h
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
  llvm/test/CodeGen/PowerPC/pr47373.ll

Index: llvm/test/CodeGen/PowerPC/pr47373.ll
===
--- llvm/test/CodeGen/PowerPC/pr47373.ll
+++ llvm/test/CodeGen/PowerPC/pr47373.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=powerpc64-unknown-freebsd13.0 -verify-machineinstrs \
+; RUN: llc -mtriple=powerpc64-unknown-freebsd12.0 -verify-machineinstrs \
 ; RUN:   -mcpu=ppc64 -ppc-asm-full-reg-names < %s | FileCheck %s
 @a = local_unnamed_addr global ptr null, align 8
 
Index: llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
===
--- llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
+++ llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
@@ -5,9 +5,14 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
-; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-linux-musl < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-openbsd < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
 ; CHECK-ELFv2: .abiversion 2
 ; CHECK-ELFv1-NOT: .abiversion 2
Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -237,7 +237,10 @@
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
   case Triple::ppc64:
-return PPCTargetMachine::PPC_ABI_ELFv1;
+if (TT.isPPC64ELFv2ABI())
+  return PPCTargetMachine::PPC_ABI_ELFv2;
+else
+  return PPCTargetMachine::PPC_ABI_ELFv1;
   default:
 return PPCTargetMachine::PPC_ABI_UNKNOWN;
   }
Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -882,6 +882,14 @@
 return getArch() == Triple::ppc64 || getArch() == Triple::ppc64le;
   }
 
+  /// Tests whether the target 64-bit PowerPC big endian ABI is ELFv2.
+  bool isPPC64ELFv2ABI() const {
+return (getArch() == Triple::ppc64 &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::OpenBSD || isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 
Index: clang/test/Driver/ppc-abi.c
===
--- clang/test/Driver/ppc-abi.c
+++ clang/test/Driver/ppc-abi.c
@@ -20,6 +20,7 @@
 // RUN: %clang -target powerpc64-unknown-freebsd12 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64-unknown-freebsd13 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64-unknown-freebsd14 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE %s
+// RUN: %clang -target powerpc64-unknown-freebsd %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE-PIE %s
 // RUN: %clang -target powerpc64-linux-musl %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE-PIE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2006,8 +2006,7 @@
   if (T.isOSBinFormatELF()) {
   

[PATCH] D143725: [llvm-objdump][ARM] support --symbolize-operands for ARM/ELF

[James Henderson via Phabricator via cfe-commits]

jhenderson added inline comments.



Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1145
+  // So far only supports ARM/Thumb, PowerPC and X86.
+  Triple triple = STI->getTargetTriple();
+  if (!triple.isPPC() && !triple.isX86() && !triple.isARM() &&

covanam wrote:
> Esme wrote:
> > Please capitalize the first letter of variable names.
> Sorry I am newcomer. clang-format didn't say anything, so I thought this is 
> okay.
> Will be fixed. Same for the other one.
clang-format is only for formatting. It won't do things like variable name 
styles. You may want to look into clang-tidy which does a lot more. Also, make 
sure you've fully read and digested the [[ 
https://llvm.org/docs/CodingStandards.html | LLVM coding standards ]].

FWIW, here you can't simply do `triple` -> `Triple`, since `Triple` is a type. 
You probably can use `TargetTriple` or something along those lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143725

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

[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D138037#4134504 , @steakhal wrote:

> I'm backporting this as #60834 
>  to the 16.x branch.

Backported with 1ab37a25e463ea144289a1cbcf32a7659b2d60bb 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138037

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

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

[Juan Manuel Martinez Caamaño via Phabricator via cfe-commits]

jmmartinez added inline comments.



Comment at: llvm/lib/IR/DIBuilder.cpp:789
+  return createLocalVariable(
+  VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name,
+  /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve, Flags, AlignInBits);

I'll be tempted to move the call to `getSubprogramNodesTrackingVector(Scope)` 
into `createLocalVariable`. It seems that every time `createLocalVariable` is 
called, `getSubprogramNodesTrackingVector(Scope)` is passed as value for 
`PreservedNodes`.

(I've just started reviewing so I may be missing some other modifications)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143984

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

[PATCH] D144334: [Clang] Add C++2b attribute [[assume(expression)]]

[Nikita Popov via Phabricator via cfe-commits]

nikic resigned from this revision.
nikic added a comment.

(not a clang reviewer)




Comment at: clang/test/CodeGenCXX/cxx2b-assume.cpp:55
+// CHECK-OPT-NEXT:   tail call void @llvm.assume(i1 %[[CMP]])
+// CHECK-OPT-NEXT:   ret i32 43
+//

Isn't the assume expression supposed to be unevaluated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144334

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

[PATCH] D144321: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

[Dimitry Andric via Phabricator via cfe-commits]

dim accepted this revision.
dim added a comment.

Yeah, this looks quite a bit nicer, and should be more maintainable. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144321

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

[PATCH] D144244: [X86]rearange X86InstrInfo.td

[Wang, Xin via Phabricator via cfe-commits]

XinWang10 added a comment.

In D144244#4134569 , @RKSimon wrote:

> LGTM - although it might be nice to pull out more ISAs from X86InstrMisc into 
> their own files (X86InstrTBM, etc.) as its always annoying trying to find 
> their defs.

Many instructions are too small size, like UINTR..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144244

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

[PATCH] D144244: [X86]rearange X86InstrInfo.td

[Wang, Xin via Phabricator via cfe-commits]

XinWang10 added a comment.

Thanks for your reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144244

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

[PATCH] D144371: [LLVM-COV] Fix an issue: a statement after calling 'assert()' function is wrongly marked as 'not executed'

[Ying Yi via Phabricator via cfe-commits]

MaggieYi created this revision.
MaggieYi added reviewers: vsk, zequanwu.
MaggieYi added a project: clang.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a subscriber: cfe-commits.

In the current coverage mapping implementation, we terminate the current region 
and start a zero region when we hit a nonreturn function. However, for logical 
OR, the second operand is not executed if the first operand evaluates to true. 
If the nonreturn function is called in the right side of logical OR and the 
left side of logical OR is TRUE, we should not start a zero `GapRegionCounter`. 
This will also apply to `VisitAbstractConditionalOperator`.

The following issues are fixed by this patch:

1. https://github.com/llvm/llvm-project/issues/59030
2. https://github.com/llvm/llvm-project/issues/57388
3. https://github.com/llvm/llvm-project/issues/57481
4. https://github.com/llvm/llvm-project/issues/60701


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144371

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/terminate-statements.cpp

Index: clang/test/CoverageMapping/terminate-statements.cpp
===
--- clang/test/CoverageMapping/terminate-statements.cpp
+++ clang/test/CoverageMapping/terminate-statements.cpp
@@ -320,6 +320,30 @@
   included_func();
 }
 
+// CHECK-LABEL: _Z7ornoretv:
+void abort() __attribute__((noreturn));
+
+int ornoret(void) {
+  ( true || (abort(), 0) );  // CHECK: Gap,File 0, [[@LINE]]:28 -> [[@LINE+1]]:3 = #0
+  ( false || (abort(), 0) ); // CHECK: Gap,File 0, [[@LINE]]:29 -> [[@LINE+1]]:3 = 0
+  return 0;
+}
+
+// CHECK-LABEL: _Z17abstractcondnoretv:
+int abstractcondnoret(void) {
+  ( true ? void (0) : abort() );  // CHECK: Gap,File 0, [[@LINE]]:33 -> [[@LINE+1]]:3 = #1
+  ( false ? void (0) : abort() ); // CHECK: Gap,File 0, [[@LINE]]:34 -> [[@LINE+1]]:3 = #2
+  return 0;
+}
+
+// CHECK-LABEL: _Z13elsecondnoretv:
+int elsecondnoret(void) {
+  if (true) {} else {
+true ? void (0) : abort();
+  } // CHECK: Gap,File 0, [[@LINE]]:4 -> [[@LINE+1]]:3 = (#1 + #2)
+  return 0;
+}
+
 int main() {
   foo(0);
   foo(1);
@@ -339,5 +363,8 @@
   while_loop();
   gotos();
   include();
+  ornoret();
+  abstractcondnoret();
+  elsecondnoret();
   return 0;
 }
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1466,6 +1466,7 @@
 Counter TrueCount = getRegionCounter(E);
 
 propagateCounts(ParentCount, E->getCond());
+Counter OutCount;
 
 if (!isa(E)) {
   // The 'then' count applies to the area immediately after the condition.
@@ -1475,12 +1476,18 @@
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), TrueCount);
 
   extendRegion(E->getTrueExpr());
-  propagateCounts(TrueCount, E->getTrueExpr());
+  OutCount = propagateCounts(TrueCount, E->getTrueExpr());
 }
 
 extendRegion(E->getFalseExpr());
-propagateCounts(subtractCounters(ParentCount, TrueCount),
-E->getFalseExpr());
+OutCount = addCounters(
+OutCount, propagateCounts(subtractCounters(ParentCount, TrueCount),
+  E->getFalseExpr()));
+
+if (OutCount != ParentCount) {
+  pushRegion(OutCount);
+  GapRegionCounter = OutCount;
+}
 
 // Create Branch Region around condition.
 createBranchRegion(E->getCond(), TrueCount,
@@ -1514,9 +1521,19 @@
subtractCounters(RHSExecCnt, RHSTrueCnt));
   }
 
+  // Determine whether the right side of OR operation need to be visited.
+  bool shouldVisitRHS(const Expr *LHS) {
+bool LHSIsTrue = false;
+bool LHSIsConst = false;
+if (!LHS->isValueDependent())
+  LHSIsConst = LHS->EvaluateAsBooleanCondition(
+  LHSIsTrue, CVM.getCodeGenModule().getContext());
+return !LHSIsConst || (LHSIsConst && !LHSIsTrue);
+  }
+
   void VisitBinLOr(const BinaryOperator *E) {
 extendRegion(E->getLHS());
-propagateCounts(getRegion().getCounter(), E->getLHS());
+Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS());
 handleFileExit(getEnd(E->getLHS()));
 
 // Counter tracks the right hand side of a logical or operator.
@@ -1529,6 +1546,10 @@
 // Extract the RHS's "False" Instance Counter.
 Counter RHSFalseCnt = getRegionCounter(E->getRHS());
 
+if (!shouldVisitRHS(E->getLHS())) {
+  GapRegionCounter = OutCount;
+}
+
 // Extract the Parent Region Counter.
 Counter ParentCnt = getRegion().getCounter();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

This is an impressive amount of work. I think it makes sense!
Thanks a lot for doing that work.
I only have a few nits after a first review of this.




Comment at: clang/include/clang/Lex/Lexer.h:89
 
-  // End of the buffer.
-  const char *BufferEnd;
+  // Size of the buffer.
+  unsigned BufferSize;

Should that use `SourceLocation::UIntTy`?
Looking at comments in SourceManager, I think there was an attempt at 
supporting > 2GB file but I don't think it got anywhere.
Nevertheless, using `SourceLocation::UIntTy` would arguably be more consistent

It does seem to be a huge undertaking to change it though, I'm not sure it 
would be worth it at all. There would be far bigger issues with ridiculously 
large source files anyway.



Comment at: clang/include/clang/Lex/Lexer.h:609
 
-  bool CheckUnicodeWhitespace(Token &Result, uint32_t C, const char *CurPtr);
+  bool CheckUnicodeWhitespace(Token &Result, uint32_t C, unsigned CurOffset);
 

davrec wrote:
> FWIW it sucks that `uint32_t` is already sprinkled throughout the interface 
> alongside `unsigned`, wish just one was used consistently, but that does not 
> need to be addressed in this patch.
here, `uint32_t` is a codepoint, should arguably be `char32_t` instead. but 
agreed, not in this patch.



Comment at: clang/lib/Lex/Lexer.cpp:213
+  L->BufferSize = L->BufferOffset + TokLen;
+  assert(L->BufferStart[L->BufferSize] == 0 && "Buffer is not nul 
terminated!");
 





Comment at: clang/lib/Lex/Lexer.cpp:1203
 if (L && !L->isLexingRawMode())
-  L->Diag(CP-2, diag::trigraph_ignored);
+  L->Diag(CP - 2 - L->getBuffer().data(), diag::trigraph_ignored);
 return 0;

sunho wrote:
> shafik wrote:
> > I wonder do we really need to do these pointer gymnastics, maybe making 
> > this a member function would eliminate the need for it.
> Yes, we can change it to offset here.
Agreed, that would be nice! (In all places `getBuffer().data()` is used)



Comment at: clang/lib/Lex/Lexer.cpp:1353
+if (unsigned EscapedNewLineSize =
+getEscapedNewLineSize(&BufferStart[Offset])) {
   // Remember that this token needs to be cleaned.





Comment at: clang/lib/Lex/Lexer.cpp:1378
 // a trigraph warning.  If so, and if trigraphs are enabled, return it.
-if (char C = DecodeTrigraphChar(Ptr + 2, Tok ? this : nullptr,
-LangOpts.Trigraphs)) {
+if (char C = DecodeTrigraphChar(&BufferStart[Offset + 2],
+Tok ? this : nullptr, LangOpts.Trigraphs)) 
{





Comment at: clang/lib/Lex/Lexer.cpp:1740
+bool Lexer::tryConsumeIdentifierUTF8Char(unsigned &CurOffset) {
+  const char *UnicodePtr = &BufferStart[CurOffset];
   llvm::UTF32 CodePoint;





Comment at: clang/lib/Lex/Lexer.cpp:1744
   llvm::convertUTF8Sequence((const llvm::UTF8 **)&UnicodePtr,
-(const llvm::UTF8 *)BufferEnd,
-&CodePoint,
-llvm::strictConversion);
+(const llvm::UTF8 *)&BufferStart[BufferSize],
+&CodePoint, llvm::strictConversion);

Ditto in all similar places, I think it reads easier


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143142

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

[PATCH] D144196: [C2x] Remove the ATOMIC_VAR_INIT macro from stdatomic.h

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:80
 
+- Removed the ``ATOMIC_VAR_INIT`` macro in C2x and later standards modes, which
+  implements `WG14 N2886 
`_

Just confirming we want to pluralize standards here? It kind of look weird to 
my non native eyes.



Comment at: clang/lib/Headers/stdatomic.h:50
+   in C2x mode; switch to the correct values once they've been published. */
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ < 202000L) ||   
\
+defined(__cplusplus)

C++ uses the date of the meeting where the change was accepted, I assume C is 
different?



Comment at: clang/lib/Headers/stdatomic.h:61
 /* ATOMIC_VAR_INIT was deprecated in C17 and C++20. */
 #pragma clang deprecated(ATOMIC_VAR_INIT)
 #endif

Should we add a message informing people it's remove in C23?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144196

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

[PATCH] D144334: [Clang] Add C++2b attribute [[assume(expression)]]

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Can you add a test to check `__has_cpp_attribute(assume)` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144334

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

[PATCH] D144334: [Clang] Add C++2b attribute [[assume(expression)]]

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:723-727
+case attr::Assume: {
+  llvm::Value *ArgValue = EmitScalarExpr(cast(A)->getCond());
+  llvm::Function *FnAssume = CGM.getIntrinsic(llvm::Intrinsic::assume);
+  Builder.CreateCall(FnAssume, ArgValue);
+  break;

I'm not familiar with codegen. what makes the expression non-evaluated here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144334

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

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/include/clang/Lex/Lexer.h:89
 
-  // End of the buffer.
-  const char *BufferEnd;
+  // Size of the buffer.
+  unsigned BufferSize;

cor3ntin wrote:
> Should that use `SourceLocation::UIntTy`?
> Looking at comments in SourceManager, I think there was an attempt at 
> supporting > 2GB file but I don't think it got anywhere.
> Nevertheless, using `SourceLocation::UIntTy` would arguably be more consistent
> 
> It does seem to be a huge undertaking to change it though, I'm not sure it 
> would be worth it at all. There would be far bigger issues with ridiculously 
> large source files anyway.
I am a bit afraid that unsigned has different sizes on different platforms. At 
least a `using BufferOffsetType = uint64_t;` would be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143142

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

[PATCH] D143840: [clang] Add the check of membership for the issue #58674 and improve the lookup process

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Thanks for working on this issue.
I don't feel confident commenting on this patch's approach (I think @erichkeane 
is your best bet), but the small typo does make a pretty big difference here!




Comment at: clang/include/clang/AST/DeclCXX.h:1549
   ///
+  /// \param LookupIndependent whether look up independent types.
+  ///





Comment at: clang/include/clang/AST/DeclCXX.h:1553
+  bool isDerivedFrom(const CXXRecordDecl *Base,
+ bool LookupIndependent = false) const;
 

Ditto in the rest of the patch


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

https://reviews.llvm.org/D143840

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

[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp updated this revision to Diff 498795.
dkrupp added a comment.

Added documentation to the newly introduced types: TaintData, TaintBugReport.


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

https://reviews.llvm.org/D144269

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Taint.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/TaintBugType.h
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  clang/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
  clang/test/Analysis/taint-checker-callback-order-has-definition.c
  clang/test/Analysis/taint-checker-callback-order-without-definition.c
  clang/test/Analysis/taint-diagnostic-visitor.c
  clang/test/Analysis/taint-dumps.c

Index: clang/test/Analysis/taint-dumps.c
===
--- clang/test/Analysis/taint-dumps.c
+++ clang/test/Analysis/taint-dumps.c
@@ -6,7 +6,7 @@
 int getchar(void);
 
 // CHECK: Tainted symbols:
-// CHECK-NEXT: conj_$2{{.*}} : 0
+// CHECK-NEXT: conj_$2{{.*}} : Type:0 Flow:0
 int test_taint_dumps(void) {
   int x = getchar();
   clang_analyzer_printState();
Index: clang/test/Analysis/taint-diagnostic-visitor.c
===
--- clang/test/Analysis/taint-diagnostic-visitor.c
+++ clang/test/Analysis/taint-diagnostic-visitor.c
@@ -2,8 +2,17 @@
 
 // This file is for testing enhanced diagnostics produced by the GenericTaintChecker
 
+typedef unsigned long size_t;
+struct _IO_FILE;
+typedef struct _IO_FILE FILE;
+
 int scanf(const char *restrict format, ...);
 int system(const char *command);
+char* getenv( const char* env_var );
+size_t strlen( const char* str );
+void *malloc(size_t size );
+char *fgets(char *str, int n, FILE *stream);
+FILE *stdin;
 
 void taintDiagnostic(void)
 {
@@ -34,3 +43,41 @@
   int vla[x]; // expected-warning {{Declared variable-length array (VLA) has tainted size}}
   // expected-note@-1 {{Declared variable-length array (VLA) has tainted size}}
 }
+
+//Tests if the originated note is correctly placed even if the path is
+//propagating through variables and expressions
+char* taintDiagnosticPropagation(){
+  char *pathbuf;
+  char *pathlist=getenv("PATH"); // expected-note {{Taint originated here}}
+  if (pathlist){ // expected-note {{Assuming 'pathlist' is non-null}}
+	   // expected-note@-1 {{Taking true branch}}
+pathbuf=(char*) malloc(strlen(pathlist)+1); // expected-warning{{Untrusted data is used to specify the buffer size}}
+// expected-note@-1{{Untrusted data is used to specify the buffer size}}
+return pathbuf;
+  }
+  return 0;
+}
+
+
+//Taint origin should be marked correctly even if there are multiple taint
+//sources in the function
+char* taintDiagnosticPropagation2(){
+  char *pathbuf;
+  char *user_env2=getenv("USER_ENV_VAR2");//unrelated taint source
+  char *pathlist=getenv("PATH"); // expected-note {{Taint originated here}}
+  char *user_env=getenv("USER_ENV_VAR");//unrelated taint source
+  if (pathlist){ // expected-note {{Assuming 'pathlist' is non-null}}
+	   // expected-note@-1 {{Taking true branch}}
+pathbuf=(char*) malloc(strlen(pathlist)+1); // expected-warning{{Untrusted data is used to specify the buffer size}}
+// expected-note@-1{{Untrusted data is used to specify the buffer size}}
+return pathbuf;
+  }
+  return 0;
+}
+
+void testReadStdIn(){
+  char buf[1024];
+  fgets(buf, sizeof(buf), stdin);// expected-note {{Taint originated here}}
+  system(buf);// expected-warning {{Untrusted data is passed to a system call}} // expected-note {{Untrusted data is passed to a system call (CERT/STR02-C. Sanitize data passed to complex subsystems)}}
+
+}
Index: clang/test/Analysis/taint-checker-callback-order-without-definition.c
===
--- clang/test/Analysis/taint-checker-callback-order-without-definition.c
+++ clang/test/Analysis/taint-checker-callback-order-without-definition.c
@@ -13,19 +13,22 @@
 
 void top(const char *fname, char *buf) {
   FILE *fp = fopen(fname, "r"); // Introduce taint.
-  // CHECK:  PreCall prepares tainting arg index: -1
-  // CHECK-NEXT: PostCall actually wants to 

[PATCH] D143767: [SVE][Builtins] Lower X forms of fp binop/mla arithmetic builtins to dedicated intrinsics

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

In D143767#4138049 , @dewen wrote:

> Hi, thank you. When will this patch be pushed to the master?@paulwalker-arm

I expect to push the patch to main in the next day or so.  I just need to look 
over some recent InstCombine work, which I think will need to be updated due to 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143767

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

I don't know why but with latest version I still got false positive with 
std::function on Clang 15.

  using Functor= std::function<... something ...>;
  
  std::unique_ptr Class::createSomething(::Functor&& functor) {
 return std::make_unique(std::move(functor));
  }




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:41
+   unless(isMoveConstructor()),
+   isDefinition(), ToParam)
+.bind("containing-ctor")),

duplicated isDefinition and ToParam


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D144004: [DebugMetadata][DwarfDebug] Fix DWARF emisson of function-local imported entities (3/7)

[Juan Manuel Martinez Caamaño via Phabricator via cfe-commits]

jmmartinez added a comment.

I've added just a few minor remarks.




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1082-1084
+  if (!includeMinimalInlineScopes() && !Scope->getInlinedAt())
+for (const auto *Decl : DD->getLocalDeclsForScope(Scope->getScopeNode()))
+  DeferredLocalDecls.insert(Decl);

NIT: You could avoid writing the for loop



Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1646-1649
+  if (LexicalBlockDIEs.count(LB))
+return LexicalBlockDIEs[LB];
+
+  return nullptr;

Nit: `lookup` returns the value in the map or returns a default value.



Comment at: llvm/test/DebugInfo/Generic/split-dwarf-local-import3.ll:17-36
+; CHECK:   DW_TAG_subprogram
+; CHECK: DW_AT_name("foo")
+; CHECK: DW_TAG_imported_declaration
+; CHECK: NULL
+
+; CHECK:   DW_TAG_base_type
+; CHECK: DW_AT_name("int")

I'd be tempted to match the offset of the abstract subprogram and of the 
imported declaration too.
At least for me, it makes clear the intention of the test without running it.

What do you think ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144004

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

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

Suggested a few adjustments in `LexTokenInternal` you might want to test for 
perf improvements (orthogonal to this patch, but could boost its numbers :).
And also noted that `Lexer::getBuffer()` has same issue as 
`getBufferLocation()` re potential for pointer invalidation IIUC.  At a minimum 
we need comments on these functions explaining any risks; better still to 
remove them from the public interface.  If downstream users use these functions 
and complain, good - they need to be aware of this change.




Comment at: clang/include/clang/Lex/Lexer.h:285
   /// Gets source code buffer.
-  StringRef getBuffer() const {
-return StringRef(BufferStart, BufferEnd - BufferStart);
-  }
+  StringRef getBuffer() const { return StringRef(BufferStart, BufferSize); }
 

Same issue as with `getBufferLocation()`, publicly returning it permits 
possible pointer invalidation. Fortunately I only see it used in a single spot 
(prior to this patch anyway) which can be easily eliminated IIUC.  Yank this 
function?  Or make private/append "Unsafe" to name (and explain in comments)?



Comment at: clang/include/clang/Lex/Lexer.h:307
   /// Return the current location in the buffer.
-  const char *getBufferLocation() const { return BufferPtr; }
+  const char *getBufferLocation() const {
+assert(BufferOffset <= BufferSize && "Invalid buffer state");

davrec wrote:
> I think I'd like this to return `unsigned`; i.e. I think after this patch we 
> should not even be publicly exposing buffer locations as pointers, IIUC.  A 
> brief search for uses of `getBufferLocation()` (there aren't many) suggests 
> this would be probably be fine and indeed would get rid of some unnecessary 
> pointer arithmetic.  And indeed if anything really needs that `const char *` 
> that might be a red flag to investigate further.
Looking more closely I see that `getCurrentBufferOffset` returns the unsigned, 
and this patch already changes some `getBufferLocation` usages to 
`getCurrentBufferOffset`.  So, I say either yank it or make it private or 
append "Unsafe" and explain in comments.



Comment at: clang/lib/Lex/Lexer.cpp:2948-2949
+  if (isHorizontalWhitespace(BufferStart[CurOffset])) {
+  SkipWhitespace(Result, CurOffset + 1, TokAtPhysicalStartOfLine);
+  return false;
   }

indent



Comment at: clang/lib/Lex/Lexer.cpp:2973-2975
+  char Char = getAndAdvanceChar(CurOffset, Tmp);
+  switch (Char) {
+  default:

indent



Comment at: clang/lib/Lex/Lexer.cpp:3630-3632
 do {
-  ++CurPtr;
-} while (isHorizontalWhitespace(*CurPtr));
+  ++CurOffset;
+} while (isHorizontalWhitespace(BufferStart[CurOffset]));

```
for (isHorizontalWhitespace(BufferStart[++CurOffset]);;)
  ;
```
might save a few instructions?  Worth trying since this function is the main 
perf-critical one.



Comment at: clang/lib/Lex/Lexer.cpp:3739-3752
+if (BufferStart[CurOffset] == '/' && BufferStart[CurOffset + 1] == '/' &&
+!inKeepCommentMode() && LineComment &&
+(LangOpts.CPlusPlus || !LangOpts.TraditionalCPP)) {
+  if (SkipLineComment(Result, CurOffset + 2, TokAtPhysicalStartOfLine))
 return true; // There is a token to return.
   goto SkipIgnoredUnits;
+} else if (BufferStart[CurOffset] == '/' &&

Spitballing again for possible minor perf improvements:
```
  if (char Char0 = BufferStart[CurOffset] == '/' && !inKeepCommentMode()) {
if (char Char1 = BufferStart[CurOffset + 1] == '/' && LineComment && 
(LangOpts.CPlusPlus || !LangOpts.TraditionalCPP)) {
  if (SkipLineComment(Result, CurOffset + 2, TokAtPhysicalStartOfLine))
return true; // There is a token to return.
  goto SkipIgnoredUnits;
} else if (Char1 == '*') {
  if (SkipBlockComment(Result, CurOffset + 2, TokAtPhysicalStartOfLine))
return true; // There is a token to return.
  goto SkipIgnoredUnits;
}
  } else if (isHorizontalWhitespace(Char0)) {
goto SkipHorizontalWhitespace;
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143142

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

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

[Jolanta Jensen via Phabricator via cfe-commits]

jolanta.jensen updated this revision to Diff 498826.
jolanta.jensen added a comment.

Extended the test to cover Windows OS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144179

Files:
  clang/include/clang/Config/config.h.cmake
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/config-file-from-env.c

Index: clang/test/Driver/config-file-from-env.c
===
--- /dev/null
+++ clang/test/Driver/config-file-from-env.c
@@ -0,0 +1,51 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/empty_dir
+// RUN: echo "-Wall" > %t/c.cfg
+// RUN: echo "-ffast-math" > %t/cxx.cfg
+// RUN: touch %t/unreadable.cfg
+// RUN: chmod 000 %t/unreadable.cfg
+// RUN: touch %t/test.c
+// RUN: touch %t/test.cpp
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/c.cfg %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C1 %} %else %{CONFIG_FILE=%t/c.cfg %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C1 %}
+// CHECK-C1: Configuration file: {{.*}}/c.cfg
+// CHECK-C1: -Wall
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/unreadable.cfg not %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C2 %} %else %{CONFIG_FILE=%t/unreadable.cfg not %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C2 %}
+// CHECK-C2: error: cannot read configuration file '{{.*}}/unreadable.cfg'
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/c.cfg %clang -S %t/test.c --config %S/Inputs/config-1.cfg -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C3 %} %else %{CONFIG_FILE=%t/c.cfg %clang -S %t/test.c --config %S/Inputs/config-1.cfg -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C3 %}
+// CHECK-C3: Configuration file: {{.*}}/config-1.cfg
+// CHECK-C3: -Werror
+// CHECK-C3-NOT: -Wall
+
+// RUN: %if system-windows %{set CONFIG_FILE= not %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C4 %} %else %{CONFIG_FILE= not %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C4 %}
+// CHECK-C4: error: cannot read configuration file ''
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/empty_dir not %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C5 %} %else %{CONFIG_FILE=%t/empty_dir not %clang -S %t/test.c -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-C5 %}
+// CHECK-C5: error: configuration file '{{.*}}/empty_dir' cannot be opened: not a regular file
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/cxx.cfg %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX1 %} %else %{CONFIG_FILE=%t/cxx.cfg %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX1 %}
+// CHECK-CXX1: Configuration file: {{.*}}/cxx.cfg
+// CHECK-CXX1: -ffast-math
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/unreadable.cfg not %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX2 %} %else %{CONFIG_FILE=%t/unreadable.cfg not %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX2 %}
+// CHECK-CXX2: error: cannot read configuration file '{{.*}}/unreadable.cfg'
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/cxx.cfg %clang++ -S %t/test.cpp --config %S/Inputs/config-1.cfg -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX3 %} %else %{CONFIG_FILE=%t/cxx.cfg %clang++ -S %t/test.cpp --config %S/Inputs/config-1.cfg -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX3 %}
+// CHECK-CXX3: Configuration file: {{.*}}/config-1.cfg
+// CHECK-CXX3: -Werror
+// CHECK-CXX3-NOT: -ffast-math
+
+// RUN: %if system-windows %{set CONFIG_FILE= not %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX4 %} %else %{CONFIG_FILE= not %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX4 %}
+// CHECK-CXX4: error: cannot read configuration file ''
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/empty_dir not %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX5 %} %else %{CONFIG_FILE=%t/empty_dir not %clang++ -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX5 %}
+// CHECK-CXX5: error: configuration file '{{.*}}/empty_dir' cannot be opened: not a regular file
+
+// RUN: %if system-windows %{set CONFIG_FILE=%t/c.cfg %clang++ --config=%t/cxx.cfg -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX6 %} %else %{CONFIG_FILE=%t/c.cfg %clang++ --config=%t/cxx.cfg -S %t/test.cpp -o /dev/null -### 2>&1 | FileCheck %s -check-prefix CHECK-CXX6 %}
+// CHECK-CXX6-NOT: Configuration file: {{.*}}/c.cfg
+// CHECK-CXX6-NOT: -Wall
+
+// RUN: rm -rf %t
Index: clang/lib/Driver/Driver.cpp
=

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

[Jolanta Jensen via Phabricator via cfe-commits]

jolanta.jensen added a comment.

In D144179#4132597 , @MaskRay wrote:

> Behaviors due to a new environment variable should be very careful. How is 
> this useful? If you want this, you can add a wrapper around `clang` to 
> specify `--config=` by yourself.
>
> Think about this: if the user sets `CONFIG_FILE=` to something, it will be 
> difficult for toolchain maintainers to triage a bug.

The feature is aimed at system administrators rather than an individual user. 
It simplifies system configuration so the user does not need to configure 
anything him/herself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144179

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

[PATCH] D144321: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai added a comment.

Thanks for doing this. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144321

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Other false-positive that I still see:

  void SomeClass::someVirtualFunction(SomeType&&)
  {
  BOOST_THROW_EXCEPTION(std::runtime_error("Unexpected call "));
  }

It shouldnt warn about virtual functions with unnamed parameter, because this 
can be required by interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

And on something like this I still got reproduction in production code:

  template 
  struct SomeClass
  {
  public:
explicit SomeClass(T&& value) : value(std::forward(value)) {}
   T value;
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:301
+  }
+  void never_moves(T&& t) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter 't' 
is never moved from inside the function body 
[cppcoreguidelines-rvalue-reference-param-not-moved]

that's not always rvalue, if T would be for example int&, then this going to be 
lvalue...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D144293: [PowerPC] Fix the implicit casting for the emulated intrinsics

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

LGTM as long as the naming nit is addressed.




Comment at: clang/lib/Headers/ppc_wrappers/emmintrin.h:57
 typedef __vector unsigned char __v16qu;
+typedef __vector float __v2f;
 

The name `__v2f` seems strange since `__vector float` is a vector of 4 `float` 
values. Should this be `__v4f`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144293

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

[PATCH] D144402: [clang] Move ParsedAttrInfo from Sema to Basic (NFC) r=aaron.ballman

[Anders Waldenborg via Phabricator via cfe-commits]

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

This moves the ParsedAttrInfo class and the registry for attribute
plugin to be part of Basic module instead of Sema module.

This will allow it to be accessed from preprocessor later on.

No functional change intended.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144402

Files:
  clang/include/clang/Basic/ParsedAttrInfo.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/ParsedAttrInfo.cpp
  clang/lib/Sema/ParsedAttr.cpp

Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -26,8 +26,6 @@
 
 using namespace clang;
 
-LLVM_INSTANTIATE_REGISTRY(ParsedAttrInfoRegistry)
-
 IdentifierLoc *IdentifierLoc::create(ASTContext &Ctx, SourceLocation Loc,
  IdentifierInfo *Ident) {
   IdentifierLoc *Result = new (Ctx) IdentifierLoc;
Index: clang/lib/Basic/ParsedAttrInfo.cpp
===
--- /dev/null
+++ clang/lib/Basic/ParsedAttrInfo.cpp
@@ -0,0 +1,18 @@
+//===- ParsedAttrInfo.cpp - Registry for attribute plugins --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains the Registry of attributes added by plugins which
+// derive the ParsedAttrInfo class.
+//
+//===--===//
+
+#include "clang/Basic/ParsedAttrInfo.h"
+
+using namespace clang;
+
+LLVM_INSTANTIATE_REGISTRY(ParsedAttrInfoRegistry)
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -61,6 +61,7 @@
   OpenCLOptions.cpp
   OpenMPKinds.cpp
   OperatorPrecedence.cpp
+  ParsedAttrInfo.cpp
   ProfileList.cpp
   NoSanitizeList.cpp
   SanitizerSpecialCaseList.cpp
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -17,12 +17,12 @@
 #include "clang/Basic/AttrSubjectMatchRules.h"
 #include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/ParsedAttrInfo.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Sema/Ownership.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Allocator.h"
-#include "llvm/Support/Registry.h"
 #include "llvm/Support/VersionTuple.h"
 #include 
 #include 
@@ -36,124 +36,10 @@
 class Expr;
 class IdentifierInfo;
 class LangOptions;
-class ParsedAttr;
 class Sema;
 class Stmt;
 class TargetInfo;
 
-struct ParsedAttrInfo {
-  /// Corresponds to the Kind enum.
-  unsigned AttrKind : 16;
-  /// The number of required arguments of this attribute.
-  unsigned NumArgs : 4;
-  /// The number of optional arguments of this attributes.
-  unsigned OptArgs : 4;
-  /// The number of non-fake arguments specified in the attribute definition.
-  unsigned NumArgMembers : 4;
-  /// True if the parsing does not match the semantic content.
-  unsigned HasCustomParsing : 1;
-  // True if this attribute accepts expression parameter pack expansions.
-  unsigned AcceptsExprPack : 1;
-  /// True if this attribute is only available for certain targets.
-  unsigned IsTargetSpecific : 1;
-  /// True if this attribute applies to types.
-  unsigned IsType : 1;
-  /// True if this attribute applies to statements.
-  unsigned IsStmt : 1;
-  /// True if this attribute has any spellings that are known to gcc.
-  unsigned IsKnownToGCC : 1;
-  /// True if this attribute is supported by #pragma clang attribute.
-  unsigned IsSupportedByPragmaAttribute : 1;
-  /// The syntaxes supported by this attribute and how they're spelled.
-  struct Spelling {
-AttributeCommonInfo::Syntax Syntax;
-const char *NormalizedFullName;
-  };
-  ArrayRef Spellings;
-  // The names of the known arguments of this attribute.
-  ArrayRef ArgNames;
-
-protected:
-  constexpr ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind =
-   AttributeCommonInfo::NoSemaHandlerAttribute)
-  : AttrKind(AttrKind), NumArgs(0), OptArgs(0), NumArgMembers(0),
-HasCustomParsing(0), AcceptsExprPack(0), IsTargetSpecific(0), IsType(0),
-IsStmt(0), IsKnownToGCC(0), IsSupportedByPragmaAttribute(0) {}
-
-  constexpr ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind, unsigned Num

[PATCH] D144403: [clang] Extract attribute plugin instantiation to function (NFC) r=aaron.ballman

[Anders Waldenborg via Phabricator via cfe-commits]

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

This moves the code to instantiate the attribute plugins to the same
place where the plugin registry is defined so they live together and the
user of the plugins doesn't have the burden of instantiating the
plugins.

No functional change intended.

Depends on D144402 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144403

Files:
  clang/include/clang/Basic/ParsedAttrInfo.h
  clang/lib/Basic/ParsedAttrInfo.cpp
  clang/lib/Sema/ParsedAttr.cpp


Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -19,7 +19,6 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/ManagedStatic.h"
 #include 
 #include 
 #include 
@@ -118,13 +117,7 @@
   if (A.getParsedKind() == AttributeCommonInfo::IgnoredAttribute)
 return IgnoredParsedAttrInfo;
 
-  // Otherwise this may be an attribute defined by a plugin. First instantiate
-  // all plugin attributes if we haven't already done so.
-  static llvm::ManagedStatic>>
-  PluginAttrInstances;
-  if (PluginAttrInstances->empty())
-for (auto It : ParsedAttrInfoRegistry::entries())
-  PluginAttrInstances->emplace_back(It.instantiate());
+  // Otherwise this may be an attribute defined by a plugin.
 
   // Search for a ParsedAttrInfo whose name and syntax match.
   std::string FullName = A.getNormalizedFullName();
@@ -132,7 +125,7 @@
   if (SyntaxUsed == AttributeCommonInfo::AS_ContextSensitiveKeyword)
 SyntaxUsed = AttributeCommonInfo::AS_Keyword;
 
-  for (auto &Ptr : *PluginAttrInstances)
+  for (auto &Ptr : getAttributePluginInstances())
 for (auto &S : Ptr->Spellings)
   if (S.Syntax == SyntaxUsed && S.NormalizedFullName == FullName)
 return *Ptr;
Index: clang/lib/Basic/ParsedAttrInfo.cpp
===
--- clang/lib/Basic/ParsedAttrInfo.cpp
+++ clang/lib/Basic/ParsedAttrInfo.cpp
@@ -12,7 +12,21 @@
 
//===--===//
 
 #include "clang/Basic/ParsedAttrInfo.h"
+#include "llvm/Support/ManagedStatic.h"
+#include 
+#include 
 
 using namespace clang;
 
 LLVM_INSTANTIATE_REGISTRY(ParsedAttrInfoRegistry)
+
+const std::list> &
+clang::getAttributePluginInstances() {
+  static llvm::ManagedStatic>>
+  PluginAttrInstances;
+  if (PluginAttrInstances->empty())
+for (auto It : ParsedAttrInfoRegistry::entries())
+  PluginAttrInstances->emplace_back(It.instantiate());
+
+  return *PluginAttrInstances;
+}
Index: clang/include/clang/Basic/ParsedAttrInfo.h
===
--- clang/include/clang/Basic/ParsedAttrInfo.h
+++ clang/include/clang/Basic/ParsedAttrInfo.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Registry.h"
 #include 
+#include 
 
 namespace clang {
 
@@ -137,6 +138,8 @@
 
 typedef llvm::Registry ParsedAttrInfoRegistry;
 
+const std::list> 
&getAttributePluginInstances();
+
 } // namespace clang
 
 #endif // LLVM_CLANG_BASIC_PARSEDATTRINFO_H


Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -19,7 +19,6 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/ManagedStatic.h"
 #include 
 #include 
 #include 
@@ -118,13 +117,7 @@
   if (A.getParsedKind() == AttributeCommonInfo::IgnoredAttribute)
 return IgnoredParsedAttrInfo;
 
-  // Otherwise this may be an attribute defined by a plugin. First instantiate
-  // all plugin attributes if we haven't already done so.
-  static llvm::ManagedStatic>>
-  PluginAttrInstances;
-  if (PluginAttrInstances->empty())
-for (auto It : ParsedAttrInfoRegistry::entries())
-  PluginAttrInstances->emplace_back(It.instantiate());
+  // Otherwise this may be an attribute defined by a plugin.
 
   // Search for a ParsedAttrInfo whose name and syntax match.
   std::string FullName = A.getNormalizedFullName();
@@ -132,7 +125,7 @@
   if (SyntaxUsed == AttributeCommonInfo::AS_ContextSensitiveKeyword)
 SyntaxUsed = AttributeCommonInfo::AS_Keyword;
 
-  for (auto &Ptr : *PluginAttrInstances)
+  for (auto &Ptr : getAttributePluginInstances())
 for (auto &S : Ptr->Spellings)
   if (S.Syntax == SyntaxUsed && S.NormalizedFullName == FullName)
 return *Ptr;
Index: clang/lib/Basic/ParsedAttrInfo.cpp
===
--- clang/lib/Basic/ParsedAttrInfo.cpp
+++ clang/lib/Basic/ParsedAttr

[PATCH] D144404: [clang] Extract function for generated part of clang::hasAttribute (NFC) r=aaron.ballman

[Anders Waldenborg via Phabricator via cfe-commits]

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

This makes it easier to add additional handling when the
tablegen-generated code does not find a match.

No functional change intended.

Depends on D144403 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144404

Files:
  clang/lib/Basic/Attributes.cpp


Index: clang/lib/Basic/Attributes.cpp
===
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -4,6 +4,15 @@
 #include "clang/Basic/IdentifierTable.h"
 using namespace clang;
 
+static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
+StringRef ScopeName, const TargetInfo &Target,
+const LangOptions &LangOpts) {
+
+#include "clang/Basic/AttrHasAttributeImpl.inc"
+
+  return 0;
+}
+
 int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
 const IdentifierInfo *Scope, const IdentifierInfo 
*Attr,
 const TargetInfo &Target, const LangOptions &LangOpts) 
{
@@ -27,7 +36,9 @@
   ScopeName == "omp")
 return (Name == "directive" || Name == "sequence") ? 1 : 0;
 
-#include "clang/Basic/AttrHasAttributeImpl.inc"
+  int res = hasAttributeImpl(Syntax, Name, ScopeName, Target, LangOpts);
+  if (res)
+return res;
 
   return 0;
 }


Index: clang/lib/Basic/Attributes.cpp
===
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -4,6 +4,15 @@
 #include "clang/Basic/IdentifierTable.h"
 using namespace clang;
 
+static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
+StringRef ScopeName, const TargetInfo &Target,
+const LangOptions &LangOpts) {
+
+#include "clang/Basic/AttrHasAttributeImpl.inc"
+
+  return 0;
+}
+
 int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
 const IdentifierInfo *Scope, const IdentifierInfo *Attr,
 const TargetInfo &Target, const LangOptions &LangOpts) {
@@ -27,7 +36,9 @@
   ScopeName == "omp")
 return (Name == "directive" || Name == "sequence") ? 1 : 0;
 
-#include "clang/Basic/AttrHasAttributeImpl.inc"
+  int res = hasAttributeImpl(Syntax, Name, ScopeName, Target, LangOpts);
+  if (res)
+return res;
 
   return 0;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144405: [clang][pp] Handle attributes defined by plugin in __has_attribute r=aaron.ballman

[Anders Waldenborg via Phabricator via cfe-commits]

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

When using attributes by plugins (both in clang and clang-tidy) the
preprocessor functions `__has_attribute`, `__has_c_attribute`,
`__has_cpp_attribute` still returned 0.

That problem is fixed by having the "hasAttribute" function also check
if any of the plugins provide that attribute.

This also adds C2x spelling to the example plugin for attributes so that
`__has_c_attribute` can be tested.

Depends on D144404 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144405

Files:
  clang/examples/Attribute/Attribute.cpp
  clang/lib/Basic/Attributes.cpp
  clang/test/Frontend/plugin-attribute-pp.cpp


Index: clang/test/Frontend/plugin-attribute-pp.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-attribute-pp.cpp
@@ -0,0 +1,36 @@
+// RUN: split-file %s %t
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %t/has_attr.cpp | 
FileCheck %t/has_attr.cpp
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %t/has_attr.c | 
FileCheck %t/has_attr.c
+// REQUIRES: plugins, examples
+
+
+//--- has_attr.cpp
+#if __has_attribute (example)
+// CHECK: YES - It did exist
+YES - It did exist
+#endif
+#if __has_cpp_attribute (example)
+// CHECK: YES - It did cpp-exist
+YES - It did cpp-exist
+#endif
+
+#if __has_attribute (doesnt_exist)
+// CHECK-NOT: NO - There is no such attribute
+NO - There is no such attribute
+#endif
+
+
+//--- has_attr.c
+#if __has_attribute (example)
+// CHECK: YES - It did exist
+YES - It did exist
+#endif
+#if __has_c_attribute (example)
+// CHECK: YES - It did c-exist
+YES - It did c-exist
+#endif
+
+#if __has_attribute (doesnt_exist)
+// CHECK-NOT: NO - There is no such attribute
+NO - There is no such attribute
+#endif
Index: clang/lib/Basic/Attributes.cpp
===
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -2,6 +2,7 @@
 #include "clang/Basic/AttrSubjectMatchRules.h"
 #include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/ParsedAttrInfo.h"
 using namespace clang;
 
 static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
@@ -40,6 +41,13 @@
   if (res)
 return res;
 
+  // Check if any plugin provides this attribute.
+  for (auto &AttrPlugin : getAttributePluginInstances()) {
+for (auto &S : AttrPlugin->Spellings)
+  if (S.Syntax == Syntax && S.NormalizedFullName == Name)
+return 1;
+  }
+
   return 0;
 }
 
Index: clang/examples/Attribute/Attribute.cpp
===
--- clang/examples/Attribute/Attribute.cpp
+++ clang/examples/Attribute/Attribute.cpp
@@ -9,6 +9,8 @@
 // Example clang plugin which adds an an annotation to file-scope declarations
 // with the 'example' attribute.
 //
+// This plugin is used by clang/test/Frontend/plugin-attribute tests.
+//
 
//===--===//
 
 #include "clang/AST/ASTContext.h"
@@ -27,9 +29,10 @@
 // number of arguments. This just illustrates how many arguments a
 // `ParsedAttrInfo` can hold, we will not use that much in this example.
 OptArgs = 15;
-// GNU-style __attribute__(("example")) and C++-style [[example]] and
+// GNU-style __attribute__(("example")) and C++/C2x-style [[example]] and
 // [[plugin::example]] supported.
 static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
+ {ParsedAttr::AS_C2x, "example"},
  {ParsedAttr::AS_CXX11, "example"},
  {ParsedAttr::AS_CXX11, 
"plugin::example"}};
 Spellings = S;


Index: clang/test/Frontend/plugin-attribute-pp.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-attribute-pp.cpp
@@ -0,0 +1,36 @@
+// RUN: split-file %s %t
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %t/has_attr.cpp | FileCheck %t/has_attr.cpp
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %t/has_attr.c | FileCheck %t/has_attr.c
+// REQUIRES: plugins, examples
+
+
+//--- has_attr.cpp
+#if __has_attribute (example)
+// CHECK: YES - It did exist
+YES - It did exist
+#endif
+#if __has_cpp_attribute (example)
+// CHECK: YES - It did cpp-exist
+YES - It did cpp-exist
+#endif
+
+#if __has_attribute (doesnt_exist)
+// CHECK-NOT: NO - There is no such attribute
+NO - There is no such attribute
+#endif
+
+
+//--- has_attr.c
+#if __has_attribute (example)
+// CHECK: YES - It did exist
+YES - It did exist
+#endif
+#if __has_c_attri

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 498873.
ccotter added a comment.

- Remove duplicated matchers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -0,0 +1,303 @@
+// RUN: %check_clang_tidy -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: false}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,CXX14 -std=c++14 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: false}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,STRICT -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: true}]}" -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept;
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept;
+
+}
+// NOLINTEND
+
+struct Obj {
+  Obj();
+  Obj(const Obj&);
+  Obj& operator=(const Obj&);
+  Obj(Obj&&);
+  Obj& operator=(Obj&&);
+  void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void copies_object(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj copy = o;
+}
+
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  // CHECK-MESSAGES: :[[@LINE-2]]:41: warning: rvalue reference parameter 'o2' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void never_moves_some_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  Obj other{std::move(o2)};
+}
+
+void never_moves_mixed(Obj o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: rvalue reference parameter 'o2' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value(Obj&& o) {
+  auto f = [o]() {
+consumes_object(std::move(o));
+  };
+  // CHECK-MESSAGES: :[[@LINE-4]]:47: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value_nested(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  auto f = [&o]() {
+auto f_nested = [o]() {
+  consumes_object(std::move(o));
+};
+  };
+  auto f2 = [o]() {
+auto f_nested = [&o]() {
+  consumes_object(std::move(o));
+};
+  };
+  auto f3 = [o]() {
+auto f_nested = [&o]() {
+  auto f_nested_inner = [&o]() {
+consumes_object(std::move(o));
+  };
+};
+ 

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 498878.
DmitryPolukhin added a comment.

Rebase and retest, cannot reproduce test failure locally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143436

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/test/Inputs/did-change-configuration-params.args
  clang-tools-extra/clangd/test/did-change-configuration-params.test
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/lib/Tooling/Tooling.cpp

Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
@@ -299,6 +300,31 @@
   }
 }
 
+void addExpandedResponseFiles(std::vector &CommandLine,
+  llvm::StringRef WorkingDir,
+  llvm::cl::TokenizerCallback Tokenizer,
+  llvm::vfs::FileSystem &FS) {
+  bool SeenRSPFile = false;
+  llvm::SmallVector Argv;
+  Argv.reserve(CommandLine.size());
+  for (auto &Arg : CommandLine) {
+Argv.push_back(Arg.c_str());
+if (!Arg.empty())
+  SeenRSPFile |= Arg.front() == '@';
+  }
+  if (!SeenRSPFile)
+return;
+  llvm::BumpPtrAllocator Alloc;
+  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
+  llvm::Error Err =
+  ECtx.setVFS(&FS).setCurrentDir(WorkingDir).expandResponseFiles(Argv);
+  if (Err)
+llvm::errs() << Err;
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  CommandLine = std::move(ExpandedArgv);
+}
+
 } // namespace tooling
 } // namespace clang
 
@@ -684,7 +710,7 @@
 
   if (!Invocation.run())
 return nullptr;
- 
+
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
@@ -48,28 +49,9 @@
 
 private:
   std::vector expand(std::vector Cmds) const {
-for (auto &Cmd : Cmds) {
-  bool SeenRSPFile = false;
-  llvm::SmallVector Argv;
-  Argv.reserve(Cmd.CommandLine.size());
-  for (auto &Arg : Cmd.CommandLine) {
-Argv.push_back(Arg.c_str());
-if (!Arg.empty())
-  SeenRSPFile |= Arg.front() == '@';
-  }
-  if (!SeenRSPFile)
-continue;
-  llvm::BumpPtrAllocator Alloc;
-  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
-  llvm::Error Err = ECtx.setVFS(FS.get())
-.setCurrentDir(Cmd.Directory)
-.expandResponseFiles(Argv);
-  if (Err)
-llvm::errs() << Err;
-  // Don't assign directly, Argv aliases CommandLine.
-  std::vector ExpandedArgv(Argv.begin(), Argv.end());
-  Cmd.CommandLine = std::move(ExpandedArgv);
-}
+for (auto &Cmd : Cmds)
+  tooling::addExpandedResponseFiles(Cmd.CommandLine, Cmd.Directory,
+Tokenizer, *FS);
 return Cmds;
   }
 
Index: clang/include/clang/Tooling/Tooling.h
===
--- clang/include/clang/Tooling/Tooling.h
+++ clang/include/clang/Tooling/Tooling.h
@@ -506,6 +506,12 @@
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs);
 
+/// Helper function that expands response files in command line.
+void addExpandedResponseFiles(std::vector &CommandLine,
+  llvm::StringRef WorkingDir,
+  llvm::cl::TokenizerCallback Tokenizer,
+  llvm::vfs::FileSystem &FS);
+
 /// Creates a \c CompilerInvocation.
 CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics,
   ArrayRef CC1Args,
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

In

  template 
  struct SomeClass
  {
  public:
explicit SomeClass(T&& value) : value(std::forward(value)) {}
   T value;
  };

`T` is not a universal reference in the constructor, it's an rvalue reference 
to `T`. There is no deducing context, so universal references are not involved 
here (and, `std::forward` would also be incorrect here). The following would be 
a deducing context with a universal reference:

  template 
  struct SomeClass
  {
  public:
template 
explicit SomeClass(T2&& value) : value(std::forward(value)) {}
   T value;
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

I was split on handling the case where the parameter that was never moved from 
was not named. For this guideline enforcement to flag all unmoved rvalue 
reference parameters, code that `std::moves` into an argument to a virtual 
method call could be especially confusing or dangerous with regards to whether 
the object was truly "moved" (move constructor/assignment invoked within the 
method call). E.g.,

  LargeObject obj;
  ClassWithVirtualMerthods* v = ...;
  v->some_virtual_call(std::move(obj));

If would be confusing if some implementations of the virtual method actually 
moved the parameter, while others didn't. I'm inclined in this case (regardless 
of whether the parameter is named in a virtual overridden method) to still flag 
this code.

I could see there being an option for this specific case (defaulted off), 
although it's very specific of a case, and for something I think the rule 
should be especially attentive towards. Could I get some additional feedback on 
this specific case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:301
+  }
+  void never_moves(T&& t) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter 't' 
is never moved from inside the function body 
[cppcoreguidelines-rvalue-reference-param-not-moved]

PiotrZSL wrote:
> that's not always rvalue, if T would be for example int&, then this going to 
> be lvalue...
Looking at the rule enforcement, I might have overlooked one thing:

 > Flag all X&& parameters (where X is not a template type parameter name) 
 > where the function body uses them without std::move.

I think this means this code should not be flagged, even if `T` is an object 
which is cheap to move but expensive to copy (and not a reference to that 
type). I would like for `AClassTemplate` to be flagged, but not 
`AClassTemplate`, so let me look into this to see if I can limit 
this to instantiations of the type, rather than looking at the template 
definition itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D144181: [clang][DebugInfo] Add abi-tags on constructors/destructors as LLVM annotations

[Michael Buch via Phabricator via cfe-commits]

Michael137 updated this revision to Diff 498898.
Michael137 added a comment.

- Hide behind LLDB tuning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144181

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/attr-abi_tag-ctors-dtors.cpp

Index: clang/test/CodeGen/attr-abi_tag-ctors-dtors.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-abi_tag-ctors-dtors.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang -glldb -S -emit-llvm -o - %s | FileCheck %s
+
+struct Foo {
+  [[gnu::abi_tag("Ctor")]] Foo() {}
+  [[gnu::abi_tag("Dtor")]] ~Foo() {}
+};
+
+struct Bar {
+  [[gnu::abi_tag("v1", ".1")]] [[gnu::abi_tag("Ignored1")]] Bar() {}
+  [[gnu::abi_tag("v2", ".0")]] [[gnu::abi_tag("Ignored2")]] ~Bar() {}
+};
+
+Bar b;
+Foo f;
+
+// CHECK:  ![[#]] = !DISubprogram(name: "Foo",
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: [[#]], annotations: ![[FOO_CTOR_ANNOTS:[0-9]+]])
+// CHECK:  ![[FOO_CTOR_ANNOTS]] = !{![[CTOR:[0-9]+]]}
+// CHECK:  ![[CTOR]] = !{!"abi_tag", !"Ctor"}
+// CHECK:  ![[#]] = !DISubprogram(name: "~Foo",
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: [[#]], annotations: ![[FOO_DTOR_ANNOTS:[0-9]+]])
+// CHECK:  ![[FOO_DTOR_ANNOTS]] = !{![[DTOR:[0-9]+]]}
+// CHECK:  ![[DTOR]] = !{!"abi_tag", !"Dtor"}
+
+// CHECK:  ![[#]] = !DISubprogram(name: "Bar",
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: [[#]], annotations: ![[BAR_CTOR_ANNOTS:[0-9]+]])
+// CHECK:  ![[BAR_CTOR_ANNOTS]] = !{![[CTOR:[0-9]+]], ![[TAG:[0-9]+]]}
+// CHECK:  ![[CTOR]] = !{!"abi_tag", !".1"}
+// CHECK:  ![[TAG]] = !{!"abi_tag", !"v1"}
+// CHECK:  ![[#]] = !DISubprogram(name: "~Bar",
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: [[#]], annotations: ![[BAR_DTOR_ANNOTS:[0-9]+]])
+// CHECK:  ![[BAR_DTOR_ANNOTS]] = !{![[DTOR:[0-9]+]], ![[TAG:[0-9]+]]}
+// CHECK:  ![[DTOR]] = !{!"abi_tag", !".0"}
+// CHECK:  ![[TAG]] = !{!"abi_tag", !"v2"}
+// CHECK:  ![[#]] = distinct !DISubprogram(name: "Bar", linkageName:
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: ![[#]], declaration: ![[#]], retainedNodes: ![[#]])
+// CHECK:  ![[#]] = distinct !DISubprogram(name: "~Bar", linkageName:
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: ![[#]], declaration: ![[#]], retainedNodes: ![[#]])
+// CHECK:  ![[#]] = distinct !DISubprogram(name: "Foo", linkageName:
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: ![[#]], declaration: ![[#]], retainedNodes: ![[#]])
+// CHECK:  ![[#]] = distinct !DISubprogram(name: "~Foo", linkageName:
+// CHECK-SAME: flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: ![[#]], declaration: ![[#]], retainedNodes: ![[#]])
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -303,6 +303,10 @@
   /// A helper function to collect debug info for btf_decl_tag annotations.
   llvm::DINodeArray CollectBTFDeclTagAnnotations(const Decl *D);
 
+  /// A helper function to collect debug info for abi_tag annotations.
+  /// Returns a null DINodeArray if 'D' doesn't have any 'clang::AbiTagAttr's.
+  llvm::DINodeArray CollectAbiTagAnnotations(const Decl *D);
+
   llvm::DIType *createFieldType(StringRef name, QualType type,
 SourceLocation loc, AccessSpecifier AS,
 uint64_t offsetInBits, uint32_t AlignInBits,
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1842,11 +1842,20 @@
   SPFlags |= llvm::DISubprogram::SPFlagDeleted;
   };
 
+  llvm::DINodeArray AbiTagAnnotations = nullptr;
+
   switch (Method->getKind()) {
 
   case Decl::CXXConstructor:
   case Decl::CXXDestructor:
 checkAttrDeleted(Method);
+
+// Add annotations for abi_tag's for constructors/destructors
+// because their declarations don't carry linkage names (which
+// encodes the existance of abi-tags). LLDB uses these annotations
+// to resolve calls to abi-tagged constructors/destructors.
+if (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB)
+  AbiTagAnnotations = CollectAbiTagAnnotations(Method);
 break;
   case Decl::CXXMethod:
 if (Method->isCopyAssignmentOperator() ||
@@ -1893,7 +1902,7 @@
   llvm::DISubprogram *SP = DBuilder.createMethod(
   RecordTy, MethodName, MethodLinkageName, MethodDefUnit, MethodLine,
   MethodTy, VIndex, ThisAdjustment, ContainingType, Flags, SPFlags,
-  TParamsArray.get());
+  TParamsArray.get(), nullptr, AbiTagAnnotations);
 
   SPCache[Method->getCanonicalDecl()].reset(SP);
 
@@ -2195,6 +2204,21 

[PATCH] D144181: [clang][DebugInfo] Add abi-tags on constructors/destructors as LLVM annotations

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a subscriber: labath.
Michael137 added a comment.

> Hmm, I'd sort of still be inclined to look further at the linkage name option 
> - but maybe the existence of a situation where the usage is built with debug 
> info, but the out of line ctor definitions are all in another library without 
> debug info is a sufficiently motivating use case to justify the addition of 
> these attributes to the ctor declaration. *shrug* Carry on then.

That would've been convenient but I don't think we can get it right with 
attaching the linkage name (even if we are willing to do the search for the 
correct linkage name). That would essentially coalesce the possibly multiple 
definitions of the constructor into a single one (since there's only ever a 
single `CXXConstructorDecl` in the AST for a given constructor in the source)

(CC @labath)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144181

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

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

[David Rector via Phabricator via cfe-commits]

davrec added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:3630-3632
 do {
-  ++CurPtr;
-} while (isHorizontalWhitespace(*CurPtr));
+  ++CurOffset;
+} while (isHorizontalWhitespace(BufferStart[CurOffset]));

davrec wrote:
> ```
> for (isHorizontalWhitespace(BufferStart[++CurOffset]);;)
>   ;
> ```
> might save a few instructions?  Worth trying since this function is the main 
> perf-critical one.
^ Ignore, erroneous :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143142

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

In D141569#4139290 , @ccotter wrote:

> In
>
>   template 
>   struct SomeClass
>   {
>   public:
> explicit SomeClass(T&& value) : value(std::forward(value)) {}
>T value;
>   };
>
> `T` is not a universal reference in the constructor, it's an rvalue reference 
> to `T`. There is no deducing context, so universal references are not 
> involved here (and, `std::forward` would also be incorrect here). The 
> following would be a deducing context with a universal reference:

Wrong:

  #include 
  
  template
  struct Wrap {
  
Wrap(T&& arg) 
  : value(arg)
{}
  
T value;  
  };
  
  struct Value {};
  
  int main()
  {
  Value value;
  Wrap v1(std::move(value));
  Wrap v2(std::move(value));
  Wrap v3(value);
  return 0;
  }

If you add `std::move` you will get compilation error, if you add std::forward, 
everything will work fine. Simply Value& and && will evaluate to Value&, no 
rvalue reference here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

I prefer to follow established convention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

In D141569#4139320 , @ccotter wrote:

> I was split on handling the case where the parameter that was never moved 
> from was not named. For this guideline enforcement to flag all unmoved rvalue 
> reference parameters, code that `std::moves` into an argument to a virtual 
> method call could be especially confusing or dangerous with regards to 
> whether the object was truly "moved" (move constructor/assignment invoked 
> within the method call). E.g.,
>
>   LargeObject obj;
>   ClassWithVirtualMerthods* v = ...;
>   v->some_virtual_call(std::move(obj));
>
> If would be confusing if some implementations of the virtual method actually 
> moved the parameter, while others didn't. I'm inclined in this case 
> (regardless of whether the parameter is named in a virtual overridden method) 
> to still flag this code. Looking at the rule enforcement criteria again, the 
> rule states "Don’t conditionally move from objects" which this case falls 
> into.
>
> I could see there being an option for this specific case (defaulted off), 
> although it's very specific of a case, and for something I think the rule 
> should be especially attentive towards. Could I get some additional feedback 
> on this specific case?

I'm not so sure, you may have Base class Car that would have void 
changeBackRightDoor(Door&&) method, and then you could have class Car5Doors and 
Car3Doors, in such case its easy to known that Car3Doors wouldn't implement 
changeBackRightDoor method, and that method wouldnt move anything, using 
unnamed parameters is a way to go. If you dont exclude this, then at least 
provide option for that, like IgnoreUnnamedParameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

If we're removing this feature it would probably make sense to remove the 
handling / flag altogether as `-fopenmp-implicit-rpath` isn't that much 
different from `-Wl,-rpath=/path/to/llvm/install/lib`.

Overall, the problem is that we want to tie the `libomptarget.so` with the same 
`clang`. If configuration files can solve this we could distribute those. I do 
remember being pinged about the Fedora concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143306

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

[PATCH] D144037: [clang-tidy] allow tests to use --config-file instead of --config

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau added a comment.

I do not have commit access to the repo, can someone commit this patch for me ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144037

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

[PATCH] D144041: [clang-tidy] add primitive types for hungarian identifier-naming (unsigned char and void)

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau added a comment.

I do not have commit access to the repo, can someone commit this patch for me ? 
(after D144037 )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144041

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

[clang-tools-extra] 38b1a17 - [clang-tidy] allow tests to use --config-file instead of --config

[Piotr Zegar via cfe-commits]

Author: Alexis Murzeau
Date: 2023-02-20T19:06:45Z
New Revision: 38b1a17c519d3873f413156c302cb7ed0d39ee3d

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

LOG: [clang-tidy] allow tests to use --config-file instead of --config

The previous way to test hungarian notation doesn't check CHECK-FIXES.

This will allow readability-identifier-naming tests of Hungarian
notation to keep the use of an external .clang-tidy file (not embedded
within the .cpp test file) and properly check CHECK-FIXES.

Also, it turns out the hungarian notation tests use the wrong
.clang-tidy file, so fix that too to make these tests ok.

This is a part of a fix for issue 
https://github.com/llvm/llvm-project/issues/60670.

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/check_clang_tidy.py

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Removed: 




diff  --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py 
b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
index 760550abd41bc..f2d9f40fe568c 100755
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -107,9 +107,10 @@ def __init__(self, args, extra_args):
 # If the test does not specify a config style, force an empty one; 
otherwise
 # auto-detection logic can discover a ".clang-tidy" file that is not 
related to
 # the test.
-if not any(
-[arg.startswith('-config=') for arg in self.clang_tidy_extra_args]):
-  self.clang_tidy_extra_args.append('-config={}')
+if not any([
+re.match('^-?-config(-file)?=', arg)
+for arg in self.clang_tidy_extra_args]):
+  self.clang_tidy_extra_args.append('--config={}')
 
 if extension in ['.m', '.mm']:
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', 
'-fblocks'] + \

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
index 60a4d9b7464c1..4efc455aa3ce9 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -1,5 +1,4 @@
-// RUN: clang-tidy %s 
--config-file=%S/Inputs/identifier-naming/hungarian-notation1/.clang-tidy 2>&1 \
-// RUN:   | FileCheck -check-prefixes=CHECK-MESSAGES %s
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- 
--config-file=%S/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
 
 // clang-format off
 typedef signed char int8_t; // NOLINT

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
index 18a39b15b0c5e..83aa88758e138 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -1,5 +1,4 @@
-// RUN: clang-tidy %s 
--config-file=%S/Inputs/identifier-naming/hungarian-notation2/.clang-tidy 2>&1 \
-// RUN:   | FileCheck -check-prefixes=CHECK-MESSAGES %s
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- 
--config-file=%S/Inputs/identifier-naming/hungarian-notation1/.clang-tidy
 
 // clang-format off
 typedef signed char int8_t; // NOLINT



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

[PATCH] D144037: [clang-tidy] allow tests to use --config-file instead of --config

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG38b1a17c519d: [clang-tidy] allow tests to use --config-file 
instead of --config (authored by amurzeau, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144037

Files:
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -1,5 +1,4 @@
-// RUN: clang-tidy %s 
--config-file=%S/Inputs/identifier-naming/hungarian-notation2/.clang-tidy 2>&1 \
-// RUN:   | FileCheck -check-prefixes=CHECK-MESSAGES %s
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- 
--config-file=%S/Inputs/identifier-naming/hungarian-notation1/.clang-tidy
 
 // clang-format off
 typedef signed char int8_t; // NOLINT
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -1,5 +1,4 @@
-// RUN: clang-tidy %s 
--config-file=%S/Inputs/identifier-naming/hungarian-notation1/.clang-tidy 2>&1 \
-// RUN:   | FileCheck -check-prefixes=CHECK-MESSAGES %s
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- 
--config-file=%S/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
 
 // clang-format off
 typedef signed char int8_t; // NOLINT
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -107,9 +107,10 @@
 # If the test does not specify a config style, force an empty one; 
otherwise
 # auto-detection logic can discover a ".clang-tidy" file that is not 
related to
 # the test.
-if not any(
-[arg.startswith('-config=') for arg in self.clang_tidy_extra_args]):
-  self.clang_tidy_extra_args.append('-config={}')
+if not any([
+re.match('^-?-config(-file)?=', arg)
+for arg in self.clang_tidy_extra_args]):
+  self.clang_tidy_extra_args.append('--config={}')
 
 if extension in ['.m', '.mm']:
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', 
'-fblocks'] + \


Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -1,5 +1,4 @@
-// RUN: clang-tidy %s --config-file=%S/Inputs/identifier-naming/hungarian-notation2/.clang-tidy 2>&1 \
-// RUN:   | FileCheck -check-prefixes=CHECK-MESSAGES %s
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- --config-file=%S/Inputs/identifier-naming/hungarian-notation1/.clang-tidy
 
 // clang-format off
 typedef signed char int8_t; // NOLINT
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -1,5 +1,4 @@
-// RUN: clang-tidy %s --config-file=%S/Inputs/identifier-naming/hungarian-notation1/.clang-tidy 2>&1 \
-// RUN:   | FileCheck -check-prefixes=CHECK-MESSAGES %s
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- --config-file=%S/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
 
 // clang-format off
 typedef signed char int8_t; // NOLINT
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -107,9 +107,10 @@
 # If the test does not specify a config style, force an empty one; otherwise
 # auto-d

[clang-tools-extra] 37e6a4f - [clang-tidy] add primitive types for hungarian identifier-naming (unsigned char and void)

[Piotr Zegar via cfe-commits]

Author: Alexis Murzeau
Date: 2023-02-20T19:18:51Z
New Revision: 37e6a4f9496c8e35efc654d7619a79d6dbb72f99

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

LOG: [clang-tidy] add primitive types for hungarian identifier-naming (unsigned 
char and void)

Add `unsigned char` and `void` types to recognized PrimitiveTypes.
Fixes: #60670

Depends on D144037

Reviewed By: carlosgalvezp

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

Added: 


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

clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index b2b2875731874..96bf035843577 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -173,6 +173,7 @@ static StringRef const StyleNames[] = {
  m(unsigned-short-int) \
  m(unsigned-short) \
  m(unsigned-int) \
+ m(unsigned-char) \
  m(unsigned) \
  m(long-long-int) \
  m(long-double) \
@@ -180,6 +181,7 @@ static StringRef const StyleNames[] = {
  m(long-int) \
  m(long) \
  m(ptr
diff _t) \
+ m(void) \
 
 static StringRef const HungarainNotationPrimitiveTypes[] = {
 #define STRINGIZE(v) #v,
@@ -751,13 +753,15 @@ void 
IdentifierNamingCheck::HungarianNotation::loadDefaultConfig(
 {"unsigned short int",  "usi" },
 {"unsigned short",  "us"  },
 {"unsigned int","ui"  },
+{"unsigned char",   "uc"  },
 {"unsigned","u"   },
 {"long long int",   "lli" },
 {"long double", "ld"  },
 {"long long",   "ll"  },
 {"long int","li"  },
 {"long","l"   },
-{"ptr
diff _t",   "p"   }};
+{"ptr
diff _t",   "p"   },
+{"void",""}};
   // clang-format on
   for (const auto &PT : PrimitiveTypes)
 HNOption.PrimitiveType.try_emplace(PT.first, PT.second);

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
index 2ae91cab1c484..e9ed35e2c6c23 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
@@ -200,6 +200,8 @@ CheckOptions:
 value:  us
   - key:
readability-identifier-naming.HungarianNotation.PrimitiveType.unsigned-int
 value:  ui
+  - key:
readability-identifier-naming.HungarianNotation.PrimitiveType.unsigned-char
+value:  uc
   - key:
readability-identifier-naming.HungarianNotation.PrimitiveType.unsigned
 value:  u
   - key:
readability-identifier-naming.HungarianNotation.PrimitiveType.long-long-int
@@ -214,6 +216,8 @@ CheckOptions:
 value:  l
   - key:readability-identifier-naming.HungarianNotation.PrimitiveType.ptr
diff _t
 value:  p
+  - key:readability-identifier-naming.HungarianNotation.PrimitiveType.void
+value:  v
   - key:
readability-identifier-naming.HungarianNotation.UserDefinedType.BOOL
 value:  b
   - key:
readability-identifier-naming.HungarianNotation.UserDefinedType.BOOLEAN

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
index 4efc455aa3ce9..d5e96f1ddedda 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -366,15 +366,15 @@ int *DataIntPtr[1] = {0};
 
 void *BufferPtr1;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global 
pointer 'BufferPtr1' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}void *pBufferPtr1;
+// CHECK-FIXES: {{^}}void *pvBufferPtr1;
 
 void **BufferPtr2;
 // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for global 
point

[PATCH] D144041: [clang-tidy] add primitive types for hungarian identifier-naming (unsigned char and void)

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG37e6a4f9496c: [clang-tidy] add primitive types for hungarian 
identifier-naming (unsigned char… (authored by amurzeau, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144041

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -567,6 +567,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for global variable 'ValueUnsignedInt' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}unsigned int uiValueUnsignedInt = 0;
 
+unsigned char ValueUnsignedChar = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'ValueUnsignedChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char ucValueUnsignedChar = 0;
+
 long int ValueLongInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'ValueLongInt' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}long int liValueLongInt = 0;
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -366,15 +366,15 @@
 
 void *BufferPtr1;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global pointer 'BufferPtr1' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}void *pBufferPtr1;
+// CHECK-FIXES: {{^}}void *pvBufferPtr1;
 
 void **BufferPtr2;
 // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for global pointer 'BufferPtr2' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}void **ppBufferPtr2;
+// CHECK-FIXES: {{^}}void **ppvBufferPtr2;
 
 void **pBufferPtr3;
 // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for global pointer 'pBufferPtr3' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}void **ppBufferPtr3;
+// CHECK-FIXES: {{^}}void **ppvBufferPtr3;
 
 int *pBufferPtr4;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global pointer 'pBufferPtr4' [readability-identifier-naming]
@@ -387,7 +387,7 @@
 
 void *ValueVoidPtr = NULL;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global pointer 'ValueVoidPtr' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}void *pValueVoidPtr = NULL;
+// CHECK-FIXES: {{^}}void *pvValueVoidPtr = NULL;
 
 ptrdiff_t PtrDiff = NULL;
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global variable 'PtrDiff' [readability-identifier-naming]
@@ -403,7 +403,7 @@
 
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
+// CHECK-FIXES: {{^}}void MyFunc2(void* pvVal){}
 
 
 //===--===//
@@ -567,6 +567,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for global variable 'ValueUnsignedInt' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}unsigned int uiValueUnsignedInt = 0;
 
+unsigned char ValueUnsignedChar = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'ValueUnsignedChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char ucValueUnsignedChar = 0;
+
 long int ValueLongInt = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'ValueLongInt' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}long int liValueLongInt = 0;
Index: clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
+++ clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
@@ -200,6 +200,8 @@
 valu

[clang] 997dc7e - [debug-info][codegen] Prevent creation of self-referential SP node

[Felipe de Azevedo Piovezan via cfe-commits]

Author: Felipe de Azevedo Piovezan
Date: 2023-02-20T14:22:49-05:00
New Revision: 997dc7e00f49663b60a78e18df1dfecdf62a4172

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

LOG: [debug-info][codegen] Prevent creation of self-referential SP node

The function `CGDebugInfo::EmitFunctionDecl` is supposed to create a
declaration -- never a _definition_ -- of a subprogram. This is made
evident by the fact that the SPFlags never have the "Declaration" bit
set by that function.

However, when `EmitFunctionDecl` calls `DIBuilder::createFunction`, it
still tries to fill the "Declaration" argument by passing it the result
of `getFunctionDeclaration(D)`. This will query an internal cache of
previously created declarations and, for most code paths, we return
nullptr; all is good.

However, as reported in [0], there are pathological cases in which we
attempt to recreate a declaration, so the cache query succeeds,
resulting in a subprogram declaration whose declaration field points to
another declaration. Through a series of RAUWs, the declaration field
ends up pointing to the SP itself. Self-referential MDNodes can't be
`unique`, which causes the verifier to fail (declarations must be
`unique`).

We can argue that the caller should check the cache first, but this is
not a correctness issue (declarations are `unique` anyway). The bug is
that `CGDebugInfo::EmitFunctionDecl` should always pass `nullptr` to the
declaration argument of `DIBuilder::createFunction`, expressing the fact
that declarations don't point to other declarations. AFAICT this is not
something for which any reasonable meaning exists.

This seems a lot like a copy-paste mistake that has survived for ~10
years, since other places in this file have the exact same call almost
token-by-token.

I've tested this by compiling LLVMSupport with and without the patch, O2
and O0, and comparing the dwarfdump of the lib. The dumps are identical
modulo the attributes decl_file/producer/comp_dir.

[0]: https://github.com/llvm/llvm-project/issues/59241

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

Added: 
llvm/test/Verifier/disubprogram_declaration.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
llvm/docs/LangRef.rst
llvm/lib/IR/Verifier.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index bb91b5116d71b..4dab595ada76b 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4225,10 +4225,9 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, 
SourceLocation Loc,
 
   llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D);
   llvm::DISubroutineType *STy = getOrCreateFunctionType(D, FnType, Unit);
-  llvm::DISubprogram *SP =
-  DBuilder.createFunction(FDContext, Name, LinkageName, Unit, LineNo, STy,
-  ScopeLine, Flags, SPFlags, TParamsArray.get(),
-  getFunctionDeclaration(D), nullptr, Annotations);
+  llvm::DISubprogram *SP = DBuilder.createFunction(
+  FDContext, Name, LinkageName, Unit, LineNo, STy, ScopeLine, Flags,
+  SPFlags, TParamsArray.get(), nullptr, nullptr, Annotations);
 
   // Preserve btf_decl_tag attributes for parameters of extern functions
   // for BPF target. The parameters created in this loop are attached as

diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 4052e58faa7fb..b4c9cff4821cc 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -5776,11 +5776,12 @@ retained, even if their IR counterparts are optimized 
out of the IR. The
 
 .. _DISubprogramDeclaration:
 
-When ``isDefinition: false``, subprograms describe a declaration in the type
-tree as opposed to a definition of a function.  If the scope is a composite
-type with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``,
-then the subprogram declaration is uniqued based only on its ``linkageName:``
-and ``scope:``.
+When ``spFlags: DISPFlagDefinition`` is not present, subprograms describe a
+declaration in the type tree as opposed to a definition of a function. In this
+case, the ``declaration`` field must be empty. If the scope is a composite type
+with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, then
+the subprogram declaration is uniqued based only on its ``linkageName:`` and
+``scope:``.
 
 .. code-block:: text
 
@@ -5789,9 +5790,9 @@ and ``scope:``.
 }
 
 !0 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
-file: !2, line: 7, type: !3, isLocal: true,
-isDefinition: true, scopeLine: 8,
-containingType: !4,
+file: !2, line: 7, type: !3,
+

[PATCH] D143921: [debug-info][codegen] Prevent creation of self-referential SP node

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG997dc7e00f49: [debug-info][codegen] Prevent creation of 
self-referential SP node (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143921

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/docs/LangRef.rst
  llvm/lib/IR/Verifier.cpp
  llvm/test/Verifier/disubprogram_declaration.ll


Index: llvm/test/Verifier/disubprogram_declaration.ll
===
--- /dev/null
+++ llvm/test/Verifier/disubprogram_declaration.ll
@@ -0,0 +1,17 @@
+; RUN: llvm-as -disable-output <%s 2>&1| FileCheck %s
+
+declare !dbg !12 i32 @declared_only()
+
+!llvm.module.flags = !{!2}
+!llvm.dbg.cu = !{!5}
+
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !6, 
producer: "clang", emissionKind: FullDebug)
+!6 = !DIFile(filename: "a.cpp", directory: "/")
+!7 = !{}
+!11 = !DISubroutineType(types: !7)
+
+!12 = !DISubprogram(name: "declared_only", scope: !6, file: !6, line: 2, type: 
!11, spFlags: DISPFlagOptimized, retainedNodes: !7, declaration: !13)
+!13 = !DISubprogram(name: "declared_only", scope: !6, file: !6, line: 2, type: 
!11, spFlags: DISPFlagOptimized, retainedNodes: !7)
+; CHECK: subprogram declaration must not have a declaration field
+; CHECK: warning: ignoring invalid debug info
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1400,6 +1400,8 @@
   } else {
 // Subprogram declarations (part of the type hierarchy).
 CheckDI(!Unit, "subprogram declarations must not have a compile unit", &N);
+CheckDI(!N.getRawDeclaration(),
+"subprogram declaration must not have a declaration field");
   }
 
   if (auto *RawThrownTypes = N.getRawThrownTypes()) {
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -5776,11 +5776,12 @@
 
 .. _DISubprogramDeclaration:
 
-When ``isDefinition: false``, subprograms describe a declaration in the type
-tree as opposed to a definition of a function.  If the scope is a composite
-type with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``,
-then the subprogram declaration is uniqued based only on its ``linkageName:``
-and ``scope:``.
+When ``spFlags: DISPFlagDefinition`` is not present, subprograms describe a
+declaration in the type tree as opposed to a definition of a function. In this
+case, the ``declaration`` field must be empty. If the scope is a composite type
+with an ODR ``identifier:`` and that does not set ``flags: DIFwdDecl``, then
+the subprogram declaration is uniqued based only on its ``linkageName:`` and
+``scope:``.
 
 .. code-block:: text
 
@@ -5789,9 +5790,9 @@
 }
 
 !0 = distinct !DISubprogram(name: "foo", linkageName: "_Zfoov", scope: !1,
-file: !2, line: 7, type: !3, isLocal: true,
-isDefinition: true, scopeLine: 8,
-containingType: !4,
+file: !2, line: 7, type: !3,
+spFlags: DISPFlagDefinition | 
DISPFlagLocalToUnit,
+scopeLine: 8, containingType: !4,
 virtuality: DW_VIRTUALITY_pure_virtual,
 virtualIndex: 10, flags: DIFlagPrototyped,
 isOptimized: true, unit: !5, templateParams: 
!6,
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4225,10 +4225,9 @@
 
   llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D);
   llvm::DISubroutineType *STy = getOrCreateFunctionType(D, FnType, Unit);
-  llvm::DISubprogram *SP =
-  DBuilder.createFunction(FDContext, Name, LinkageName, Unit, LineNo, STy,
-  ScopeLine, Flags, SPFlags, TParamsArray.get(),
-  getFunctionDeclaration(D), nullptr, Annotations);
+  llvm::DISubprogram *SP = DBuilder.createFunction(
+  FDContext, Name, LinkageName, Unit, LineNo, STy, ScopeLine, Flags,
+  SPFlags, TParamsArray.get(), nullptr, nullptr, Annotations);
 
   // Preserve btf_decl_tag attributes for parameters of extern functions
   // for BPF target. The parameters created in this loop are attached as


Index: llvm/test/Verifier/disubprogram_declaration.ll
===
--- /dev/null
+++ llvm/test/Verifier/disubprogram_declaration.ll
@@ -0,0 +1,17 @@
+; RUN: llvm-as -disable-output <%s 2>&1| F

[PATCH] D115103: Leak Sanitizer port to Windows

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

@clemenswasser We hit this once in a while, when Microsoft update their 
libraries. But I'm surprised about the CC at the beginning. How can this issue 
be reproduced? Can you show a disassembly dump from the VS debugger, around the 
address of the function that fails?


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

https://reviews.llvm.org/D115103

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

[PATCH] D144422: [clang-tidy] update docs for new hungarian identifier-naming types (unsigned char and void)

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau created this revision.
amurzeau added reviewers: njames93, alexfh, kazu, dougpuob, aaron.ballman, 
carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
amurzeau requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Since 37e6a4f9496c8e35efc654d7619a79d6dbb72f99 
, `void` 
and
`unsigned char` were added to primitive types for hungarian notation.

This commit adds them to the documentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144422

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
@@ -2541,15 +2541,15 @@
 doubled  unsigned short int usiULONG   
ul
 char  c  unsigned short us ULONG32 
ul32
 bool  b  unsigned int   ui ULONG64 
ul64
-_Bool b  unsigned   u  
ULONGLONG   ull
-int   i  long long int  lliHANDLE  
h
-size_tn  long doubleld INT 
i
-short s  long long  ll INT8
i8
-signedi  long int   li INT16   
i16
-unsigned  u  long   l  INT32   
i32
-long  l  ptrdiff_t  p  INT64   
i64
-long long ll   UINT
ui
-unsigned long ul   UINT8   
u8
+_Bool b  unsigned char  uc 
ULONGLONG   ull
+int   i  unsigned   u  HANDLE  
h
+size_tn  long long int  lliINT 
i
+short s  long doubleld INT8
i8
+signedi  long long  ll INT16   
i16
+unsigned  u  long int   li INT32   
i32
+long  l  long   l  INT64   
i64
+long long ll ptrdiff_t  p  UINT
ui
+unsigned long ul void   *none* UINT8   
u8
 long double   ld   UINT16  
u16
 ptrdiff_t pUINT32  
u32
 wchar_t   wc   UINT64  
u64


Index: clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
@@ -2541,15 +2541,15 @@
 doubled  unsigned short int usiULONG   ul
 char  c  unsigned short us ULONG32 ul32
 bool  b  unsigned int   ui ULONG64 ul64
-_Bool b  unsigned   u  ULONGLONG   ull
-int   i  long long int  lliHANDLE  h
-size_tn  long doubleld INT i
-short s  long long  ll INT8i8
-signedi  long int   li INT16   i16
-unsigned  u  long   l  INT32   i32
-long  l  ptrdiff_t  p  INT64   i64
-long long ll   UINTui
-unsigned long ul   UINT8   u8
+_Bool b  unsigned char  uc ULONGLONG   ull
+int   i  unsigned   u  HANDLE  h
+size_tn  long long int  lliINT i
+short s  long doubleld INT8i8
+signedi  long long  ll INT16   i16

[PATCH] D144422: [clang-tidy] update docs for new hungarian identifier-naming types (unsigned char and void)

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau added a comment.

I don't have commit access to the repo, so please commit this for me when 
accepted. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144422

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

[PATCH] D143825: [clang-format] Put ports on separate lines in Verilog module headers

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2665
+if (Style.isVerilog() && Precedence == prec::Comma &&
+VerilogFirstOfType != nullptr) {
+  addFakeParenthesis(VerilogFirstOfType, prec::Comma);

owenpan wrote:
> sstwcw wrote:
> > owenpan wrote:
> > > And other places as well.
> > @HazardyKnusperkeks Do you agree?  I remember you once said you preferred 
> > the `nullptr` style.
> See D144355. Not using `nullptr` in conditionals was one of the first LLVM 
> styles I had to get used to when I started contributing.
Apart from Owens comment about the style guide. My personal preference is 
without `nullptr`, but I don't know if I maybe said otherwise sometime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143825

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

[PATCH] D115103: Leak Sanitizer port to Windows

[Clemens Wasser via Phabricator via cfe-commits]

clemenswasser added a comment.

@aganea You can find my current changes on my D115103 branch 
. I am using the 
currently failing `use_stacks.cpp` test.
Something like this should make debugging of this issue possible:

  > cd llvm-project
  > cmake -G Ninja -B build -D CMAKE_BUILD_TYPE=Debug 
-DLLVM_ENABLE_PROJECTS=clang;compiler-rt
  > cmake --build build -t check-lsan
  > cp 
build\projects\compiler-rt\test\lsan\X86_64LsanConfig\TestCases\Output\use_stacks.cpp.tmp
 
build\projects\compiler-rt\test\lsan\X86_64LsanConfig\TestCases\Output\use_stacks.cpp.exe
  > devenv /debugexe 
build\projects\compiler-rt\test\lsan\X86_64LsanConfig\TestCases\Output\use_stacks.cpp.exe

You should see that the `LSAN_MAYBE_INTERCEPT_CREATE_THREAD` in 
`__lsan::InitializeInterceptors` fails because of the wrong instruction read of 
the `CreateThread` function address in `GetInstructionSize` as described above.


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

https://reviews.llvm.org/D115103

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

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

@nridge @kadircet please take another look, all tests now pass. As for the 
location of inserting driver mode please see my previous comment about MSVC CL 
check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143436

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

[PATCH] D143301: [flang] Handle unsupported warning flags

[Ethan Luis McDonough via Phabricator via cfe-commits]

elmcdonough updated this revision to Diff 498944.
elmcdonough added a comment.

Clang format + documentation update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143301

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/docs/FlangDriver.md
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/werror-all.f90
  flang/test/Driver/wextra-ok.f90

Index: flang/test/Driver/wextra-ok.f90
===
--- /dev/null
+++ flang/test/Driver/wextra-ok.f90
@@ -0,0 +1,11 @@
+! Ensure that supplying -Wextra into flang-new does not raise error
+! The first check should be changed if -Wextra is implemented
+
+! RUN: %flang -std=f2018 -Wextra %s 2>&1 | FileCheck %s --check-prefix=CHECK-OK
+! RUN: not %flang -std=f2018 -Wblah -Wextra %s 2>&1 | FileCheck %s --check-prefix=WRONG
+
+! CHECK-OK: The warning option '-Wextra' is not supported
+! WRONG: Only `-Werror` is supported currently.
+
+program wextra_ok
+end program wextra_ok
Index: flang/test/Driver/werror-all.f90
===
--- /dev/null
+++ flang/test/Driver/werror-all.f90
@@ -0,0 +1,13 @@
+! Ensures that -Werror is read regardless of whether or not other -W
+! flags are present in the CLI args
+
+! RUN: not %flang -std=f2018 -Werror -Wextra %s 2>&1 | FileCheck %s --check-prefix=WRONG 
+! RUN: %flang -std=f2018 -Wextra -Wall %s 2>&1 | FileCheck %s --check-prefix=CHECK-OK
+
+! WRONG: Semantic errors in
+! CHECK-OK: FORALL index variable
+
+program werror_check_all
+  integer :: a(3)
+  forall (j=1:n) a(i) = 1
+end program werror_check_all
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -604,14 +604,17 @@
   // TODO: Currently throws a Diagnostic for anything other than -W,
   // this has to change when other -W's are supported.
   if (args.hasArg(clang::driver::options::OPT_W_Joined)) {
-if (args.getLastArgValue(clang::driver::options::OPT_W_Joined)
-.equals("error")) {
-  res.setWarnAsErr(true);
-} else {
-  const unsigned diagID =
-  diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
-"Only `-Werror` is supported currently.");
-  diags.Report(diagID);
+const auto &wArgs =
+args.getAllArgValues(clang::driver::options::OPT_W_Joined);
+for (const auto &wArg : wArgs) {
+  if (wArg == "error") {
+res.setWarnAsErr(true);
+  } else {
+const unsigned diagID =
+diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
+  "Only `-Werror` is supported currently.");
+diags.Report(diagID);
+  }
 }
   }
 
Index: flang/docs/FlangDriver.md
===
--- flang/docs/FlangDriver.md
+++ flang/docs/FlangDriver.md
@@ -581,6 +581,10 @@
 GCC/GFortran will also set flush-to-zero mode: linking `crtfastmath.o`, the same
 as Flang.
 
+The only GCC/GFortran warning option currently supported is `-Werror`.  Passing
+any unsupported GCC/GFortran warning flags into Flang's compiler driver will
+result in warnings being emitted.
+
 ### Comparison with nvfortran
 nvfortran defines `-fast` as
 `-O2 -Munroll=c:1 -Mnoframe -Mlre -Mpre -Mvect=simd -Mcache_align -Mflushz -Mvect`.
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -328,6 +328,13 @@
 A->render(Args, CmdArgs);
   }
 
+  // Remove any unsupported gfortran diagnostic options
+  for (const Arg *A : Args.filtered(options::OPT_flang_ignored_w_Group)) {
+A->claim();
+D.Diag(diag::warn_drv_unsupported_diag_option_for_flang)
+<< A->getOption().getName();
+  }
+
   // Optimization level for CodeGen.
   if (const Arg *A = Args.getLastArg(options::OPT_O_Group)) {
 if (A->getOption().matches(options::OPT_O4)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -235,6 +235,10 @@
 def clang_ignored_m_Group : OptionGroup<"">,
   Group, Flags<[Ignored]>;
 
+// Unsupported flang groups
+def flang_ignored_w_Group : OptionGroup<"">,
+  Group, Flags<[FlangOnlyOption, Ignored]>;
+
 // Group for clang options in the process of deprecation.
 // Please include the version that deprecated the flag as comment to allow
 // easier garbage collection.
@@ -4874,6 +4878,10 @@
   def fno_#NAME : Flag<["-"], "fno-"#name>;
 }
 
+multiclass FlangIgnoredDiagOpt {
+  def unsu

[PATCH] D144429: [clang-tidy] Add bugprone-chained-comparison check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL added a comment.
PiotrZSL published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Note that looks like there is currently in implementation new warning 
-Wcomparison-op-parentheses under D142800 , 
that warning currently handle only chained comparison by using BinaryOperator, 
not by using CXXOperatorCallExpr.
Scope of "-Wcomparison-op-parentheses" and whatever that warning will be 
delivered in LLVM 17 will determinate future of this check.
This check is feature-complete.


Check that flags chained comparison expressions,
such as a < b < c or a == b == c, which may have
unintended behavior due to implicit operator
associativity.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144429

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ChainedComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ChainedComparisonCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/chained-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/chained-comparison.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s bugprone-chained-comparison %t
+
+void badly_chained_1(int x, int y, int z)
+{
+bool result = x < y < z;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a < b < c' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_2(int x, int y, int z)
+{
+bool result = x <= y <= z;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a <= b <= c' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_3(int x, int y, int z)
+{
+bool result = x > y > z;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a > b > c' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_4(int x, int y, int z)
+{
+bool result = x >= y >= z;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a >= b >= c' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_5(int x, int y, int z)
+{
+bool result = x == y != z;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a == b != c' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_6(bool x, bool y, bool z)
+{
+bool result = x != y == z;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a != b == c' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_multiple(bool a, bool b, bool c, bool d, bool e, bool f, bool g, bool h)
+{
+bool result = a == b == c == d == e == f == g == h;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: chained comparison 'a == b == c == d == e == f == g == h' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+
+void badly_chained_limit(bool v[29])
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: chained comparison 'a == b == c == d == e == f == g == h == i == j == k == l == m == n == o == p == q == r == s == t == u == v == w == x == y == z ...' may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions [bugprone-chained-comparison]
+bool result = v[0] == v[1] == v[2] == v[3] == v[4] == v[5] == v[6] == v[7] ==
+  v[8] == v[9] == v[10] == v[11] == v[12] == v[13] == v[14] ==
+  v[15] == v[16] == v[17] == v[18] == v[19] == v[20] == v[21] ==
+  v[22] == v[23] == v[24] == v[25] == v[26] == v[27] == v[28];
+
+}
+
+void badly_chained_parens2(int x, int y, int z,

[PATCH] D144431: [clang-tidy] Fix readability-identifer-naming Hungarian CString options

[Alexis Murzeau via Phabricator via cfe-commits]

amurzeau created this revision.
amurzeau added reviewers: njames93, alexfh, kazu, dougpuob, aaron.ballman, 
carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
amurzeau requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang-tools-extra.

When reading readability-identifier-naming.HungarianNotation.CString
options, correctly use the type string stored in CStr.second instead of
the option name (CStr.first) as the HNOption.CString map key.

This will make CString options really working and properly parsed by the
checker.

Also adjust identifier-naming-hungarian-notation-cfgfile test with
changes for all configs in .clang-tidy to ensure they are really taken
into account.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144431

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -57,159 +57,159 @@
 public:
   static int ClassMemberCase;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for class member 'ClassMemberCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  static int iClassMemberCase;
+  // CHECK-FIXES: {{^}}  static int custiClassMemberCase;
 
   char const ConstantMemberCase = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'ConstantMemberCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  char const cConstantMemberCase = 0;
+  // CHECK-FIXES: {{^}}  char const custcConstantMemberCase = 0;
 
   void MyFunc1(const int ConstantParameterCase);
   // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for constant parameter 'ConstantParameterCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  void MyFunc1(const int iConstantParameterCase);
+  // CHECK-FIXES: {{^}}  void MyFunc1(const int custiConstantParameterCase);
 
   void MyFunc2(const int* ConstantPointerParameterCase);
   // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: invalid case style for pointer parameter 'ConstantPointerParameterCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  void MyFunc2(const int* piConstantPointerParameterCase);
+  // CHECK-FIXES: {{^}}  void MyFunc2(const int* custpcustiConstantPointerParameterCase);
 
   static constexpr int ConstexprVariableCase = 123;
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: invalid case style for constexpr variable 'ConstexprVariableCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  static constexpr int iConstexprVariableCase = 123;
+  // CHECK-FIXES: {{^}}  static constexpr int custiConstexprVariableCase = 123;
 };
 
 const int GlobalConstantCase = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global constant 'GlobalConstantCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}const int iGlobalConstantCase = 0;
+// CHECK-FIXES: {{^}}const int custiGlobalConstantCase = 0;
 
 const int* GlobalConstantPointerCase = nullptr;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for global pointer 'GlobalConstantPointerCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}const int* piGlobalConstantPointerCase = nullptr;
+// CHECK-FIXES: {{^}}const int* custpcustiGlobalConstantPointerCase = nullptr;
 
 int* GlobalPointerCase = nullptr;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global pointer 'GlobalPointerCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}int* piGlobalPointerCase = nullptr;
+// CHECK-FIXES: {{^}}int* custpcustiGlobalPointerCase = nullptr;
 
 int GlobalVariableCase = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'GlobalVariableCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}int iGlobalVariableCase = 0;
+// CHECK-FIXES: {{^}}int custiGlobalVariableCase = 0;
 
 void Func1(){
   int const LocalConstantCase = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for local constant 'LocalConstantCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  int const iLocalConstantCase = 3;
+  // CHECK-FIXES: {{^}}  int const custiLocalConstantCase = 3;
 
   unsigned const ConstantCase = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'ConstantCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  unsigned const uConstantCa

[clang] 4b09cb2 - [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2023-02-20T18:11:24-05:00
New Revision: 4b09cb2b16ebac179264b2bf5d99d19f363f24b8

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

LOG: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

Add a member function isPPC64ELFv2ABI() to determine what ABI is used on the
64-bit PowerPC big endian operating environment.

Reviewed By: nemanjai, dim, pkubaj

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

Added: 


Modified: 
clang/lib/Basic/Targets/PPC.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/ppc-abi.c
llvm/include/llvm/TargetParser/Triple.h
llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
llvm/test/CodeGen/PowerPC/pr47373.ll

Removed: 




diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 79f5d6e8c720b..27fef6a2d5d47 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -427,7 +427,10 @@ class LLVM_LIBRARY_VISIBILITY PPC64TargetInfo : public 
PPCTargetInfo {
   ABI = "elfv2";
 } else {
   DataLayout = "E-m:e-i64:64-n32:64";
-  ABI = "elfv1";
+  if (Triple.isPPC64ELFv2ABI())
+ABI = "elfv2";
+  else
+ABI = "elfv1";
 }
 
 if (Triple.isOSFreeBSD() || Triple.isOSOpenBSD() || Triple.isMusl()) {

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 31c9c1a37f09c..3703a1e78d8b2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2006,8 +2006,7 @@ void Clang::AddPPCTargetArgs(const ArgList &Args,
   if (T.isOSBinFormatELF()) {
 switch (getToolChain().getArch()) {
 case llvm::Triple::ppc64: {
-  if ((T.isOSFreeBSD() && T.getOSMajorVersion() >= 13) ||
-  T.isOSOpenBSD() || T.isMusl())
+  if (T.isPPC64ELFv2ABI())
 ABIName = "elfv2";
   else
 ABIName = "elfv1";

diff  --git a/clang/test/Driver/ppc-abi.c b/clang/test/Driver/ppc-abi.c
index a3840c655011d..cc07b084132f1 100644
--- a/clang/test/Driver/ppc-abi.c
+++ b/clang/test/Driver/ppc-abi.c
@@ -20,6 +20,7 @@
 // RUN: %clang -target powerpc64-unknown-freebsd12 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64-unknown-freebsd14 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
+// RUN: %clang -target powerpc64-unknown-freebsd %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s
 // RUN: %clang -target powerpc64-linux-musl %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s

diff  --git a/llvm/include/llvm/TargetParser/Triple.h 
b/llvm/include/llvm/TargetParser/Triple.h
index 8d600989c8cf5..59513fa2f2062 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -882,6 +882,14 @@ class Triple {
 return getArch() == Triple::ppc64 || getArch() == Triple::ppc64le;
   }
 
+  /// Tests whether the target 64-bit PowerPC big endian ABI is ELFv2.
+  bool isPPC64ELFv2ABI() const {
+return (getArch() == Triple::ppc64 &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::OpenBSD || isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 

diff  --git a/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp 
b/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
index 2944736937eb8..683a84e86c2dc 100644
--- a/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ b/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -237,7 +237,10 @@ static PPCTargetMachine::PPCABI computeTargetABI(const 
Triple &TT,
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
   case Triple::ppc64:
-return PPCTargetMachine::PPC_ABI_ELFv1;
+if (TT.isPPC64ELFv2ABI())
+  return PPCTargetMachine::PPC_ABI_ELFv2;
+else
+  return PPCTargetMachine::PPC_ABI_ELFv1;
   default:
 return PPCTargetMachine::PPC_ABI_UNKNOWN;
   }

diff  --git a/llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll 
b/llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
index 8b1cf6b588210..d418194b338f0 100644
--- a/llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
+++ b/llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
@@ -5,9 +5,14 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu 
-target-abi elfv1 < %s | FileCheck %s -check-

[PATCH] D144321: [PowerPC] Correctly use ELFv2 ABI on all OS's that use the ELFv2 ABI

[Brad Smith via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b09cb2b16eb: [PowerPC] Correctly use ELFv2 ABI on all 
OS's that use the ELFv2 ABI (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144321

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ppc-abi.c
  llvm/include/llvm/TargetParser/Triple.h
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
  llvm/test/CodeGen/PowerPC/pr47373.ll

Index: llvm/test/CodeGen/PowerPC/pr47373.ll
===
--- llvm/test/CodeGen/PowerPC/pr47373.ll
+++ llvm/test/CodeGen/PowerPC/pr47373.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=powerpc64-unknown-freebsd13.0 -verify-machineinstrs \
+; RUN: llc -mtriple=powerpc64-unknown-freebsd12.0 -verify-machineinstrs \
 ; RUN:   -mcpu=ppc64 -ppc-asm-full-reg-names < %s | FileCheck %s
 @a = local_unnamed_addr global ptr null, align 8
 
Index: llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
===
--- llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
+++ llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
@@ -5,9 +5,14 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
-; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-linux-musl < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-openbsd < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
 ; CHECK-ELFv2: .abiversion 2
 ; CHECK-ELFv1-NOT: .abiversion 2
Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -237,7 +237,10 @@
   case Triple::ppc64le:
 return PPCTargetMachine::PPC_ABI_ELFv2;
   case Triple::ppc64:
-return PPCTargetMachine::PPC_ABI_ELFv1;
+if (TT.isPPC64ELFv2ABI())
+  return PPCTargetMachine::PPC_ABI_ELFv2;
+else
+  return PPCTargetMachine::PPC_ABI_ELFv1;
   default:
 return PPCTargetMachine::PPC_ABI_UNKNOWN;
   }
Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -882,6 +882,14 @@
 return getArch() == Triple::ppc64 || getArch() == Triple::ppc64le;
   }
 
+  /// Tests whether the target 64-bit PowerPC big endian ABI is ELFv2.
+  bool isPPC64ELFv2ABI() const {
+return (getArch() == Triple::ppc64 &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::OpenBSD || isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 
Index: clang/test/Driver/ppc-abi.c
===
--- clang/test/Driver/ppc-abi.c
+++ clang/test/Driver/ppc-abi.c
@@ -20,6 +20,7 @@
 // RUN: %clang -target powerpc64-unknown-freebsd12 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64-unknown-freebsd13 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64-unknown-freebsd14 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE %s
+// RUN: %clang -target powerpc64-unknown-freebsd %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE-PIE %s
 // RUN: %clang -target powerpc64-linux-musl %s -### 2>&1 | FileCheck --check-prefix=CHECK-ELFv2-BE-PIE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
==

[PATCH] D135495: [clang-tidy] handle exceptions properly `ExceptionAnalyzer`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Overall, I like the structure of this patch and the references back to the 
standard. But every time we compare type pointers, they might compare inequal 
when one of the types have sugar on it while the other does not. Please review 
all of those comparisons to see where do we need to get the canonical types 
instead, and add some tests with type aliases.




Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:66
+   CXXBasePath &) {
+if (BS->getType()->getAsCXXRecordDecl() == BaseClass &&
+BS->getAccessSpecifier() == AS_public) {

Will this work with typedefs and other sugar or do you need to get the 
canonical type here? I do not see test coverage for that scenario.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:84
+
+QualType getPointeeOrArrayElementQualType(QualType T) {
+  if (T->isAnyPointerType())

What about `Type::getPointeeOrArrayElementType`?


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

https://reviews.llvm.org/D135495

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

[PATCH] D144341: [Driver][FreeBSD] Correct usage of --hash-style=both with triple without version

[Brad Smith via Phabricator via cfe-commits]

brad updated this revision to Diff 498980.
Herald added a subscriber: fedor.sergeev.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144341

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/test/Driver/constructors.c
  clang/test/Driver/freebsd.c


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -81,17 +81,15 @@
 // CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
 //
 // Check that the new linker flags are passed to FreeBSD
-// RUN: %clang --target=x86_64-pc-freebsd8 -m32 %s \
-// RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS8 %s
 // RUN: %clang --target=x86_64-pc-freebsd9 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
 // RUN: %clang --target=x86_64-pc-freebsd10.0 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
-// CHECK-LDFLAGS8-NOT: --hash-style=both
-// CHECK-LDFLAGS8: --enable-new-dtags
+// RUN: %clang --target=x86_64-pc-freebsd -m32 %s \
+// RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
 // CHECK-LDFLAGS9: --hash-style=both
 // CHECK-LDFLAGS9: --enable-new-dtags
 //
Index: clang/test/Driver/constructors.c
===
--- clang/test/Driver/constructors.c
+++ clang/test/Driver/constructors.c
@@ -84,7 +84,11 @@
 // RUN: %clang -### %s -fsyntax-only 2>&1   \
 // RUN: --target=i386-unknown-freebsd11 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s
-
+//
+// RUN: %clang -### %s -fsyntax-only 2>&1   \
+// RUN: --target=i386-unknown-freebsd \
+// RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
+//
 // RUN: %clang -### %s -fsyntax-only 2>&1   \
 // RUN: --target=i386-unknown-freebsd12 \
 // RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -175,11 +175,8 @@
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld-elf.so.1");
 }
-const llvm::Triple &T = ToolChain.getTriple();
-if (T.getOSMajorVersion() >= 9) {
-  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc || 
T.isX86())
-CmdArgs.push_back("--hash-style=both");
-}
+if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc || T.isX86())
+  CmdArgs.push_back("--hash-style=both");
 CmdArgs.push_back("--enable-new-dtags");
   }
 
@@ -404,9 +401,10 @@
 }
 
 unsigned FreeBSD::GetDefaultDwarfVersion() const {
-  if (getTriple().getOSMajorVersion() < 12)
-return 2;
-  return 4;
+  unsigned Major = getTriple().getOSMajorVersion();
+  if (Major >= 12 || Major == 0)
+return 4;
+  return 2;
 }
 
 void FreeBSD::AddClangSystemIncludeArgs(
@@ -550,8 +548,10 @@
 void FreeBSD::addClangTargetOptions(const ArgList &DriverArgs,
 ArgStringList &CC1Args,
 Action::OffloadKind) const {
+  const llvm::Triple &T = getToolChain().getTriple();
   if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
   options::OPT_fno_use_init_array,
-  getTriple().getOSMajorVersion() >= 12))
+  (T.getOSMajorVersion().empty() ||
+   T.getOSMajorVersion() >= 12)))
 CC1Args.push_back("-fno-use-init-array");
 }


Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -81,17 +81,15 @@
 // CHECK-RV64I-LD: ld{{.*}}" {{.*}} "-m" "elf64lriscv"
 //
 // Check that the new linker flags are passed to FreeBSD
-// RUN: %clang --target=x86_64-pc-freebsd8 -m32 %s \
-// RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS8 %s
 // RUN: %clang --target=x86_64-pc-freebsd9 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
 // RUN: %clang --target=x86_64-pc-freebsd10.0 -m32 %s \
 // RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
-// CHECK-LDFLAGS8-NOT: --hash-style=both
-// CHECK-LDFLAGS8: --enable-new-dtags
+// RUN: %clang --target=x86_64-pc-freebsd -m32 %s \
+// RUN:   --sysroot=%S/Inputs/multiarch_freebsd64_tree -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LDFLAGS9 %s
 // CHECK-LDFLAGS9: --ha

[PATCH] D144444: [PowerPC] Use member function to determine PowerPC Secure PLT

[Brad Smith via Phabricator via cfe-commits]

brad created this revision.
brad added a reviewer: nemanjai.
brad added a project: LLVM.
Herald added subscribers: steven.zhang, shchenz, kbarton, hiraditya.
Herald added a project: All.
brad requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Use member function to determine PowerPC Secure PLT. Less bits to keep in sync.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D14

Files:
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  llvm/include/llvm/TargetParser/Triple.h
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp


Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -106,9 +106,7 @@
   if (IsPPC64 && has64BitSupport())
 Use64BitRegs = true;
 
-  if ((TargetTriple.isOSFreeBSD() && TargetTriple.getOSMajorVersion() >= 13) ||
-  TargetTriple.isOSNetBSD() || TargetTriple.isOSOpenBSD() ||
-  TargetTriple.isMusl())
+  if ((TargetTriple.isPPCSecurePlt())
 IsSecurePlt = true;
 
   if (HasSPE && IsPPC64)
Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -890,6 +890,15 @@
 getOS() == Triple::OpenBSD || isMusl()));
   }
 
+  /// Tests whether the target 32-bit PowerPC uses Secure PLT.
+  bool isPPCSecurePlt() const {
+return ((getArch() == Triple::ppc || getArch() == Triple::ppcle) &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::NetBSD || getOS() == Triple::OpenBSD ||
+isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -119,8 +119,7 @@
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if ((Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 13) ||
-  Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
+  if (Triple.isPPCSecurePlt())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;


Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -106,9 +106,7 @@
   if (IsPPC64 && has64BitSupport())
 Use64BitRegs = true;
 
-  if ((TargetTriple.isOSFreeBSD() && TargetTriple.getOSMajorVersion() >= 13) ||
-  TargetTriple.isOSNetBSD() || TargetTriple.isOSOpenBSD() ||
-  TargetTriple.isMusl())
+  if ((TargetTriple.isPPCSecurePlt())
 IsSecurePlt = true;
 
   if (HasSPE && IsPPC64)
Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -890,6 +890,15 @@
 getOS() == Triple::OpenBSD || isMusl()));
   }
 
+  /// Tests whether the target 32-bit PowerPC uses Secure PLT.
+  bool isPPCSecurePlt() const {
+return ((getArch() == Triple::ppc || getArch() == Triple::ppcle) &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::NetBSD || getOS() == Triple::OpenBSD ||
+isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -119,8 +119,7 @@
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if ((Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 13) ||
-  Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
+  if (Triple.isPPCSecurePlt())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144444: [PowerPC] Use member function to determine PowerPC Secure PLT

[Brad Smith via Phabricator via cfe-commits]

brad updated this revision to Diff 499018.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D14

Files:
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  llvm/include/llvm/TargetParser/Triple.h
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp


Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -106,9 +106,7 @@
   if (IsPPC64 && has64BitSupport())
 Use64BitRegs = true;
 
-  if ((TargetTriple.isOSFreeBSD() && TargetTriple.getOSMajorVersion() >= 13) ||
-  TargetTriple.isOSNetBSD() || TargetTriple.isOSOpenBSD() ||
-  TargetTriple.isMusl())
+  if (TargetTriple.isPPCSecurePlt())
 IsSecurePlt = true;
 
   if (HasSPE && IsPPC64)
Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -890,6 +890,15 @@
 getOS() == Triple::OpenBSD || isMusl()));
   }
 
+  /// Tests whether the target 32-bit PowerPC uses Secure PLT.
+  bool isPPCSecurePlt() const {
+return ((getArch() == Triple::ppc || getArch() == Triple::ppcle) &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::NetBSD || getOS() == Triple::OpenBSD ||
+isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -119,8 +119,7 @@
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if ((Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 13) ||
-  Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
+  if (Triple.isPPCSecurePlt())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;


Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -106,9 +106,7 @@
   if (IsPPC64 && has64BitSupport())
 Use64BitRegs = true;
 
-  if ((TargetTriple.isOSFreeBSD() && TargetTriple.getOSMajorVersion() >= 13) ||
-  TargetTriple.isOSNetBSD() || TargetTriple.isOSOpenBSD() ||
-  TargetTriple.isMusl())
+  if (TargetTriple.isPPCSecurePlt())
 IsSecurePlt = true;
 
   if (HasSPE && IsPPC64)
Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -890,6 +890,15 @@
 getOS() == Triple::OpenBSD || isMusl()));
   }
 
+  /// Tests whether the target 32-bit PowerPC uses Secure PLT.
+  bool isPPCSecurePlt() const {
+return ((getArch() == Triple::ppc || getArch() == Triple::ppcle) &&
+((getOS() == Triple::FreeBSD &&
+(getOSMajorVersion() >= 13 || getOSVersion().empty())) ||
+getOS() == Triple::NetBSD || getOS() == Triple::OpenBSD ||
+isMusl()));
+  }
+
   /// Tests whether the target is 32-bit RISC-V.
   bool isRISCV32() const { return getArch() == Triple::riscv32; }
 
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -119,8 +119,7 @@
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if ((Triple.isOSFreeBSD() && Triple.getOSMajorVersion() >= 13) ||
-  Triple.isOSNetBSD() || Triple.isOSOpenBSD() || Triple.isMusl())
+  if (Triple.isPPCSecurePlt())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 499034.
ccotter added a comment.

- Ignore params of template type
- Add option for unnamed params


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -0,0 +1,323 @@
+// RUN: %check_clang_tidy -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: false},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,CXX14 -std=c++14 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: false},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,STRICT -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true}]}" -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffix=,UNNAMED -std=c++11 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \
+// RUN: -config="{CheckOptions: [{key: cppcoreguidelines-rvalue-reference-param-not-moved.StrictMode, value: true},{key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: false}]}" -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+template 
+struct remove_reference;
+
+template  struct remove_reference { typedef _Tp type; };
+template  struct remove_reference<_Tp&> { typedef _Tp type; };
+template  struct remove_reference<_Tp&&> { typedef _Tp type; };
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept;
+
+template 
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept;
+
+}
+// NOLINTEND
+
+struct Obj {
+  Obj();
+  Obj(const Obj&);
+  Obj& operator=(const Obj&);
+  Obj(Obj&&);
+  Obj& operator=(Obj&&);
+  void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void copies_object(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  Obj copy = o;
+}
+
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  // CHECK-MESSAGES: :[[@LINE-2]]:41: warning: rvalue reference parameter 'o2' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void never_moves_some_params(Obj&& o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: rvalue reference parameter 'o1' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  Obj other{std::move(o2)};
+}
+
+void never_moves_unnamed(Obj&&) {}
+// CHECK-MESSAGES-UNNAMED: :[[@LINE-1]]:31: warning: rvalue reference parameter '' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+void never_moves_mixed(Obj o1, Obj&& o2) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: r

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via cfe-commits]

kpdev42 added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3896
+  if (D->hasAttrs())
+ToField->setAttrs(D->getAttrs());
   ToField->setAccess(D->getAccess());

balazske wrote:
> The import of attributes is handled in function `ASTImporter::Import(Decl*)`. 
> This new line will probably copy all attributes, that may not work in all 
> cases dependent on the attribute types. This may interfere with the later 
> import of attributes, probably these will be duplicated. What was the need 
> for this line? (Simple attributes that do not have references to other nodes 
> could be copied at this place.)
Unfortunately it is too late to copy attribute in `ASTImporter::Import(Decl*)`, 
because field has already been added to record in a call to `ImportImpl 
(VisitFieldDecl/addDeclInternal)`. I've reused the current way of cloning 
attributes in `VisitFieldDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

  > If you add std::move you will get compilation error, if you add 
std::forward, everything will work fine. Simply Value& and && will evaluate to 
Value&, no rvalue reference here.

I agree that examining the template definition alone is not correct. In your 
original example with `SomeClass`, `T` is not a forwarding reference since `T` 
is not deduced in the constructor, so it's misleading to use `forward` in this 
context. `forward` "works" and leads to the behavior you are likely seeking in 
your `SomeClass` example, but not in the usual way the most readers would 
expect (the spirit of the CppCoreGuidelines is to provide certain restricted 
ideas as recommendations, and using `forward` in this way violates ES.56: "Flag 
when std::forward is applied to other than a forwarding reference."). Usually, 
I'd expect to see a class template on a type to have different specializations 
for `T` being a non-reference and another for `T` being a reference (e.g., 
`std::future`), or only allow `T` to be a reference or non-reference (e.g., 
`std::optional`), but not both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:45
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: rvalue reference parameter is 
never moved from inside the function body 
[cppcoreguidelines-rvalue-reference-param-not-moved]

ccotter wrote:
> carlosgalvezp wrote:
> > Could we add something like this too?
> > 
> > ```
> > template 
> > struct Foo
> > {
> >   void never_moves(T&& t) {}
> > };
> > ```
> > 
> > Here `t` is not a universal reference so the check should warn.
> Done in the `AClassTemplate::never_moves` test.
@carlosgalvezp Note I've changed the check to not flag `never_moves` based on 
more recent discussion. I do think this code should flag, although I'm a little 
torn. If `Foo` constrains `T` to be a reference, then `Foo::never_moves` 
is "OK", although I think it's still confusing as `never_moves` could instead 
accept `T&`.

I'll need to rethink this a bit, but I've never the test in place with a 
`FIXME` and brief comment explaining the situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp:45
+template 
+void never_moves_param_template(Obj&& o, T t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: rvalue reference parameter is 
never moved from inside the function body 
[cppcoreguidelines-rvalue-reference-param-not-moved]

ccotter wrote:
> ccotter wrote:
> > carlosgalvezp wrote:
> > > Could we add something like this too?
> > > 
> > > ```
> > > template 
> > > struct Foo
> > > {
> > >   void never_moves(T&& t) {}
> > > };
> > > ```
> > > 
> > > Here `t` is not a universal reference so the check should warn.
> > Done in the `AClassTemplate::never_moves` test.
> @carlosgalvezp Note I've changed the check to not flag `never_moves` based on 
> more recent discussion. I do think this code should flag, although I'm a 
> little torn. If `Foo` constrains `T` to be a reference, then 
> `Foo::never_moves` is "OK", although I think it's still confusing as 
> `never_moves` could instead accept `T&`.
> 
> I'll need to rethink this a bit, but I've never the test in place with a 
> `FIXME` and brief comment explaining the situation.
That should read "I've left the test in place"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569

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

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: efriedma, aaron.ballman, andrew.w.kaylor, kpn, 
spatel, uweigand.
Herald added a subscriber: StephenFan.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added a project: clang.

Allows us to handle expressions like -(a * b) + c

Based on the examples from D144366  that gcc 
seems to get.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D17

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  clang/test/CodeGen/fp-contract-pragma.cpp

Index: clang/test/CodeGen/fp-contract-pragma.cpp
===
--- clang/test/CodeGen/fp-contract-pragma.cpp
+++ clang/test/CodeGen/fp-contract-pragma.cpp
@@ -89,3 +89,54 @@
   #pragma STDC FP_CONTRACT ON
   return c - a * b;
 }
+
+float fp_contract_10(float a, float b, float c) {
+// CHECK: _Z14fp_contract_10fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) + c;
+}
+
+float fp_contract_11(float a, float b, float c) {
+// CHECK: _Z14fp_contract_11fff
+// CHECK: fneg float %a
+// CHECK: fneg float %c
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) - c;
+}
+
+float fp_contract_12(float a, float b, float c) {
+// CHECK: _Z14fp_contract_12fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c + -(a * b);
+}
+
+float fp_contract_13(float a, float b, float c) {
+// CHECK: _Z14fp_contract_13fff
+// CHECK-NOT: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c - -(a * b);
+}
+
+float fp_contract_14(float a, float b, float c) {
+// CHECK: _Z14fp_contract_14fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return (d = -(a * b)) + c;
+}
+
+float fp_contract_15(float a, float b, float c) {
+// CHECK: _Z14fp_contract_15fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return -(d = (a * b)) + c;
+}
Index: clang/test/CodeGen/constrained-math-builtins.c
===
--- clang/test/CodeGen/constrained-math-builtins.c
+++ clang/test/CodeGen/constrained-math-builtins.c
@@ -300,10 +300,17 @@
   f * f + f;
   (double)f * f - f;
   (long double)-f * f + f;
+  -(f * f) - f;
+  f + -(f * f);
 
   // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call double @llvm.experimental.constrained.fmuladd.f64(double %{{.*}}, double %{{.*}}, double %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call x86_fp80 @llvm.experimental.constrained.fmuladd.f80(x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 };
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3734,8 +3734,6 @@
 static Value* buildFMulAdd(llvm::Instruction *MulOp, Value *Addend,
const CodeGenFunction &CGF, CGBuilderTy &Builder,
bool negMul, bool negAdd) {
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be set.");
-
   Value *MulOp0 = MulOp->getOperand(0);
   Value *MulOp1 = MulOp->getOperand(1);
   if (negMul)
@@ -3780,31 +3778,70 @@
   if (!op.FPFeatures.allowFPContractWithinStatement())
 return nullptr;
 
+  Value *LHS = op.LHS;
+  Value *RHS = op.RHS;
+
+  // Peek through fneg to look for fmul. Make sure fneg has no users, and that
+  // it is the only use of its operand.
+  bool NegLHS = false;
+  if (auto *LHSUnOp = dyn_cast(LHS)) {
+if (LHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+LHSUnOp->use_empty() && LHSUnOp->getOperand(0)->hasOneUse()) {
+  LHS = LHSUnOp->getOperand(0);
+  NegLHS = true;
+}
+  }
+
+  bool NegRHS = false;
+  if (auto *RHSUnOp = dyn_cast(RHS)) {
+if (RHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+RHSUnOp->use_empty() && RHSUnOp->getOperand(0)->hasOneUse()) {
+  RHS = RHSUnOp->getOperand(0);
+  NegRHS = true;
+}
+  }
+
 

[PATCH] D144317: [clang-format] Fix handling of TypeScript tuples with optional last member

[Taymon A. Beal via Phabricator via cfe-commits]

taymonbeal added a comment.

Can anyone please commit this? I don't have commit access. Thanks.

Name: Taymon A. Beal
Email: tay...@google.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144317

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

[clang-tools-extra] dc43f71 - [clang-tidy] update docs for new hungarian identifier-naming types (unsigned char and void)

[Carlos Galvez via cfe-commits]

Author: Alexis Murzeau
Date: 2023-02-21T07:37:42Z
New Revision: dc43f7107e2916035b3503a10977182b03203046

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

LOG: [clang-tidy] update docs for new hungarian identifier-naming types 
(unsigned char and void)

Since 37e6a4f9496c8e35efc654d7619a79d6dbb72f99, `void` and
`unsigned char` were added to primitive types for hungarian notation.

This commit adds them to the documentation.

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst 
b/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
index efb01fe95ef2d..3f98ed73dd953 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
@@ -2541,15 +2541,15 @@ float f  unsigned long  
ul LONG
 doubled  unsigned short int usiULONG   
ul
 char  c  unsigned short us ULONG32 
ul32
 bool  b  unsigned int   ui ULONG64 
ul64
-_Bool b  unsigned   u  
ULONGLONG   ull
-int   i  long long int  lliHANDLE  
h
-size_tn  long doubleld INT 
i
-short s  long long  ll INT8
i8
-signedi  long int   li INT16   
i16
-unsigned  u  long   l  INT32   
i32
-long  l  ptr
diff _t  p  INT64   i64
-long long ll   UINT
ui
-unsigned long ul   UINT8   
u8
+_Bool b  unsigned char  uc 
ULONGLONG   ull
+int   i  unsigned   u  HANDLE  
h
+size_tn  long long int  lliINT 
i
+short s  long doubleld INT8
i8
+signedi  long long  ll INT16   
i16
+unsigned  u  long int   li INT32   
i32
+long  l  long   l  INT64   
i64
+long long ll ptr
diff _t  p  UINTui
+unsigned long ul void   *none* UINT8   
u8
 long double   ld   UINT16  
u16
 ptr
diff _t pUINT32 
 u32
 wchar_t   wc   UINT64  
u64



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

[PATCH] D144422: [clang-tidy] update docs for new hungarian identifier-naming types (unsigned char and void)

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc43f7107e29: [clang-tidy] update docs for new hungarian 
identifier-naming types (unsigned… (authored by amurzeau, committed by 
carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144422

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
@@ -2541,15 +2541,15 @@
 doubled  unsigned short int usiULONG   
ul
 char  c  unsigned short us ULONG32 
ul32
 bool  b  unsigned int   ui ULONG64 
ul64
-_Bool b  unsigned   u  
ULONGLONG   ull
-int   i  long long int  lliHANDLE  
h
-size_tn  long doubleld INT 
i
-short s  long long  ll INT8
i8
-signedi  long int   li INT16   
i16
-unsigned  u  long   l  INT32   
i32
-long  l  ptrdiff_t  p  INT64   
i64
-long long ll   UINT
ui
-unsigned long ul   UINT8   
u8
+_Bool b  unsigned char  uc 
ULONGLONG   ull
+int   i  unsigned   u  HANDLE  
h
+size_tn  long long int  lliINT 
i
+short s  long doubleld INT8
i8
+signedi  long long  ll INT16   
i16
+unsigned  u  long int   li INT32   
i32
+long  l  long   l  INT64   
i64
+long long ll ptrdiff_t  p  UINT
ui
+unsigned long ul void   *none* UINT8   
u8
 long double   ld   UINT16  
u16
 ptrdiff_t pUINT32  
u32
 wchar_t   wc   UINT64  
u64


Index: clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/identifier-naming.rst
@@ -2541,15 +2541,15 @@
 doubled  unsigned short int usiULONG   ul
 char  c  unsigned short us ULONG32 ul32
 bool  b  unsigned int   ui ULONG64 ul64
-_Bool b  unsigned   u  ULONGLONG   ull
-int   i  long long int  lliHANDLE  h
-size_tn  long doubleld INT i
-short s  long long  ll INT8i8
-signedi  long int   li INT16   i16
-unsigned  u  long   l  INT32   i32
-long  l  ptrdiff_t  p  INT64   i64
-long long ll   UINTui
-unsigned long ul   UINT8   u8
+_Bool b  unsigned char  uc ULONGLONG   ull
+int   i  unsigned   u  HANDLE  h
+size_tn  long long int  lliINT i
+short s  long doubleld INT8i8
+signedi  long long  ll INT16   i16
+unsigned  u  long int   li INT32   i32
+long  l  long   l  INT64   i64
+long long ll ptrdiff_t  p  UINT  

[PATCH] D144431: [clang-tidy] Fix readability-identifer-naming Hungarian CString options

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Please document fix in the Release Notes, as people might have adopted to the 
buggy behavior and should know they need to change to the proper one.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy:118-125
+value:  custa
   - key:readability-identifier-naming.HungarianNotation.DerivedType.Pointer
-value:  p
+value:  custp
   - key:
readability-identifier-naming.HungarianNotation.DerivedType.FunctionPointer
-value:  fn
+value:  custfn
   - key:readability-identifier-naming.HungarianNotation.CString.CharPrinter
+value:  custsz

These changes generate a lot of review noise and don't seem to be related to 
the fix in the patch (which has to do with `HungarianNotation.General`. Please 
revert them and if wanted make another patch just for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144431

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

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:222
+  tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS);
+  tooling::addTargetAndModeForProgramName(Cmd, Cmd.front());
   auto &OptTable = clang::driver::getDriverOptTable();

DmitryPolukhin wrote:
> nridge wrote:
> > nridge wrote:
> > > The target needs to be added **after** the call to 
> > > `SystemIncludeExtractor` later in this function (this is what D138546 is 
> > > trying to fix). The reason is that `SystemIncludeExtractor` includes any 
> > > `--target` flag in the compiler driver being queried for system includes, 
> > > which may be gcc, which does not support `--target`.
> > (I guess we could make that change separately in D138546, but if we're 
> > changing the place where the `--target` is added in this patch, I figure we 
> > might as well move it directly to the desired place.)
> I think there are order problems here:
> - we need `--driver-mode=cl` injected here to make check on line #229 work as 
> expected
> - if we don't inject it driver mode here, command line edits won't see the 
> change; see how I modified test clangd/unittests/CompileCommandsTests.cpp 
> line #203, with D138546 edits were not applied properly to driver mode
> 
> I'll double check how it works on Windows but I expect that moving it after 
> SystemIncludeExtractor will break proper driver mode detection.
> I think there are order problems here:
> - we need `--driver-mode=cl` injected here to make check on line #229 work as 
> expected

Looking at the [line in 
question](https://searchfox.org/llvm/rev/0cbb8ec030e23c0e13331b5d54155def8c901b36/clang-tools-extra/clangd/CompileCommands.cpp#213),
 it calls `driver::getDriverMode()` which [falls back on extracting the driver 
mode](https://searchfox.org/llvm/rev/0cbb8ec030e23c0e13331b5d54155def8c901b36/clang/lib/Driver/Driver.cpp#6407)
 from the program name anyways -- so I think that should be fine.

> - if we don't inject it driver mode here, command line edits won't see the 
> change; see how I modified test clangd/unittests/CompileCommandsTests.cpp 
> line #203, with D138546 edits were not applied properly to driver mode

I'm not following the motivation for this test. Is there any real-world use 
case which requires the command line edits seeing the `--driver-mode` parameter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143436

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