[PATCH] D146715: [NVPTX] Enforce half type support is present for builtins

[Jakub Chlanda 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 rGae3c981aa4b8: [NVPTX] Enforce half type support is present 
for builtins (authored by jchlanda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146715

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
  llvm/include/llvm/IR/IntrinsicsNVVM.td

Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -583,7 +583,6 @@
   "_xorsign_abs_f16", "_ftz_xorsign_abs_f16", "_nan_xorsign_abs_f16",
   "_ftz_nan_xorsign_abs_f16"] in {
   def int_nvvm_f # operation # variant :
-ClangBuiltin,
 DefaultAttrsIntrinsic<[llvm_half_ty], [llvm_half_ty, llvm_half_ty],
   [IntrNoMem, IntrSpeculatable, Commutative]>;
 }
@@ -592,7 +591,6 @@
   "_ftz_nan_f16x2", "_xorsign_abs_f16x2", "_ftz_xorsign_abs_f16x2",
   "_nan_xorsign_abs_f16x2", "_ftz_nan_xorsign_abs_f16x2"] in {
   def int_nvvm_f # operation # variant :
-ClangBuiltin,
 DefaultAttrsIntrinsic<[llvm_v2f16_ty], [llvm_v2f16_ty, llvm_v2f16_ty],
   [IntrNoMem, IntrSpeculatable, Commutative]>;
 }
@@ -828,9 +826,9 @@
   DefaultAttrsIntrinsic<[llvm_float_ty], [llvm_float_ty], [IntrNoMem]>;
   def int_nvvm_ex2_approx_d : ClangBuiltin<"__nvvm_ex2_approx_d">,
   DefaultAttrsIntrinsic<[llvm_double_ty], [llvm_double_ty], [IntrNoMem]>;
-  def int_nvvm_ex2_approx_f16 : ClangBuiltin<"__nvvm_ex2_approx_f16">,
+  def int_nvvm_ex2_approx_f16 :
   DefaultAttrsIntrinsic<[llvm_half_ty], [llvm_half_ty], [IntrNoMem]>;
-  def int_nvvm_ex2_approx_f16x2 : ClangBuiltin<"__nvvm_ex2_approx_f16x2">,
+  def int_nvvm_ex2_approx_f16x2 :
   DefaultAttrsIntrinsic<[llvm_v2f16_ty], [llvm_v2f16_ty], [IntrNoMem]>;
 
   def int_nvvm_lg2_approx_ftz_f : ClangBuiltin<"__nvvm_lg2_approx_ftz_f">,
@@ -860,18 +858,16 @@
 
   foreach variant = ["_rn_f16", "_rn_ftz_f16", "_rn_sat_f16",
 "_rn_ftz_sat_f16", "_rn_relu_f16", "_rn_ftz_relu_f16"] in {
-def int_nvvm_fma # variant : ClangBuiltin,
-DefaultAttrsIntrinsic<[llvm_half_ty],
-  [llvm_half_ty, llvm_half_ty, llvm_half_ty],
-  [IntrNoMem, IntrSpeculatable]>;
+def int_nvvm_fma # variant : DefaultAttrsIntrinsic<[llvm_half_ty],
+  [llvm_half_ty, llvm_half_ty, llvm_half_ty],
+  [IntrNoMem, IntrSpeculatable]>;
   }
 
   foreach variant = ["_rn_f16x2", "_rn_ftz_f16x2", "_rn_sat_f16x2",
 "_rn_ftz_sat_f16x2", "_rn_relu_f16x2", "_rn_ftz_relu_f16x2"] in {
-def int_nvvm_fma # variant : ClangBuiltin,
-  DefaultAttrsIntrinsic<[llvm_v2f16_ty],
-[llvm_v2f16_ty, llvm_v2f16_ty, llvm_v2f16_ty],
-[IntrNoMem, IntrSpeculatable]>;
+def int_nvvm_fma # variant : DefaultAttrsIntrinsic<[llvm_v2f16_ty],
+  [llvm_v2f16_ty, llvm_v2f16_ty, llvm_v2f16_ty],
+  [IntrNoMem, IntrSpeculatable]>;
   }
 
   foreach variant = ["_rn_bf16", "_rn_relu_bf16"] in {
Index: clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
===
--- clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
+++ clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
@@ -1,21 +1,119 @@
 // REQUIRES: nvptx-registered-target
 //
 // RUN: not %clang_cc1 -fsyntax-only -ffp-contract=off -triple nvptx-unknown-unknown -target-cpu \
-// RUN:   sm_75 -target-feature +ptx70 -fcuda-is-device -x cuda -emit-llvm -o - %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHECK-ERROR %s
+// RUN:   sm_86 -target-feature +ptx72 -fcuda-is-device -x cuda -emit-llvm -o - %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK_ERROR %s
 
 #define __device__ __attribute__((device))
 typedef __fp16 __fp16v2 __attribute__((ext_vector_type(2)));
 
-__device__ void nvvm_ldg_ldu_native_half_types(const void *p) {
-  __nvvm_ldg_h((const __fp16 *)p);
-  __nvvm_ldg_h2((const __fp16v2 *)p);
+__device__ void nvvm_native_half_types(void *a, void*b, void*c, __fp16* out) {
+  __fp16v2 resv2 = {0, 0};
+  *out += __nvvm_ex2_approx_f16(*(__fp16 *)a);
+  resv2 = __nvvm_ex2_approx_f16x2(*(__fp16v2*)a);
 
-  __nvvm_ldu_h((const __fp16 *)p);
-  __nvvm_ldu_h2((const __fp16v2 *)p);
+  *out += __nvvm_fma_rn_relu_f16(*(__fp16*)a, *(__fp16*)b, *(__fp16*)c);
+  *out += __nvvm_fma_rn_ftz_relu_f16(*(__fp16*)a, *(__fp16*)b, *(__fp16 *)c);
+  resv2 += __nvvm_fma_rn_relu_f16x2(*(__fp16v2*)a, *(__fp16v2*)b, *(__fp16v2*)c);
+  resv2 += __nvvm_fma_rn_ftz_relu_f16x2(*(__fp16v2*)a, *(__fp16v2*)b, *(__fp16v2*)c);
+  *out += __nvvm_fma_rn_ftz_f16(*(__fp16*)a, *(__fp16*)b, *(__fp16*)c);
+  *out += __nvvm_fma_rn_sat_f16(*(__fp16*)a, *(__fp16*)b, *(__fp16*)c);
+  *out += __nvvm_fma_rn_ftz_sat_f16(*(__fp16*)a, *(__fp16*)b, *(__fp16*)c);
+  resv2

[clang] ae3c981 - [NVPTX] Enforce half type support is present for builtins

[Jakub Chlanda via cfe-commits]

Author: Jakub Chlanda
Date: 2023-03-28T08:48:10+02:00
New Revision: ae3c981aa4b85cfae6531ba50df7ad84feebe43c

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

LOG: [NVPTX] Enforce half type support is present for builtins

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
llvm/include/llvm/IR/IntrinsicsNVVM.td

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index c8112b0ea0ec0..f399b0770143a 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18162,32 +18162,63 @@ static NVPTXMmaInfo getNVPTXMmaInfo(unsigned 
BuiltinID) {
 #undef MMA_VARIANTS_B1_XOR
 }
 
+static Value *MakeLdgLdu(unsigned IntrinsicID, CodeGenFunction &CGF,
+ const CallExpr *E) {
+  Value *Ptr = CGF.EmitScalarExpr(E->getArg(0));
+  QualType ArgType = E->getArg(0)->getType();
+  clang::CharUnits Align = CGF.CGM.getNaturalPointeeTypeAlignment(ArgType);
+  llvm::Type *ElemTy = CGF.ConvertTypeForMem(ArgType->getPointeeType());
+  return CGF.Builder.CreateCall(
+  CGF.CGM.getIntrinsic(IntrinsicID, {ElemTy, Ptr->getType()}),
+  {Ptr, ConstantInt::get(CGF.Builder.getInt32Ty(), Align.getQuantity())});
+}
+
+static Value *MakeScopedAtomic(unsigned IntrinsicID, CodeGenFunction &CGF,
+   const CallExpr *E) {
+  Value *Ptr = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Type *ElemTy =
+  CGF.ConvertTypeForMem(E->getArg(0)->getType()->getPointeeType());
+  return CGF.Builder.CreateCall(
+  CGF.CGM.getIntrinsic(IntrinsicID, {ElemTy, Ptr->getType()}),
+  {Ptr, CGF.EmitScalarExpr(E->getArg(1))});
+}
+
+static Value *MakeHalfType(unsigned IntrinsicID, unsigned BuiltinID,
+   const CallExpr *E, CodeGenFunction &CGF) {
+  auto &C = CGF.CGM.getContext();
+  if (!(C.getLangOpts().NativeHalfType ||
+!C.getTargetInfo().useFP16ConversionIntrinsics())) {
+CGF.CGM.Error(E->getExprLoc(), C.BuiltinInfo.getName(BuiltinID).str() +
+   " requires native half type support.");
+return nullptr;
+  }
+
+  if (IntrinsicID == Intrinsic::nvvm_ldg_global_f ||
+  IntrinsicID == Intrinsic::nvvm_ldu_global_f)
+return MakeLdgLdu(IntrinsicID, CGF, E);
+
+  SmallVector Args;
+  auto *F = CGF.CGM.getIntrinsic(IntrinsicID);
+  auto *FTy = F->getFunctionType();
+  unsigned ICEArguments = 0;
+  ASTContext::GetBuiltinTypeError Error;
+  C.GetBuiltinType(BuiltinID, Error, &ICEArguments);
+  assert(Error == ASTContext::GE_None && "Should not codegen an error");
+  for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
+assert((ICEArguments & (1 << i)) == 0);
+auto *ArgValue = CGF.EmitScalarExpr(E->getArg(i));
+auto *PTy = FTy->getParamType(i);
+if (PTy != ArgValue->getType())
+  ArgValue = CGF.Builder.CreateBitCast(ArgValue, PTy);
+Args.push_back(ArgValue);
+  }
+
+  return CGF.Builder.CreateCall(F, Args);
+}
 } // namespace
 
-Value *
-CodeGenFunction::EmitNVPTXBuiltinExpr(unsigned BuiltinID, const CallExpr *E) {
-  auto HasHalfSupport = [&](unsigned BuiltinID) {
-auto &Context = getContext();
-return Context.getLangOpts().NativeHalfType ||
-   !Context.getTargetInfo().useFP16ConversionIntrinsics();
-  };
-  auto MakeLdgLdu = [&](unsigned IntrinsicID) {
-Value *Ptr = EmitScalarExpr(E->getArg(0));
-QualType ArgType = E->getArg(0)->getType();
-clang::CharUnits Align = CGM.getNaturalPointeeTypeAlignment(ArgType);
-llvm::Type *ElemTy = ConvertTypeForMem(ArgType->getPointeeType());
-return Builder.CreateCall(
-CGM.getIntrinsic(IntrinsicID, {ElemTy, Ptr->getType()}),
-{Ptr, ConstantInt::get(Builder.getInt32Ty(), Align.getQuantity())});
-  };
-  auto MakeScopedAtomic = [&](unsigned IntrinsicID) {
-Value *Ptr = EmitScalarExpr(E->getArg(0));
-llvm::Type *ElemTy =
-ConvertTypeForMem(E->getArg(0)->getType()->getPointeeType());
-return Builder.CreateCall(
-CGM.getIntrinsic(IntrinsicID, {ElemTy, Ptr->getType()}),
-{Ptr, EmitScalarExpr(E->getArg(1))});
-  };
+Value *CodeGenFunction::EmitNVPTXBuiltinExpr(unsigned BuiltinID,
+ const CallExpr *E) {
   switch (BuiltinID) {
   case NVPTX::BI__nvvm_atom_add_gen_i:
   case NVPTX::BI__nvvm_atom_add_gen_l:
@@ -18297,22 +18328,13 @@ CodeGenFunction::EmitNVPTXBuiltinExpr(unsigned 
BuiltinID, const CallExpr *E) {
 // PTX Interoperability section 2.2: "For a vector with an even number of
 // elements, its alignment is set to number of elements times the alignment
 // of its member: n*alignof(t)."
-return MakeLdgLdu

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

ChuanqiXu wrote:
> Can we try to move the logic to `CoroutineStmtBuilder`? That makes me feel 
> better. And it will be helpful to add a comment to tell that we're handling 
> the case the function body is function-try-block.
It looks like you didn't address the comments. Would you like to address it? I 
don't mind to address it later myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

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

[PATCH] D145545: [clang][Interp] Fix local variable (destructor) management in loop bodies

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:199-200
+bool ByteCodeStmtGen::visitUnscopedCompoundStmt(const Stmt *S) {
+  if (isa(S))
+return true;
+

aaron.ballman wrote:
> Errr, I'm surprised it isn't UB to call this with anything but a 
> `CompoundStmt` given the function name.
Yeah, I'm not too happy about that either. I'll see what I can do.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:209
+
+  return this->visitStmt(S);
+}

aaron.ballman wrote:
> It's a bit of a surprise that we visit the entire body of the compound 
> statement before we visit the compound statement itself. I was thinking the 
> compound statement could potentially have prologue work it needs to do, but 
> now I'm second-guessing that. But this design still feels a little bit off... 
> it's basically doing a post-order traversal just for this one node, and I 
> wonder if we want something more general like a `preVisitFoo()` followed by 
> `visitFoo()` followed by `postVisitFoo()` that applies to any AST node.
I think you're overthinking this, this just for the `!isa(S)` 
case (which IIRC happens at least for comma operator bin ops?)



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:324-328
+  LocalScope Scope(this);
+  if (!this->visitUnscopedCompoundStmt(Body))
 return false;
+
+  Scope.emitDestructors();

aaron.ballman wrote:
> `AutoScope` and some curly braces to delimit the scope object lifetime?
The problem is that we want to emit the destructors before the jump, but not 
destroy the scope. That should happen after the end label, when the loop is 
finished altogether so an `AutoScope` doesn't work.


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

https://reviews.llvm.org/D145545

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

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

In D140760#4226213 , @ccotter wrote:

> Should we handle that in a separate patch?

Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a subscriber: cjdb.
tbaeder added a comment.

Pinging @cjdb and @aaron.ballman for some feedback on the wording


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

https://reviews.llvm.org/D146358

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

[PATCH] D146678: Fix unexpected nullptr in ConceptSpecializationExpr's ArgsAsWritten field

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:843
   Expr *NewIDC = ConceptSpecializationExpr::Create(
-  C, CSE->getNamedConcept(), CSD, nullptr, CSE->isInstantiationDependent(),
-  CSE->containsUnexpandedParameterPack());
+  C, CSE->getNamedConcept(), CSE->getTemplateArgsAsWritten(), CSD, nullptr,
+  CSE->isInstantiationDependent(), CSE->containsUnexpandedParameterPack());

nit follow clang-tidy [bugprone-argument-comment 
convention](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146678

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

[PATCH] D146844: [clang-format] Handle '_' in ud-suffix for IntegerLiteralSeparator

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 508889.
owenpan added a comment.

Treats imaginary number suffixes (starting with an `i`) just like any 
user-defined suffixes (starting with an underscore). Also handles ud-suffixes 
(e.g. `_km` and `_Pa`) containing letters that can be part of a floating-point 
number.


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

https://reviews.llvm.org/D146844

Files:
  clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
  clang/unittests/Format/IntegerLiteralSeparatorTest.cpp


Index: clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
===
--- clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -49,7 +49,21 @@
 
   verifyFormat("o0 = 0;\n"
"o1 = 07;\n"
-   "o5 = 012345",
+   "o5 = 012345;",
+   Style);
+
+  verifyFormat("bi = 0b1'i;\n"
+   "dif = 1'234if;\n"
+   "hil = 0xA'BCil;",
+   "bi = 0b1i;\n"
+   "dif = 1234if;\n"
+   "hil = 0xABCil;",
+   Style);
+
+  verifyFormat("d = 5'678_km;\n"
+   "h = 0xD'EF_u16;",
+   "d = 5678_km;\n"
+   "h = 0xDEF_u16;",
Style);
 }
 
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -106,6 +106,12 @@
 (IsBase16 && SkipHex) || B == Base::Other) {
   continue;
 }
+if (Style.isCpp()) {
+  if (const auto Pos = Text.find_first_of("_i"); Pos != StringRef::npos) {
+Text = Text.substr(0, Pos);
+Length = Pos;
+  }
+}
 if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) ||
 (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
   continue;
@@ -116,7 +122,7 @@
   continue;
 }
 const auto Start = Text[0] == '0' ? 2 : 0;
-auto End = Text.find_first_of("uUlLzZn");
+auto End = Text.find_first_of("uUlLzZn", Start);
 if (End == StringRef::npos)
   End = Length;
 if (Start > 0 || End < Length) {


Index: clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
===
--- clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -49,7 +49,21 @@
 
   verifyFormat("o0 = 0;\n"
"o1 = 07;\n"
-   "o5 = 012345",
+   "o5 = 012345;",
+   Style);
+
+  verifyFormat("bi = 0b1'i;\n"
+   "dif = 1'234if;\n"
+   "hil = 0xA'BCil;",
+   "bi = 0b1i;\n"
+   "dif = 1234if;\n"
+   "hil = 0xABCil;",
+   Style);
+
+  verifyFormat("d = 5'678_km;\n"
+   "h = 0xD'EF_u16;",
+   "d = 5678_km;\n"
+   "h = 0xDEF_u16;",
Style);
 }
 
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -106,6 +106,12 @@
 (IsBase16 && SkipHex) || B == Base::Other) {
   continue;
 }
+if (Style.isCpp()) {
+  if (const auto Pos = Text.find_first_of("_i"); Pos != StringRef::npos) {
+Text = Text.substr(0, Pos);
+Length = Pos;
+  }
+}
 if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) ||
 (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
   continue;
@@ -116,7 +122,7 @@
   continue;
 }
 const auto Start = Text[0] == '0' ? 2 : 0;
-auto End = Text.find_first_of("uUlLzZn");
+auto End = Text.find_first_of("uUlLzZn", Start);
 if (End == StringRef::npos)
   End = Length;
 if (Start > 0 || End < Length) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:714-716
+if (!Args.hasFlag(options::OPT_fdata_sections,
+  options::OPT_fno_data_sections, UseSeparateSections) &&
+Args.hasArg(options::OPT_fno_data_sections))

I think this (undesirably) generates an error even `-mno-roptr` is in effect.

This logic seems otherwise convoluted. I think the main logic should only care 
that `data-sections` is off. We can separately assert that `data-sections` is 
on by default on AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D146986: Downgrade reserved module identifier error into a warning

[Chuanqi Xu via Phabricator via cfe-commits]

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

> Presumably adding an alias for #pragma clang system_header called 
> system_module wouldn't be too hard? (though the pragma is also being removed 
> from libc++ soon, I think, in favor of -isystem usage, so maybe that's a sign 
> the pragma's not a great way to do)

Yeah, it shouldn't be hard but it looks not good too. I feel the current patch 
is better/simpler as a workaround.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146986

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

[PATCH] D146929: [clang-tidy] Ignore unevaluated exprs in rvalue-reference-param-not-moved

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

@PiotrZSL if you're happy with these changes, would you mind committing them 
for me? "Chris Cotter "


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146929

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

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

In D140760#4222989 , @PiotrZSL wrote:

> What about classes that doesn't have begin/end method but got cbegin/cend ? 
> I thing there is open issue for that.

Should we handle that in a separate patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to S11

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added a comment.

Sorry for cross post, but I guess some people might not follow closely in 
discourse (like me :P):

Another proposal from me is using gp as platform register: 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/371

Some advantage on taking gp as platform register rather than other GPRs:

- Compiler doesn’t use gp register anywhere for now.
- All assembly files (which conform with current ABI) didn’t use that except 
the __global_pointer$ initialization code in CRT files.
- The main user is linker, linker will use that to perform linker relaxation, 
and we already have the command option to tune that off.

Potential issues:

- Loss the code size and performance gain from gp relaxation
  - The most gain from gp relaxation is embedded application, it’s different 
target audience as SCS, so this should not blocker issues.
  - Android is an example, it’s already disable GP relaxation at all, so we 
don’t have any loss for this case.
- Will it break any existing platform?
  - Treat gp as platform register is optional, it’s still default use as gp 
relaxation, so NO breakage on existing platform, but give the freedom of the 
platform to use gp register as other purpose if they don’t want gp relxation.
  - Added an attribute to let linker to help mixing up different gp usage 
objects, also linker could check that attribute to make sure gp relaxation is 
do-able or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146463

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

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 508869.
ccotter marked 2 inline comments as done.
ccotter added a comment.

- fix docs, fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference  { using type = T; };
+template  struct remove_reference { using type = T; };
+
+template  using remove_reference_t = typename remove_reference::type;
+
+template  constexpr T &&forward(remove_reference_t &t) noexcept;
+template  constexpr T &&forward(remove_reference_t &&t) noexcept;
+template  constexpr remove_reference_t &&move(T &&x);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+  S();
+  S(const S&);
+  S(S&&) noexcept;
+  S& operator=(const S&);
+  S& operator=(S&&) noexcept;
+};
+
+template 
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template 
+void does_not_forward(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t;
+}
+
+template 
+void does_not_forward_invoked(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  T other = t();
+}
+
+template 
+void forwards_pairwise(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  auto first = std::forward(t.first);
+  auto second = std::forward(t.second);
+}
+
+template 
+void does_not_forward_pack(Ts&&... ts) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  consumes_all(ts...);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(u) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+  template 
+  AClass& operator=(U&& u) { }
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+  template 
+  void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+  }
+
+  T data;
+};
+
+template 
+void does_not_forward_in_evaluated_code(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+  using result_t = decltype(std::forward(t));
+  unsigned len = sizeof(std::forward(t));
+  T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template 
+void just_a_decl(T&&t);
+
+template 
+void does_forward(T&& t) {
+  T other = std::forward(t);
+}
+
+template 
+void does_forward_pack(Ts&&... ts) {
+  consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+  S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+  S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+  S other = std::move(s);
+}
+
+template 
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+  T other = std::move(t);
+}
+
+template 
+class AClass {
+
+  template 
+  AClass(U&& u) : data(std::forward(u)) {}
+
+

[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Chris Cotter via Phabricator via cfe-commits]

ccotter added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:20
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+  if (isa(Node) || isa(Node))

PiotrZSL wrote:
> move this matcher to some utils/...
> It may be needed by other checks.
> 
Will be done in https://reviews.llvm.org/D146929



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases

PiotrZSL wrote:
> what about when someone uses std::move instead of std::format ?
> maybe some "note" for such issue ?
Are you suggesting to have the tool add a special note in something like

```
template 
void foo(T&& t) { T other = std::move(t); }
```

I'm not sure I completely followed what you were saying. Or perhaps a fixit for 
this specific case of using move on a forwarding reference (fixit to replace 
`move` with `forward`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921

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

[PATCH] D146929: [clang-tidy] Ignore unevaluated exprs in rvalue-reference-param-not-moved

[Chris Cotter via Phabricator via cfe-commits]

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

- move to utility file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146929

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h
  
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
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -156,6 +156,13 @@
   Obj moved = std::move((o));
 }
 
+void does_not_move_in_evaluated(Obj&& o) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: rvalue reference parameter 'o' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+  using result_t = decltype(std::move(o));
+  unsigned size = sizeof(std::move(o));
+  Obj moved = o;
+}
+
 template 
 struct mypair {
   T1 first;
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H
 
 #include "TypeTraits.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include 
 
@@ -48,6 +49,23 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+  if (isa(Node) || isa(Node))
+return true;
+  if (const auto *UnaryExpr = dyn_cast(&Node)) {
+switch (UnaryExpr->getKind()) {
+case UETT_SizeOf:
+case UETT_AlignOf:
+  return true;
+default:
+  return false;
+}
+  }
+  if (const auto *TypeIDExpr = dyn_cast(&Node))
+return !TypeIDExpr->isPotentiallyEvaluated();
+  return false;
+}
+
 // A matcher implementation that matches a list of type name regular expressions
 // against a NamedDecl. If a regular expression contains the substring "::"
 // matching will occur against the qualified name, otherwise only the typename.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "RvalueReferenceParamNotMovedCheck.h"
+#include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
@@ -14,6 +15,8 @@
 
 namespace clang::tidy::cppcoreguidelines {
 
+using matchers::hasUnevaluatedContext;
+
 namespace {
 AST_MATCHER_P(LambdaExpr, valueCapturesVar, DeclarationMatcher, VarMatcher) {
   return std::find_if(Node.capture_begin(), Node.capture_end(),
@@ -39,16 +42,18 @@
 
   StatementMatcher MoveCallMatcher =
   callExpr(
+  argumentCountIs(1),
   anyOf(callee(functionDecl(hasName("::std::move"))),
 callee(unresolvedLookupExpr(hasAnyDeclaration(
 namedDecl(hasUnderlyingDecl(hasName("::std::move"))),
-  argumentCountIs(1),
   hasArgument(
   0, argumentOf(
  AllowPartialMove,
  declRefExpr(to(equalsBoundNode("param"))).bind("ref"))),
   unless(hasAncestor(
-  lambdaExpr(valueCapturesVar(equalsBoundNode("param"))
+  lambdaExpr(valueCapturesVar(equalsBoundNode("param"),
+  unless(anyOf(hasAncestor(typeLoc()),
+   hasAncestor(expr(hasUnevaluatedContext())
   .bind("move-call");
 
   Finder->addMatcher(
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -10,13 +10,13 @@
 
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
-#include "clang/AST/ExprConcepts.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 
 #include "../utils/ExprSequence.h"
+#include "../utils/Matchers.h"
 #include 
 
 using namespace clang::ast_matchers;
@@ -24,24 +24,9 @@
 
 namespace clang::tidy::bugprone {
 
-namespace {
+using matchers::hasUnevaluatedContext

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

@sivachandra Host and device should agree on all ABI related properties of 
types or things turn badly, fast. That has to be a given for things to work, 
and from there, users expect types to be copyable, and usable on both sides.

@jhuber6 There are things we might need to define, sure. However, we can 
actually do that. If we do not use the host headers we can never support 
various things, like, no matter how much we try and want to. My examples with 
unused members inside of classes is one of them that is critical. We cannot 
tell people they can't use their objects because in the type is a subtype with 
a subtype with a subtype that has a different alignment/padding/size/layout on 
the host and device so every offset (read access) on one side will fail. We 
worked around issues in various headers already, this is expected to happen 
again as we include more host headers, but all of those problems are solvable 
by us, without requiring the user to modify arbitrarily complex parts of an 
existing codebase.
So far I've heard we need to define a `__ctype_b_loc`, and maybe a few other 
pointers like that. For one, that is doable, e.g., we also provide the right 
hooks to work with libgomp and other external libraries. The entire macro mess 
is actually a reason to use the host code since the user expects consistency. 
If it is a function here and a macro there, constexpr here and not constexpr 
there, ... will cause problems for people, and eventually us. Declaring a few 
compatibility pointers and functions seems a small price to pay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D146973#4225983 , @jdoerfert wrote:

> I said this before, many times:
>
> We don't want to have different host and device libraries that are 
> incompatible.
> Effectively, what we really want, is the host environment to just work on the 
> GPU.
> That includes extensions in the host headers, macros, taking the address of 
> stuff, etc.
> This became clear when we made (c)math.h available on the GPU (for OpenMP).

The problem is that we cannot control the system headers, they are not expected 
to work with `llvm-libc`. For example: the GNU `ctype.h` includes `features.h` 
which will attempt to include the 32-bit stubs file because the GPU is not a 
recognized target on the host. If you work around that, like we do in OpenMP, 
then you will realize that `isalnum` is actually a macro to `__isctype` which 
references and external table called `__ctype_b_loc` which isn't defined in the 
C standard. So, now we have a header that causes `isalnum` to not longer call 
the implementation in LLVM's `libc`, it also fails at link time because there 
is no reference to `__ctype_b_loc` in LLVM's `libc`. What is the solution here? 
Do we implement `libc` in LLVM with a workaround for every internal 
implementation in the GNU `libc`?

> For most of libc, we might get away with custom GPU headers but eventually it 
> will break "expected to work" user code, at the latest when we arrive at 
> libc++.
> A user can, right now, map a std::vector from the host to the device, and, 
> assuming they properly did the deep copy, it will work.
> If we end up with different sizes, alignments, layouts, this will not only 
> break, but we will also break any structure that depends on those sizes, 
> e.g., mapping an object with a std::map inside even if it is not accessed 
> will cause problems.
>
> In addition, systems are never "vanilla". We want to include the system 
> headers to get the extensions users might rely on. Providing only alternative 
> headers even breaks working code (in the OpenMP case), e.g., when we 
> auto-translate definitions in the header to the device (not a CUDA thing, I 
> think).

Using custom generated headers is the only approach that is guaranteed to 
actually work when we compile this. We cannot sanely implement a library using 
headers unique to another implementation targeting an entirely different 
machine, we will endlessly be chasing implementation details like above. This 
works in OpenMP currently because we've chosen a handful of headers that this 
doesn't completely break for.

> I strongly suggest to include our GPU headers first, in them we setup the 
> overlays for the system headers, and then we include the system versions.
> This works for (c)math.h, complex, and other parts of libc and libc++ 
> already, even though we don't ship them as libraries.

The wrapper approach works fine for the ones we've selected. And in the GPU 
`libc` we could generate our own headers that have `#include_next` in them if 
we verify that it works for that header. I think in general though, we need to 
work with custom headers first, and implement a set of features we know to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added a comment.

In D146973#4225983 , @jdoerfert wrote:

> For most of libc, we might get away with custom GPU headers but eventually it 
> will break "expected to work" user code, at the latest when we arrive at 
> libc++.
> A user can, right now, map a std::vector from the host to the device, and, 
> assuming they properly did the deep copy, it will work.

I do not have any strong opinions about how things ought to work. However, ISTM 
that the above is assuming that the type topology on the host matches the one 
on the GPU. Not sure if that is really an assumption or a restriction or a 
requirement. May be the host / device compatibility ensures this, I do not 
know, I know almost nothing about GPUs. But in general, I would expect a host 
<=> device communication channel to be a curated one. As in, the communication 
model can understand and serialize/deserialize only a small set of primitive 
types and compound types crafted from these primitive types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D138247#4217189 , @efriedma wrote:

> The relevant text of the current Itanium ABI (which was updated in 
> https://github.com/itanium-cxx-abi/cxx-abi/commit/d8e9d102c83f177970f0db6cc8bee170f2779bc1)
>
>> In the following contexts, however, the one-definition rule requires closure 
>> types in different translation units to "correspond":
>>
>> - default arguments appearing in class definitions
>> - default member initializers
>> - the bodies of inline or templated functions
>> - the initializers of inline or templated variables
>
> Could you update the references to the ABI document to use the new text?

Oh, yeah - good call. Done.

> Given the new rules, I think ContextKind::StaticDataMember shouldn't exist?  
> It's not one of the given categories; should be subsumed by the existing 
> categories.

Fair - I assume the "initializers of inline or templated variables" is intended 
to cover the case of static member variables of class templates, though they 
aren't technically variable templates presumably they are "templated variables".

So I renamed the `VariableTemplate` kind to `TemplatedVariable` to match the 
ABI wording/semantics a bit better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138247

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

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

[David Blaikie via Phabricator via cfe-commits]

dblaikie updated this revision to Diff 508847.
dblaikie added a comment.

Rename VariableTemplate classification to TemplatedVariable to match ABI wording


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138247

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCXX/mangle-lambdas.cpp

Index: clang/test/CodeGenCXX/mangle-lambdas.cpp
===
--- clang/test/CodeGenCXX/mangle-lambdas.cpp
+++ clang/test/CodeGenCXX/mangle-lambdas.cpp
@@ -290,6 +290,17 @@
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZZ3ft4IiEvvEN2lc2mfEiEd_NKUlvE_clEv
 
 
+extern int ExternalVariable;
+struct StaticInlineMember {
+  static constexpr auto x = [] { return ExternalVariable; };
+};
+
+// CHECK-LABEL: define void @_Z23test_StaticInlineMemberv
+// CHECK: call {{.*}} @_ZNK18StaticInlineMember1xMUlvE_clEv
+void test_StaticInlineMember() {
+  StaticInlineMember::x();
+}
+
 // Check linkage of the various lambdas.
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZ11inline_funciENKUlvE_clEv
 // CHECK: ret i32 1
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -283,12 +283,14 @@
 Normal,
 DefaultArgument,
 DataMember,
-StaticDataMember,
 InlineVariable,
-VariableTemplate,
+TemplatedVariable,
 Concept
   } Kind = Normal;
 
+  bool IsInNonspecializedTemplate =
+  inTemplateInstantiation() || CurContext->isDependentContext();
+
   // Default arguments of member function parameters that appear in a class
   // definition, as well as the initializers of data members, receive special
   // treatment. Identify them.
@@ -299,15 +301,15 @@
 if (LexicalDC->isRecord())
   Kind = DefaultArgument;
 } else if (VarDecl *Var = dyn_cast(ManglingContextDecl)) {
-  if (Var->getDeclContext()->isRecord())
-Kind = StaticDataMember;
-  else if (Var->getMostRecentDecl()->isInline())
+  if (Var->getMostRecentDecl()->isInline())
 Kind = InlineVariable;
+  else if (Var->getDeclContext()->isRecord() && IsInNonspecializedTemplate)
+Kind = TemplatedVariable;
   else if (Var->getDescribedVarTemplate())
-Kind = VariableTemplate;
+Kind = TemplatedVariable;
   else if (auto *VTS = dyn_cast(Var)) {
 if (!VTS->isExplicitSpecialization())
-  Kind = VariableTemplate;
+  Kind = TemplatedVariable;
   }
 } else if (isa(ManglingContextDecl)) {
   Kind = DataMember;
@@ -319,12 +321,9 @@
   // Itanium ABI [5.1.7]:
   //   In the following contexts [...] the one-definition rule requires closure
   //   types in different translation units to "correspond":
-  bool IsInNonspecializedTemplate =
-  inTemplateInstantiation() || CurContext->isDependentContext();
   switch (Kind) {
   case Normal: {
-//  -- the bodies of non-exported nonspecialized template functions
-//  -- the bodies of inline functions
+//  -- the bodies of inline or templated functions
 if ((IsInNonspecializedTemplate &&
  !(ManglingContextDecl && isa(ManglingContextDecl))) ||
 isInInlineFunction(CurContext)) {
@@ -341,21 +340,13 @@
 // however the ManglingContextDecl is important for the purposes of
 // re-forming the template argument list of the lambda for constraint
 // evaluation.
-  case StaticDataMember:
-//  -- the initializers of nonspecialized static members of template classes
-if (!IsInNonspecializedTemplate)
-  return std::make_tuple(nullptr, ManglingContextDecl);
-// Fall through to get the current context.
-[[fallthrough]];
-
   case DataMember:
-//  -- the in-class initializers of class members
+//  -- default member initializers
   case DefaultArgument:
 //  -- default arguments appearing in class definitions
   case InlineVariable:
-//  -- the initializers of inline variables
-  case VariableTemplate:
-//  -- the initializers of templated variables
+  case TemplatedVariable:
+//  -- the initializers of inline or templated variables
 return std::make_tuple(
 &Context.getManglingNumberContext(ASTContext::NeedExtraManglingDecl,
   ManglingContextDecl),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

[David Blaikie via Phabricator via cfe-commits]

dblaikie updated this revision to Diff 508842.
dblaikie added a comment.

Remove `StaticDataMember` and classify static member variables of templates as 
variable templates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138247

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/CodeGenCXX/mangle-lambdas.cpp


Index: clang/test/CodeGenCXX/mangle-lambdas.cpp
===
--- clang/test/CodeGenCXX/mangle-lambdas.cpp
+++ clang/test/CodeGenCXX/mangle-lambdas.cpp
@@ -290,6 +290,17 @@
 // CHECK-LABEL: define linkonce_odr noundef i32 
@_ZZZ3ft4IiEvvEN2lc2mfEiEd_NKUlvE_clEv
 
 
+extern int ExternalVariable;
+struct StaticInlineMember {
+  static constexpr auto x = [] { return ExternalVariable; };
+};
+
+// CHECK-LABEL: define void @_Z23test_StaticInlineMemberv
+// CHECK: call {{.*}} @_ZNK18StaticInlineMember1xMUlvE_clEv
+void test_StaticInlineMember() {
+  StaticInlineMember::x();
+}
+
 // Check linkage of the various lambdas.
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZ11inline_funciENKUlvE_clEv
 // CHECK: ret i32 1
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -283,12 +283,14 @@
 Normal,
 DefaultArgument,
 DataMember,
-StaticDataMember,
 InlineVariable,
 VariableTemplate,
 Concept
   } Kind = Normal;
 
+  bool IsInNonspecializedTemplate =
+  inTemplateInstantiation() || CurContext->isDependentContext();
+
   // Default arguments of member function parameters that appear in a class
   // definition, as well as the initializers of data members, receive special
   // treatment. Identify them.
@@ -299,10 +301,10 @@
 if (LexicalDC->isRecord())
   Kind = DefaultArgument;
 } else if (VarDecl *Var = dyn_cast(ManglingContextDecl)) {
-  if (Var->getDeclContext()->isRecord())
-Kind = StaticDataMember;
-  else if (Var->getMostRecentDecl()->isInline())
+  if (Var->getMostRecentDecl()->isInline())
 Kind = InlineVariable;
+  else if (Var->getDeclContext()->isRecord() && IsInNonspecializedTemplate)
+Kind = VariableTemplate;
   else if (Var->getDescribedVarTemplate())
 Kind = VariableTemplate;
   else if (auto *VTS = dyn_cast(Var)) {
@@ -319,12 +321,9 @@
   // Itanium ABI [5.1.7]:
   //   In the following contexts [...] the one-definition rule requires closure
   //   types in different translation units to "correspond":
-  bool IsInNonspecializedTemplate =
-  inTemplateInstantiation() || CurContext->isDependentContext();
   switch (Kind) {
   case Normal: {
-//  -- the bodies of non-exported nonspecialized template functions
-//  -- the bodies of inline functions
+//  -- the bodies of inline or templated functions
 if ((IsInNonspecializedTemplate &&
  !(ManglingContextDecl && isa(ManglingContextDecl))) ||
 isInInlineFunction(CurContext)) {
@@ -341,21 +340,13 @@
 // however the ManglingContextDecl is important for the purposes of
 // re-forming the template argument list of the lambda for constraint
 // evaluation.
-  case StaticDataMember:
-//  -- the initializers of nonspecialized static members of template 
classes
-if (!IsInNonspecializedTemplate)
-  return std::make_tuple(nullptr, ManglingContextDecl);
-// Fall through to get the current context.
-[[fallthrough]];
-
   case DataMember:
-//  -- the in-class initializers of class members
+//  -- default member initializers
   case DefaultArgument:
 //  -- default arguments appearing in class definitions
   case InlineVariable:
-//  -- the initializers of inline variables
   case VariableTemplate:
-//  -- the initializers of templated variables
+//  -- the initializers of inline or templated variables
 return std::make_tuple(
 &Context.getManglingNumberContext(ASTContext::NeedExtraManglingDecl,
   ManglingContextDecl),


Index: clang/test/CodeGenCXX/mangle-lambdas.cpp
===
--- clang/test/CodeGenCXX/mangle-lambdas.cpp
+++ clang/test/CodeGenCXX/mangle-lambdas.cpp
@@ -290,6 +290,17 @@
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZZ3ft4IiEvvEN2lc2mfEiEd_NKUlvE_clEv
 
 
+extern int ExternalVariable;
+struct StaticInlineMember {
+  static constexpr auto x = [] { return ExternalVariable; };
+};
+
+// CHECK-LABEL: define void @_Z23test_StaticInlineMemberv
+// CHECK: call {{.*}} @_ZNK18StaticInlineMember1xMUlvE_clEv
+void test_StaticInlineMember() {
+  StaticInlineMember::x();
+}
+
 // Check linkage of the various lambdas.
 // CHECK-LABEL: define linkonce_odr noundef i32 @_ZZ11inline_funciENKUlvE_clEv
 // CHECK: ret i32 1
Index: clang/lib/Sema/SemaLambda.cpp

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I said this before, many times:

We don't want to have different host and device libraries that are incompatible.
Effectively, what we really want, is the host environment just work on the GPU, 
that includes extensions in the host headers, macros, taking the address of 
stuff, etc.
This became clear when we made (c)math.h available on the GPU (for OpenMP).

For most of libc, we might get away with custom GPU headers but eventually it 
will break "expected to work" user code, at the latest when we arrive at libc++.
A user can, right now, map a std::vector from the host to the device, and, 
assuming they properly did the deep copy, it will work.
If we end up with different sizes, alignments, layouts, this will not only 
break, but we will also break any structure that depends on those sizes, e.g., 
mapping an object with a std::map inside even if it is not accessed will cause 
problems.

In addition, systems are never "vanilla". We want to include the system headers 
to get the extensions users might rely on. Providing only alternative headers 
even breaks working code (in the OpenMP case), e.g., when we auto-translate 
definitions in the header to the device (not a CUDA thing, I think).

I strongly suggest to include our GPU headers first, in them we setup the 
overlays for the system headers, and then we include the system versions.
This works for (c)math.h, complex, and other parts of libc and libc++ already, 
even though we don't ship them as libraries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

Michael137 wrote:
> dblaikie wrote:
> > Michael137 wrote:
> > > probinson wrote:
> > > > This doesn't become `Foo` ?
> > > The name stays as `Foo>` but the type of the template parameter 
> > > becomes `BarInt`. So the dwarf would look like:
> > > ```
> > > DW_TAG_structure_type
> > >   DW_AT_name  ("Foo >")
> > > 
> > >   DW_TAG_template_type_parameter
> > > DW_AT_type(0x0095 "BarInt")
> > > DW_AT_name("T")
> > > 
> > > ```
> > > Will add the parameter metadata to the test
> > Hmm, that seems buggy - why doesn't `Foo >` become `Foo`? 
> > (is the preferred name ever used in the DW_AT_name? I'd have thought it 
> > should be unless it's explicitly avoided to ensure type 
> > compatibility/collapsing with debug info built without this feature 
> > enabled?)
> I suspect it's because the name is constructed using the clang TypePrinter. 
> And we turn off `PrintingPolicy::UsePreferredNames` by default to avoid 
> divergence from GCC.
> 
> Will double check though
Yeah, that sounds right/OK to me. Sorry for the diversion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145803

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

[PATCH] D146975: [NVPTX] Add __CUDA_ARCH__ macro to standalone NVPTX compilations

[Joseph Huber via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
jhuber6 marked an inline comment as done.
Closed by commit rGbed7005eb4d4: [NVPTX] Add __CUDA_ARCH__ macro to standalone 
NVPTX compilations (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D146975?vs=508672&id=508828#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146975

Files:
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/test/Frontend/standalone-nvptx-macros.c


Index: clang/test/Frontend/standalone-nvptx-macros.c
===
--- /dev/null
+++ clang/test/Frontend/standalone-nvptx-macros.c
@@ -0,0 +1,5 @@
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang %s -c -E -dM --target=nvptx64-nvidia-cuda -march=sm_70 -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-CUDA-ARCH %s
+// CHECK-CUDA-ARCH: #define __CUDA_ARCH__ 700
Index: clang/lib/Basic/Targets/NVPTX.cpp
===
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -168,7 +168,7 @@
MacroBuilder &Builder) const {
   Builder.defineMacro("__PTX__");
   Builder.defineMacro("__NVPTX__");
-  if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice) {
+  if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice || !HostTarget) {
 // Set __CUDA_ARCH__ for the GPU specified.
 std::string CUDAArchCode = [this] {
   switch (GPU) {


Index: clang/test/Frontend/standalone-nvptx-macros.c
===
--- /dev/null
+++ clang/test/Frontend/standalone-nvptx-macros.c
@@ -0,0 +1,5 @@
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang %s -c -E -dM --target=nvptx64-nvidia-cuda -march=sm_70 -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-CUDA-ARCH %s
+// CHECK-CUDA-ARCH: #define __CUDA_ARCH__ 700
Index: clang/lib/Basic/Targets/NVPTX.cpp
===
--- clang/lib/Basic/Targets/NVPTX.cpp
+++ clang/lib/Basic/Targets/NVPTX.cpp
@@ -168,7 +168,7 @@
MacroBuilder &Builder) const {
   Builder.defineMacro("__PTX__");
   Builder.defineMacro("__NVPTX__");
-  if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice) {
+  if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice || !HostTarget) {
 // Set __CUDA_ARCH__ for the GPU specified.
 std::string CUDAArchCode = [this] {
   switch (GPU) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bed7005 - [NVPTX] Add __CUDA_ARCH__ macro to standalone NVPTX compilations

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-03-27T18:08:15-05:00
New Revision: bed7005eb4d4850b6f9d93707213ced5c0c19de0

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

LOG: [NVPTX] Add __CUDA_ARCH__ macro to standalone NVPTX compilations

We can now target the NVPTX architecture directly via
`--target=nvptx64-nvidia-cuda`. This currently does not define the
`__CUDA_ARCH__` macro with is used to allow code to target different
codes based on support. This patch simply adds this support.

Reviewed By: tra, jdoerfert

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

Added: 
clang/test/Frontend/standalone-nvptx-macros.c

Modified: 
clang/lib/Basic/Targets/NVPTX.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/NVPTX.cpp 
b/clang/lib/Basic/Targets/NVPTX.cpp
index aca51b2b95b59..7f4c5d83d8bd7 100644
--- a/clang/lib/Basic/Targets/NVPTX.cpp
+++ b/clang/lib/Basic/Targets/NVPTX.cpp
@@ -168,7 +168,7 @@ void NVPTXTargetInfo::getTargetDefines(const LangOptions 
&Opts,
MacroBuilder &Builder) const {
   Builder.defineMacro("__PTX__");
   Builder.defineMacro("__NVPTX__");
-  if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice) {
+  if (Opts.CUDAIsDevice || Opts.OpenMPIsDevice || !HostTarget) {
 // Set __CUDA_ARCH__ for the GPU specified.
 std::string CUDAArchCode = [this] {
   switch (GPU) {

diff  --git a/clang/test/Frontend/standalone-nvptx-macros.c 
b/clang/test/Frontend/standalone-nvptx-macros.c
new file mode 100644
index 0..5cf20ecdde25d
--- /dev/null
+++ b/clang/test/Frontend/standalone-nvptx-macros.c
@@ -0,0 +1,5 @@
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang %s -c -E -dM --target=nvptx64-nvidia-cuda -march=sm_70 -o - | \
+// RUN:   FileCheck --check-prefix=CHECK-CUDA-ARCH %s
+// CHECK-CUDA-ARCH: #define __CUDA_ARCH__ 700



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

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 508823.
yaxunl added a comment.

revised by Fangrui's comments


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

https://reviews.llvm.org/D146686

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/OHOS.cpp
  clang/lib/Driver/ToolChains/OHOS.h
  clang/lib/Driver/ToolChains/VEToolchain.cpp
  clang/test/Driver/arch-specific-libdir-rpath.c

Index: clang/test/Driver/arch-specific-libdir-rpath.c
===
--- clang/test/Driver/arch-specific-libdir-rpath.c
+++ clang/test/Driver/arch-specific-libdir-rpath.c
@@ -75,6 +75,15 @@
 // RUN: -frtlib-add-rpath \
 // RUN:   | FileCheck --check-prefixes=RESDIR,NO-LIBPATH,NO-RPATH %s
 
+// Test that the driver adds an per-target arch-specific subdirectory in
+// {RESOURCE_DIR}/lib/{triple} to the linker search path and to '-rpath'
+//
+// RUN: %clang %s -### 2>&1 --target=x86_64-linux-gnu \
+// RUN: -fsanitize=address -shared-libasan \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=PERTARGET %s
+
 // RESDIR: "-resource-dir" "[[RESDIR:[^"]*]]"
 //
 // LIBPATH-X86_64: -L[[RESDIR]]{{(/|)lib(/|)linux(/|)x86_64}}
@@ -88,3 +97,7 @@
 //
 // NO-LIBPATH-NOT: "-L{{[^"]*Inputs(/|)resource_dir}}"
 // NO-RPATH-NOT:   "-rpath" {{.*(/|)Inputs(/|)resource_dir}}
+
+// PERTARGET: "-resource-dir" "[[PTRESDIR:[^"]*]]"
+// PERTARGET: -L[[PTRESDIR]]{{(/|)lib(/|)x86_64-unknown-linux-gnu}}
+// PERTARGET:   "-rpath" "[[PTRESDIR]]{{(/|)lib(/|)x86_64-unknown-linux-gnu}}"
Index: clang/lib/Driver/ToolChains/VEToolchain.cpp
===
--- clang/lib/Driver/ToolChains/VEToolchain.cpp
+++ clang/lib/Driver/ToolChains/VEToolchain.cpp
@@ -33,7 +33,7 @@
   // These are OK.
 
   // Default file paths are following:
-  //   ${RESOURCEDIR}/lib/linux/ve, (== getArchSpecificLibPath)
+  //   ${RESOURCEDIR}/lib/linux/ve, (== getArchSpecificLibPaths)
   //   /lib/../lib64,
   //   /usr/lib/../lib64,
   //   ${BINPATH}/../lib,
@@ -45,12 +45,13 @@
   getFilePaths().clear();
 
   // Add library directories:
-  //   ${BINPATH}/../lib/ve-unknown-linux-gnu, (== getStdlibPath)
-  //   ${RESOURCEDIR}/lib/linux/ve, (== getArchSpecificLibPath)
+  //   ${BINPATH}/../lib/ve-unknown-linux-gnu, (== getStdlibPaths)
+  //   ${RESOURCEDIR}/lib/linux/ve, (== getArchSpecificLibPaths)
   //   ${SYSROOT}/opt/nec/ve/lib,
   for (auto &Path : getStdlibPaths())
 getFilePaths().push_back(std::move(Path));
-  getFilePaths().push_back(getArchSpecificLibPath());
+  for (const auto &Path : getArchSpecificLibPaths())
+getFilePaths().push_back(Path);
   getFilePaths().push_back(computeSysRoot() + "/opt/nec/ve/lib");
 }
 
Index: clang/lib/Driver/ToolChains/OHOS.h
===
--- clang/lib/Driver/ToolChains/OHOS.h
+++ clang/lib/Driver/ToolChains/OHOS.h
@@ -82,7 +82,8 @@
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
  llvm::opt::ArgStringList &CmdArgs) const override;
-  std::string getArchSpecificLibPath() const override;
+  path_list getArchSpecificLibPaths() const override;
+
 private:
   Multilib SelectedMultilib;
 };
Index: clang/lib/Driver/ToolChains/OHOS.cpp
===
--- clang/lib/Driver/ToolChains/OHOS.cpp
+++ clang/lib/Driver/ToolChains/OHOS.cpp
@@ -139,9 +139,9 @@
   SelectedMultilib = Result.SelectedMultilib;
 
   getFilePaths().clear();
-  std::string CandidateLibPath = getArchSpecificLibPath();
-  if (getVFS().exists(CandidateLibPath))
-getFilePaths().push_back(CandidateLibPath);
+  for (const auto &CandidateLibPath : getArchSpecificLibPaths())
+if (getVFS().exists(CandidateLibPath))
+  getFilePaths().push_back(CandidateLibPath);
 
   getLibraryPaths().clear();
   for (auto &Path : getRuntimePaths())
@@ -401,9 +401,12 @@
   ToolChain::addProfileRTLibs(Args, CmdArgs);
 }
 
-std::string OHOS::getArchSpecificLibPath() const {
+ToolChain::path_list OHOS::getArchSpecificLibPaths() const {
+  ToolChain::path_list Paths;
   llvm::Triple Triple = getTriple();
-  return makePath({getDriver().ResourceDir, "lib", getMultiarchTriple(Triple)});
+  Paths.push_back(
+  makePath({getDriver().ResourceDir, "lib", getMultiarchTriple(Triple)}));
+  return Paths;
 }
 
 ToolChain::UnwindLibType OHOS::GetUnwindLibType(const llvm::opt::ArgList &Args) const {
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -815,10 +815,11 @@
 optio

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j added a comment.

In D145770#4220246 , @mdtoguchi wrote:

> @lamb-j - is it expected for any bundled objects created before your change 
> without the explicit env field to be able to be unbundled?  Newly generated 
> bundles work as expected given similar `-target` values, but older generated 
> binaries fail to unbundle the target given equivalent commands.  Is it 
> possible to provide the ability to do so?

That should still be supported. The target triple for the old bundle should be 
converted to the new format (and compared against the Target-ID that was also 
converted to the new format). Can you provide an example to recreate the 
failure? I just tried one example locally and didn't hit any failures:

  old-clang -c --offload-arch=gfx906 -emit-llvm -fgpu-rdc --gpu-bundle-output 
square.hip
  new-clang-offload-bundler -unbundle -type=bc 
-targets=hip-amdgcn-amd-amdhsa-gfx906 -input=square.bc 
-output=square-hip-gfx906.bc -allow-missing-bundles 
-debug-only=CodeObjectCompatibility
  
  Compatible: Exact match:[CodeObject: hip-amdgcn-amd-amdhsa--gfx906]   
  :   [Target: hip-amdgcn-amd-amdhsa--gfx906]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D146892: [documentation]Fixed Random Typo

[Ayushi Shukla via Phabricator via cfe-commits]

ayushi-8102 added a comment.

My Pleasure!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146892

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a subscriber: rnk.
aprantl added a comment.

In D146595#4225630 , @aaron.ballman 
wrote:

> It's less about other debug formats and more about user experience. My two 
> big concerns are: 1) writing multiple attributes to do the same thing is 
> somewhat user-hostile unless "the same thing" is actually different in some 
> way users will care about, 2) we have a policy that user-facing attributes 
> that do nothing are diagnosed as being ignored, so if we don't support 
> Windows for this attribute, then compiling on that platform is going to 
> generate a pile of "attribute ignored" warnings. If we can support the 
> attribute with PDB as well, that solves both of my big concerns because it 
> makes this attribute generally useful (which is what we aim for with 
> attributes when possible).

That's a valid concern. However, in its current form it would still get lowered 
into LLVM IR, and LLVM may or may not silently ignore the attribute when 
lowering to PDB. It could be argued that that's even worse than warning about 
an ignored attribute. I just wanted to point this out.

> I don't want to sign y'all up for more work you can't easily do yourself, so 
> I don't want to block on support for Windows unless it turns out to be 
> completely trivial to support. But from a review POV, I'd like to hear back 
> from someone who knows something about PDB to validate that the attribute 
> doesn't require a breaking change for support there (like going from taking 
> no args to requiring an arg for some reason, basically). If we don't think 
> breaking changes should be needed, then I think documentation + diagnostics 
> will be sufficient for me.
>
> I did have two questions though:
> What happens when stepping into the attributed function through a function 
> pointer?
> Are there any special situations where a function cannot/should not be marked 
> with the attribute (virtual functions, lambda, blocks)?

@rnk Do you happen to know anything about how PDB handles these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D147003: [clang-format] JSON Add ability to add a space before the colon

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

You need to add `CHECK_PARSE_BOOL(SpaceBeforeJsonColon)` in ConfigParseTest.cpp.




Comment at: clang/include/clang/Format/Format.h:3717
 
+  /// If ``true``, a space will be add before a json colon 
+  /// \code





Comment at: clang/lib/Format/TokenAnnotator.cpp:4155
 if (Right.is(tok::colon))
-  return false;
+  return Style.SpaceBeforeJsonColon;
   } else if (Style.isCSharp()) {

Do we need to check if `Left.is(tok::string_literal)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147003

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

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

To be honest, i got this check implemented and running already on some big code 
base since years, and inserted few thousands of std::move's with 0 
false-positives, maybe not finding all cases, but I'm satisfied. 
I will put here source code, maybe you will get some ideas, but still I 
originally implemented it on Clang 5, so code seen some evolution, and now 
probably there are better ways to do these things. You don't need any magical 
heuristic or algorithms.
F26913338: MissingStdMove.cpp 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D146973#4225641 , @aaron.ballman 
wrote:

>> This lets offloading languages such as OpenMP use the system string.h when 
>> compiling for the host and then the LLVM libc string.h when targeting the 
>> GPU.
>
> How do we avoid ABI issues when the two headers get sufficiently out of sync? 
> (In general, I'm pretty surprised to hear we would want different headers for 
> the GPU and the system -- does this affect conformance requirements from the 
> C standard?)

I'm not entirely sure if there's a good method. I think no matter what we do 
we'll need to implement some kind of 'glue'. I think most should be fine if we 
go by the C-standard. We expect pointer sizes and everything to be compatible 
at least.

> Excuse my ignorance on this point, but is llvm-libc intended to work with any 
> compiler other than Clang? (e.g., will other compilers need to do this dance 
> as well?)

Right now the GPU target I'm working on can only be built with `clang` and it 
will be that way for the foreseeable future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> This lets offloading languages such as OpenMP use the system string.h when 
> compiling for the host and then the LLVM libc string.h when targeting the GPU.

How do we avoid ABI issues when the two headers get sufficiently out of sync? 
(In general, I'm pretty surprised to hear we would want different headers for 
the GPU and the system -- does this affect conformance requirements from the C 
standard?)

Excuse my ignorance on this point, but is llvm-libc intended to work with any 
compiler other than Clang? (e.g., will other compilers need to do this dance as 
well?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146847: [NFC] Fix uninitialized member variable use in RVVEmitter::createRVVIntrinsics()

[Sindhu Chittireddy 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 rG9972c9a89347: [NFC] Remove unused member variable 
`PolicyBitMask` in SemaRecord (authored by schittir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146847

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9972c9a - [NFC] Remove unused member variable `PolicyBitMask` in SemaRecord

[via cfe-commits]

Author: Chittireddy, Sindhu
Date: 2023-03-27T17:39:40-04:00
New Revision: 9972c9a893478241b7c9d8b587df978bc6d7e5a0

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

LOG: [NFC] Remove unused member variable `PolicyBitMask` in SemaRecord

Coverity static analysis tool found PolicyBitMask being used
uninitialized in push_back call in RVVEmitter::createRVVIntrinsics()
but this variable has no uses.

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

Added: 


Modified: 
clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/RISCVVEmitter.cpp 
b/clang/utils/TableGen/RISCVVEmitter.cpp
index b19dd4b6b9808..365fa8a67ffa1 100644
--- a/clang/utils/TableGen/RISCVVEmitter.cpp
+++ b/clang/utils/TableGen/RISCVVEmitter.cpp
@@ -57,9 +57,6 @@ struct SemaRecord {
   // Suffix of overloaded intrinsic name.
   SmallVector OverloadedSuffix;
 
-  // BitMask for supported policies.
-  uint16_t PolicyBitMask;
-
   // Number of field, large than 1 if it's segment load/store.
   unsigned NF;
 



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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D146595#4225490 , @aprantl wrote:

>> We don't want to have one attribute per debug format, because that's not 
>> going to scale.
>
> LLVM supports outputting a total of 2 debug info formats.

Why should the user have to write two separate attributes that do the same 
thing?

>> So how do we validate that this attribute should be exposed to users *now* 
>> before we know what the story is for other debug formats?
>
> Do you have any other debug info formats in mind? To my knowledge LLVM 
> doesn't support lowering all of the the debug info metadata into PDB because 
> there are some corners where it is less expressive than DWARF.
> I believe that the closest equivalent to this feature in PDB is a feature 
> called "just my code". A web search came up with 
> https://learn.microsoft.com/en-us/visualstudio/debugger/just-my-code?view=vs-2022
>  and it seems like it's powered by an external database of function names. 
> Would be interesting to hear from someone with knowledge and an interest in 
> PDB about this.

It's less about other debug formats and more about user experience. My two big 
concerns are: 1) writing multiple attributes to do the same thing is somewhat 
user-hostile unless "the same thing" is actually different in some way users 
will care about, 2) we have a policy that user-facing attributes that do 
nothing are diagnosed as being ignored, so if we don't support Windows for this 
attribute, then compiling on that platform is going to generate a pile of 
"attribute ignored" warnings. If we can support the attribute with PDB as well, 
that solves both of my big concerns because it makes this attribute generally 
useful (which is what we aim for with attributes when possible).

> Would you say that we shouldn't create this attribute if it cannot be 
> supported on Windows? Or would documenting that the attribute only has an 
> effect on DWARF platforms be sufficient?

I don't want to sign y'all up for more work you can't easily do yourself, so I 
don't want to block on support for Windows unless it turns out to be completely 
trivial to support. But from a review POV, I'd like to hear back from someone 
who knows something about PDB to validate that the attribute doesn't require a 
breaking change for support there (like going from taking no args to requiring 
an arg for some reason, basically). If we don't think breaking changes should 
be needed, then I think documentation + diagnostics will be sufficient for me.

I did have two questions though:
What happens when stepping into the attributed function through a function 
pointer?
Are there any special situations where a function cannot/should not be marked 
with the attribute (virtual functions, lambda, blocks)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D147003: [clang-format] JSON Add ability to add a space before the colon

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: owenpan, HazardyKnusperkeks, rymiel.
MyDeveloperDay added projects: clang, clang-format.
Herald added a project: All.
MyDeveloperDay requested review of this revision.

I've seen a couple of request for extra Json formatting to match prettier 
capability.

This patch allows for a space between the "key" and the colon

  {
  "A" : "value"
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147003

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestJson.cpp

Index: clang/unittests/Format/FormatTestJson.cpp
===
--- clang/unittests/Format/FormatTestJson.cpp
+++ clang/unittests/Format/FormatTestJson.cpp
@@ -236,5 +236,20 @@
  Style);
 }
 
+TEST_F(FormatTestJson, SpaceBeforeJsonColon) {
+  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Json);
+  verifyFormatStable("{\n"
+ "  \"name\": 1\n"
+ "}",
+ Style);
+
+  Style.SpaceBeforeJsonColon = true;
+  verifyFormatStable("{}", Style);
+  verifyFormatStable("{\n"
+ "  \"name\" : 1\n"
+ "}",
+ Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3629,8 +3629,6 @@
   Right.MatchingParen->is(TT_CastRParen)) {
 return true;
   }
-  if (Style.isJson() && Left.is(tok::string_literal) && Right.is(tok::colon))
-return false;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)
 return true;
   if (Style.ObjCSpaceAfterProperty && Line.Type == LT_ObjCProperty &&
@@ -4154,7 +4152,7 @@
   return Right.hasWhitespaceBefore();
   } else if (Style.isJson()) {
 if (Right.is(tok::colon))
-  return false;
+  return Style.SpaceBeforeJsonColon;
   } else if (Style.isCSharp()) {
 // Require spaces around '{' and  before '}' unless they appear in
 // interpolated strings. Interpolated strings are merged into a single token
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1006,6 +1006,7 @@
Style.SpaceBeforeCtorInitializerColon);
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
+IO.mapOptional("SpaceBeforeJsonColon", Style.SpaceBeforeJsonColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
 IO.mapOptional("SpaceBeforeParensOptions", Style.SpaceBeforeParensOptions);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
@@ -1429,6 +1430,7 @@
   LLVMStyle.SpaceBeforeCaseColon = false;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
+  LLVMStyle.SpaceBeforeJsonColon = false;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeParensOptions = {};
   LLVMStyle.SpaceBeforeParensOptions.AfterControlStatements = true;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3714,6 +3714,16 @@
   /// \version 7
   bool SpaceBeforeInheritanceColon;
 
+  /// If ``true``, a space will be add before a json colon 
+  /// \code
+  ///true:  false:
+  ///{  {
+  ///  "key" : "value"  vs.   "key": "value"
+  ///}  }
+  /// \endcode
+  /// \version 17
+  bool SpaceBeforeJsonColon;
+
   /// Different ways to put a space before opening parentheses.
   enum SpaceBeforeParensStyle : int8_t {
 /// Never put a space before opening parentheses.
@@ -4323,6 +4333,7 @@
SpaceBeforeCtorInitializerColon ==
R.SpaceBeforeCtorInitializerColon &&
SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
+   SpaceBeforeJsonColon == R.SpaceBeforeJsonColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceBeforeParensOptions == R.SpaceBeforeParensOptions &&
SpaceAroundPointerQualifiers == R.SpaceAroundPointerQualifiers &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -4746,6 +4746,18 @@
  true:  false:
  class Foo : Bar {} vs. class Foo: Bar {

[PATCH] D147002: [clang][deps] Don't cache stat failures for .framework directories

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added a reviewer: Bigcheese.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The .framework directory might be created during the course of the build. Some 
scanning worker finding out it
doesn't exist yet at the start shouldn't make it impossible for subsequent 
workers to see it when it evetually gets
created.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147002

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -169,8 +169,9 @@
   .CasesLower(".m", ".mm", 
PathPolicy::cache(ScanFile::Yes))
   .CasesLower(".i", ".ii", ".mi", ".mmi",  
PathPolicy::cache(ScanFile::Yes))
   .CasesLower(".def", ".inc",  
PathPolicy::cache(ScanFile::Yes))
+  .CaseLower(".framework", PathPolicy::cache(ScanFile::No, 
CacheStatFailure::No))
   .CasesLower(".modulemap", ".map",  PathPolicy::cache(ScanFile::No))
-  .CasesLower(".framework", ".apinotes", PathPolicy::cache(ScanFile::No))
+  .CaseLower(".apinotes",PathPolicy::cache(ScanFile::No))
   .CasesLower(".yaml", ".json", ".hmap", PathPolicy::cache(ScanFile::No))
   .Default(PathPolicy::fallThrough());
   // clang-format on


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -169,8 +169,9 @@
   .CasesLower(".m", ".mm", PathPolicy::cache(ScanFile::Yes))
   .CasesLower(".i", ".ii", ".mi", ".mmi",  PathPolicy::cache(ScanFile::Yes))
   .CasesLower(".def", ".inc",  PathPolicy::cache(ScanFile::Yes))
+  .CaseLower(".framework", PathPolicy::cache(ScanFile::No, CacheStatFailure::No))
   .CasesLower(".modulemap", ".map",  PathPolicy::cache(ScanFile::No))
-  .CasesLower(".framework", ".apinotes", PathPolicy::cache(ScanFile::No))
+  .CaseLower(".apinotes",PathPolicy::cache(ScanFile::No))
   .CasesLower(".yaml", ".json", ".hmap", PathPolicy::cache(ScanFile::No))
   .Default(PathPolicy::fallThrough());
   // clang-format on
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 158a431 - Revert ExtractAPI from https://reviews.llvm.org/D146656

[Daniel Grumberg via cfe-commits]

Author: Daniel Grumberg
Date: 2023-03-27T22:12:36+01:00
New Revision: 158a431227a876306fe5838936413dd51588d0c6

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

LOG: Revert ExtractAPI from https://reviews.llvm.org/D146656

Added: 
clang/lib/ExtractAPI/ExtractAPIVisitor.cpp
clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h

Modified: 
clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
clang/lib/ExtractAPI/CMakeLists.txt
clang/lib/ExtractAPI/DeclarationFragments.cpp
clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
clang/test/Index/extract-api-cursor.m
clang/tools/c-index-test/c-index-test.c
clang/tools/libclang/CXExtractAPI.cpp

Removed: 
clang/include/clang/ExtractAPI/TypedefUnderlyingTypeResolver.h



diff  --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h 
b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
index a31648b80195a..f6546fb4776a6 100644
--- a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -14,27 +14,24 @@
 #ifndef LLVM_CLANG_EXTRACTAPI_EXTRACT_API_VISITOR_H
 #define LLVM_CLANG_EXTRACTAPI_EXTRACT_API_VISITOR_H
 
-#include "llvm/ADT/FunctionExtras.h"
-
-#include "clang/AST/ASTContext.h"
-#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/ExtractAPI/API.h"
-#include "clang/ExtractAPI/TypedefUnderlyingTypeResolver.h"
-#include 
+#include "llvm/ADT/FunctionExtras.h"
 
 namespace clang {
 namespace extractapi {
-namespace impl {
-
-template 
-class ExtractAPIVisitorBase : public RecursiveASTVisitor {
-protected:
-  ExtractAPIVisitorBase(ASTContext &Context, APISet &API)
-  : Context(Context), API(API) {}
 
+/// The RecursiveASTVisitor to traverse symbol declarations and collect API
+/// information.
+class ExtractAPIVisitor : public RecursiveASTVisitor {
 public:
+  ExtractAPIVisitor(ASTContext &Context,
+llvm::unique_function 
LocationChecker,
+APISet &API)
+  : Context(Context), API(API),
+LocationChecker(std::move(LocationChecker)) {}
+
   const APISet &getAPI() const { return API; }
 
   bool VisitVarDecl(const VarDecl *Decl);
@@ -53,11 +50,7 @@ class ExtractAPIVisitorBase : public 
RecursiveASTVisitor {
 
   bool VisitObjCCategoryDecl(const ObjCCategoryDecl *Decl);
 
-  bool shouldDeclBeIncluded(const Decl *Decl) const;
-
-  const RawComment *fetchRawCommentForDecl(const Decl *Decl) const;
-
-protected:
+private:
   /// Collect API information for the enum constants and associate with the
   /// parent enum.
   void recordEnumConstants(EnumRecord *EnumRecord,
@@ -84,582 +77,9 @@ class ExtractAPIVisitorBase : public 
RecursiveASTVisitor {
 
   void recordObjCProtocols(ObjCContainerRecord *Container,
ObjCInterfaceDecl::protocol_range Protocols);
-
   ASTContext &Context;
   APISet &API;
-
-  StringRef getTypedefName(const TagDecl *Decl) {
-if (const auto *TypedefDecl = Decl->getTypedefNameForAnonDecl())
-  return TypedefDecl->getName();
-
-return {};
-  }
-
-  bool isInSystemHeader(const Decl *D) {
-return Context.getSourceManager().isInSystemHeader(D->getLocation());
-  }
-
-private:
-  Derived &getDerivedExtractAPIVisitor() {
-return *static_cast(this);
-  }
-};
-
-template 
-bool ExtractAPIVisitorBase::VisitVarDecl(const VarDecl *Decl) {
-  // skip function parameters.
-  if (isa(Decl))
-return true;
-
-  // Skip non-global variables in records (struct/union/class).
-  if (Decl->getDeclContext()->isRecord())
-return true;
-
-  // Skip local variables inside function or method.
-  if (!Decl->isDefinedOutsideFunctionOrMethod())
-return true;
-
-  // If this is a template but not specialization or instantiation, skip.
-  if (Decl->getASTContext().getTemplateOrSpecializationInfo(Decl) &&
-  Decl->getTemplateSpecializationKind() == TSK_Undeclared)
-return true;
-
-  if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl))
-return true;
-
-  // Collect symbol information.
-  StringRef Name = Decl->getName();
-  StringRef USR = API.recordUSR(Decl);
-  PresumedLoc Loc =
-  Context.getSourceManager().getPresumedLoc(Decl->getLocation());
-  LinkageInfo Linkage = Decl->getLinkageAndVisibility();
-  DocComment Comment;
-  if (auto *RawComment =
-  getDerivedExtractAPIVisitor().fetchRawCommentForDecl(Decl))
-Comment = RawComment->getFormattedLines(Context.getSourceManager(),
-Context.getDiagnostics());
-
-  // Build declaration fragments and sub-heading for the variable.
-  DeclarationFragments Declaration =
-  DeclarationFragmentsBuilder:

[PATCH] D146866: [clang][ExtractAPI] Remove extra pointer indirection from declaration fragments for Obj-C lightweight generics on id

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav added a comment.

looks like its waiting on libcxx 
build(https://buildkite.com/llvm-project/premerge-checks/builds/143382#job-01872209-2788-44ef-930c-11e741ff906e)
  and it is actively being debugged as per comments on other differential.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146866

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 508792.
MatzeB marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

Files:
  clang/include/clang/AST/StmtCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCoroutines/coro-function-try-block.cpp

Index: clang/test/CodeGenCoroutines/coro-function-try-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-function-try-block.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct task {
+  struct promise_type {
+task get_return_object();
+std::suspend_never initial_suspend();
+std::suspend_never final_suspend() noexcept;
+void return_void();
+void unhandled_exception() noexcept;
+  };
+};
+
+task f() try {
+  co_return;
+} catch(...) {
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void @_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4546,7 +4546,8 @@
 
   FSI->setHasCXXTry(TryLoc);
 
-  return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers);
+  return CXXTryStmt::Create(Context, TryLoc, cast(TryBlock),
+Handlers);
 }
 
 StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -721,8 +721,17 @@
   auto Loc = S.getBeginLoc();
   CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
  CurCoro.Data->ExceptionHandler);
-  auto *TryStmt =
-  CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
+  Stmt *BodyStmt = S.getBody();
+  // We are about to create a `CXXTryStmt` which requires a `CompoundStmt`.
+  // We have to create a new CompoundStmt if the function body is not a
+  // `CompoundStmt` yet in cases like the "function-try-block" syntax.
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }
+  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, Body, &Catch);
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Index: clang/lib/AST/StmtCXX.cpp
===
--- clang/lib/AST/StmtCXX.cpp
+++ clang/lib/AST/StmtCXX.cpp
@@ -23,7 +23,8 @@
 }
 
 CXXTryStmt *CXXTryStmt::Create(const ASTContext &C, SourceLocation tryLoc,
-   Stmt *tryBlock, ArrayRef handlers) {
+   CompoundStmt *tryBlock,
+   ArrayRef handlers) {
   const size_t Size = totalSizeToAlloc(handlers.size() + 1);
   void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
   return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
@@ -36,7 +37,7 @@
   return new (Mem) CXXTryStmt(Empty, numHandlers);
 }
 
-CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
+CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
ArrayRef handlers)
 : Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
   Stmt **Stmts = getStmts();
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6793,8 +6793,8 @@
   return ToHandlerOrErr.takeError();
   }
 
-  return CXXTryStmt::Create(
-  Importer.getToContext(), *ToTryLocOrErr,*ToTryBlockOrErr, ToHandlers);
+  return CXXTryStmt::Create(Importer.getToContext(), *ToTryLocOrErr,
+cast(*ToTryBlockOrErr), ToHandlers);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -75,7 +75,8 @@
   unsigned NumHandlers;
   size_t numTrailingObjects(OverloadToken) const { return NumHandlers; }
 
-  CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock, ArrayRef handlers);
+  CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
+ ArrayRef handlers);
   CXXTryStmt(EmptyShell Empty,

[PATCH] D146895: [clang-format] Don't annotate left brace of struct as FunctionLBrace

[Owen Pan 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 rG767aee1de9e9: [clang-format] Don't annotate left brace 
of struct as FunctionLBrace (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146895

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,14 @@
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct EXPORT_MACRO [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct [[deprecated]] [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25308,6 +25308,11 @@
"};",
Style);
 
+  verifyFormat("struct EXPORT_MACRO [[nodiscard]] C {\n"
+   "  int i;\n"
+   "};",
+   Style);
+
   verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);
 
   // These tests are here to show a problem that may not be easily
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2602,16 +2602,17 @@
   // Handle AttributeMacro, e.g. `if (x) UNLIKELY`.
   if (FormatTok->is(TT_AttributeMacro))
 nextToken();
-  handleCppAttributes();
+  if (FormatTok->is(tok::l_square))
+handleCppAttributes();
 }
 
 bool UnwrappedLineParser::handleCppAttributes() {
   // Handle [[likely]] / [[unlikely]] attributes.
-  if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute()) {
-parseSquare();
-return true;
-  }
-  return false;
+  assert(FormatTok->is(tok::l_square));
+  if (!tryToParseSimpleAttribute())
+return false;
+  parseSquare();
+  return true;
 }
 
 /// Returns whether \c Tok begins a block.
@@ -3728,13 +3729,13 @@
 void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   const FormatToken &InitialToken = *FormatTok;
   nextToken();
-  handleAttributes();
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
+  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, tok::l_square) ||
  ((Style.Language == FormatStyle::LK_Java || Style.isJavaScript()) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
 if (Style.isJavaScript() &&
@@ -3748,16 +3749,15 @@
 continue;
   }
 }
+if (FormatTok->is(tok::l_square) && handleCppAttributes())
+  continue;
 bool IsNonMacroIdentifier =
 FormatTok->is(tok::identifier) &&
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
 // We can have macros in between 'class' and the class name.
-if (!IsNonMacroIdentifier) {
-  if (FormatTok->is(tok::l_paren)) {
-parseParens();
-  }
-}
+if (!IsNonMacroIdentifier && FormatTok->is(tok::l_paren))
+  parseParens();
   }
 
   // Note that parsing away template declarations here leads to incorrectly


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,14 @@
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct EXPORT_MACRO [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct [[deprecated]] [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
Index: clang/unittests/Format/FormatTest.cpp
=

[clang] 767aee1 - [clang-format] Don't annotate left brace of struct as FunctionLBrace

[Owen Pan via cfe-commits]

Author: Owen Pan
Date: 2023-03-27T14:07:15-07:00
New Revision: 767aee1de9e98256a62ae8b4c2f84381203613c3

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

LOG: [clang-format] Don't annotate left brace of struct as FunctionLBrace

Related to a02c3af9f19d. Fixes #61700.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 9b17c28280f19..3a661292b2d72 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2602,16 +2602,17 @@ void UnwrappedLineParser::handleAttributes() {
   // Handle AttributeMacro, e.g. `if (x) UNLIKELY`.
   if (FormatTok->is(TT_AttributeMacro))
 nextToken();
-  handleCppAttributes();
+  if (FormatTok->is(tok::l_square))
+handleCppAttributes();
 }
 
 bool UnwrappedLineParser::handleCppAttributes() {
   // Handle [[likely]] / [[unlikely]] attributes.
-  if (FormatTok->is(tok::l_square) && tryToParseSimpleAttribute()) {
-parseSquare();
-return true;
-  }
-  return false;
+  assert(FormatTok->is(tok::l_square));
+  if (!tryToParseSimpleAttribute())
+return false;
+  parseSquare();
+  return true;
 }
 
 /// Returns whether \c Tok begins a block.
@@ -3728,13 +3729,13 @@ void UnwrappedLineParser::parseJavaEnumBody() {
 void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   const FormatToken &InitialToken = *FormatTok;
   nextToken();
-  handleAttributes();
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
+  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas) ||
+tok::kw_alignas, tok::l_square) ||
  ((Style.Language == FormatStyle::LK_Java || Style.isJavaScript()) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
 if (Style.isJavaScript() &&
@@ -3748,16 +3749,15 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) 
{
 continue;
   }
 }
+if (FormatTok->is(tok::l_square) && handleCppAttributes())
+  continue;
 bool IsNonMacroIdentifier =
 FormatTok->is(tok::identifier) &&
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
 // We can have macros in between 'class' and the class name.
-if (!IsNonMacroIdentifier) {
-  if (FormatTok->is(tok::l_paren)) {
-parseParens();
-  }
-}
+if (!IsNonMacroIdentifier && FormatTok->is(tok::l_paren))
+  parseParens();
   }
 
   // Note that parsing away template declarations here leads to incorrectly

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 80efa8c4babfc..1c960eb28ef94 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25308,6 +25308,11 @@ TEST_F(FormatTest, RemoveSemicolon) {
"};",
Style);
 
+  verifyFormat("struct EXPORT_MACRO [[nodiscard]] C {\n"
+   "  int i;\n"
+   "};",
+   Style);
+
   verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);
 
   // These tests are here to show a problem that may not be easily

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 94b3a77ed5a00..b058e62654551 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -374,6 +374,14 @@ TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
   auto Tokens = annotate("struct S {};");
   EXPECT_EQ(Tokens.size(), 6u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct EXPORT_MACRO [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_StructLBrace);
+
+  Tokens = annotate("struct [[deprecated]] [[nodiscard]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_StructLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUnions) {



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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

jhuber6 wrote:
> sivachandra wrote:
> > tra wrote:
> > > sivachandra wrote:
> > > > jhuber6 wrote:
> > > > > tra wrote:
> > > > > > jhuber6 wrote:
> > > > > > > tra wrote:
> > > > > > > > Ensuring the right include order will be tricky. Interaction 
> > > > > > > > between the headers provided by llvm-libc vs the system headers 
> > > > > > > > will be interesting if we end up mixing the headers. It may be 
> > > > > > > > less problematic compared to the C++ standard library, but I 
> > > > > > > > doubt that mixing two implementations would work well here, 
> > > > > > > > either. 
> > > > > > > > 
> > > > > > > > So, the major question I have -- does adding include path here 
> > > > > > > > guarantees that we're not picking up the host headers? I do not 
> > > > > > > > see any changes that would exclude system headers from include 
> > > > > > > > paths.
> > > > > > > > If we do have include paths leading to both llvm-libc and the 
> > > > > > > > host headers, what's expected to happen if user code ends up 
> > > > > > > > including a header that's not present in  llvm-libc? Is that 
> > > > > > > > possible?
> > > > > > > > 
> > > > > > > Right now I'm just kind of relying on an expectation that since 
> > > > > > > this will be the first `c-isystem` path set, then it will pull in 
> > > > > > > these libraries first if they exist. It's not captured by the 
> > > > > > > tests, but compiling with `-v` shows this path being used first 
> > > > > > > in my experience. So, theoretically, if there is an 
> > > > > > > implementation of said header in this location, it will be picked 
> > > > > > > up before anything else. Otherwise it'll just search the other 
> > > > > > > standard locations.
> > > > > > I think this will be a problem. We're cross-compiling here and for 
> > > > > > that to work reliably we need to make sure that only target headers 
> > > > > > are in effect. The problem is that we likely just do not have 
> > > > > > sufficiently complete set of headers for the GPU. Do we? I have no 
> > > > > > idea what exactly llvm-libc provides and whether it is sufficient 
> > > > > > for normal user code to cross-compile for a GPU. 
> > > > > > 
> > > > > > It would be interesting to try to compile some C++ code which would 
> > > > > > include commonly used, but generally target-agnostic, headers like 
> > > > > >   , etc and check whether we end up 
> > > > > > pulling in any system headers. Then check what happens if we do not 
> > > > > > have system headers available at all.
> > > > > No, it's definitely not complete. Some headers work on the GPU, most 
> > > > > break in some way or another. The only ones `llvm-libc` provides 
> > > > > currently is `string.h` and `ctype.h`. But, I figured this wouldn't 
> > > > > be a problem since it would just go to the system headers anyway if 
> > > > > we didn't provide them. So we are merely replacing maybe broken with 
> > > > > probably works.
> > > > > 
> > > > > I was talking with Johannes and he brings up other issues about the 
> > > > > idea of host-device compatibility between these headers. Since, 
> > > > > fundamentally, right now `libc` generates its own headers and needs 
> > > > > to generate its own headers to function. But there can be a problem 
> > > > > when migrating data between the host and the device is the headers on 
> > > > > the host differ somewhat to those on the device. I'm not sure what a 
> > > > > good overall solution to that problem is.
> > > > Normally, one should not expect target and host headers to be 
> > > > compatible. So, if you are building for the host, you should use host 
> > > > headers and if you are building for the target, you should use target 
> > > > headers. Does general GPU build not follow this approach? May be there 
> > > > are some common headers but I do not expect them to be from the 
> > > > standard libraries.
> > > We can generally assume that the GPU and the host do have largely 
> > > identical types. At least the subset of the types we expect to exchange 
> > > between host and GPU.
> > > CUDA compilation cheats, and allows the host to provide most of the 
> > > headers, with clang and CUDA SDK providing a subset of GPU-side 
> > > overloads. This way, if GPU-side functions do implicitly rely on the code 
> > > nominally provided for the host by the host headers, but if we need to 
> > > code-gen it, we can only do so for a subset that's actually compileable 
> > > for the GPU -- either constexpr functions, lambdas or `__device__` 
> > > overloads provided by us.
> > > 
> > > Standalone compilation does not have the benefit of having the cake and 
> > > being able to eat it. It has to be all or nothing, as we do not have the 
> > >

[PATCH] D146814: [Flang] Add debug flag to enable current debug information pass

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thanks for the updates! A few more pointers, but nothing major.

Btw, are there any tests that check for debug info in the compiled files? For 
example:

  ! RUN: flang -g1 -S %s | FileCheck -check-prefixes=DEBUG-INFO-PRESENT
  ! RUN: flang -g0 -S %s | FileCheck -check-prefixes=DEBUG-INFO-MISSING
  
  end program




Comment at: clang/lib/Driver/ToolChains/Flang.cpp:32
+static void
+addFortranDebugOptions(ArgStringList &CmdArgs,
+   llvm::codegenoptions::DebugInfoKind DebugInfoKind) {

There isn't anything Fortran specific here, is there? And this method basically 
implements 
https://github.com/llvm/llvm-project/blob/cf60d3f1a688671c8eb7859bf0572c403c3c0cca/clang/lib/Driver/ToolChains/Clang.cpp#L975-L1000,
 right? Why not extract it into e.g. `renderDebugEnablingArgs` and move to 
CommonArgs.cpp?



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:868
   parsePreprocessorArgs(res.getPreprocessorOpts(), args);
-  parseCodeGenArgs(res.getCodeGenOpts(), args, diags);
+  success &= parseCodeGenArgs(res.getCodeGenOpts(), args, diags);
   success &= parseSemaArgs(res, args, diags);

WDYT? I think that it makes sense to keep these separate.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:144
+val != llvm::codegenoptions::NoDebugInfo) {
+  // TODO: This is not a great warning message, could be improved
+  const auto debugErr = diags.getCustomDiagID(

SBallantyne wrote:
> awarzynski wrote:
> > Please improve it :)
> > 
> > In particular, you are testing for `DebugLineTablesOnly` and `NoDebugInfo`, 
> > yet the diagnostic refers to `-g1`.
> I previously had it emit these debug names, but i think its more confusing 
> for the user as they will be passing `-g2 / -g3` in order to get this error, 
> and the internal name is not as helpful. The TODO was from a previous patch 
> and i just forgot to remove it, i am happy with the current state of warning. 
Thanks for the explanation! My main reservation is that it's not obvious how 
`-g2` and `-g3` map onto `llvm::codegenoptions`. This warnings refers to `-g1`, 
but there's no mention of `-g1` in this function. 

TBH, I would replace "Debug options greater than -g1 not yet implemented" with 
something a bit more generic and future-proof, e.g. `"Unsupported debug option: 
%0" << arg.getValue()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146814

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

sivachandra wrote:
> tra wrote:
> > sivachandra wrote:
> > > jhuber6 wrote:
> > > > tra wrote:
> > > > > jhuber6 wrote:
> > > > > > tra wrote:
> > > > > > > Ensuring the right include order will be tricky. Interaction 
> > > > > > > between the headers provided by llvm-libc vs the system headers 
> > > > > > > will be interesting if we end up mixing the headers. It may be 
> > > > > > > less problematic compared to the C++ standard library, but I 
> > > > > > > doubt that mixing two implementations would work well here, 
> > > > > > > either. 
> > > > > > > 
> > > > > > > So, the major question I have -- does adding include path here 
> > > > > > > guarantees that we're not picking up the host headers? I do not 
> > > > > > > see any changes that would exclude system headers from include 
> > > > > > > paths.
> > > > > > > If we do have include paths leading to both llvm-libc and the 
> > > > > > > host headers, what's expected to happen if user code ends up 
> > > > > > > including a header that's not present in  llvm-libc? Is that 
> > > > > > > possible?
> > > > > > > 
> > > > > > Right now I'm just kind of relying on an expectation that since 
> > > > > > this will be the first `c-isystem` path set, then it will pull in 
> > > > > > these libraries first if they exist. It's not captured by the 
> > > > > > tests, but compiling with `-v` shows this path being used first in 
> > > > > > my experience. So, theoretically, if there is an implementation of 
> > > > > > said header in this location, it will be picked up before anything 
> > > > > > else. Otherwise it'll just search the other standard locations.
> > > > > I think this will be a problem. We're cross-compiling here and for 
> > > > > that to work reliably we need to make sure that only target headers 
> > > > > are in effect. The problem is that we likely just do not have 
> > > > > sufficiently complete set of headers for the GPU. Do we? I have no 
> > > > > idea what exactly llvm-libc provides and whether it is sufficient for 
> > > > > normal user code to cross-compile for a GPU. 
> > > > > 
> > > > > It would be interesting to try to compile some C++ code which would 
> > > > > include commonly used, but generally target-agnostic, headers like 
> > > > >   , etc and check whether we end up 
> > > > > pulling in any system headers. Then check what happens if we do not 
> > > > > have system headers available at all.
> > > > No, it's definitely not complete. Some headers work on the GPU, most 
> > > > break in some way or another. The only ones `llvm-libc` provides 
> > > > currently is `string.h` and `ctype.h`. But, I figured this wouldn't be 
> > > > a problem since it would just go to the system headers anyway if we 
> > > > didn't provide them. So we are merely replacing maybe broken with 
> > > > probably works.
> > > > 
> > > > I was talking with Johannes and he brings up other issues about the 
> > > > idea of host-device compatibility between these headers. Since, 
> > > > fundamentally, right now `libc` generates its own headers and needs to 
> > > > generate its own headers to function. But there can be a problem when 
> > > > migrating data between the host and the device is the headers on the 
> > > > host differ somewhat to those on the device. I'm not sure what a good 
> > > > overall solution to that problem is.
> > > Normally, one should not expect target and host headers to be compatible. 
> > > So, if you are building for the host, you should use host headers and if 
> > > you are building for the target, you should use target headers. Does 
> > > general GPU build not follow this approach? May be there are some common 
> > > headers but I do not expect them to be from the standard libraries.
> > We can generally assume that the GPU and the host do have largely identical 
> > types. At least the subset of the types we expect to exchange between host 
> > and GPU.
> > CUDA compilation cheats, and allows the host to provide most of the 
> > headers, with clang and CUDA SDK providing a subset of GPU-side overloads. 
> > This way, if GPU-side functions do implicitly rely on the code nominally 
> > provided for the host by the host headers, but if we need to code-gen it, 
> > we can only do so for a subset that's actually compileable for the GPU -- 
> > either constexpr functions, lambdas or `__device__` overloads provided by 
> > us.
> > 
> > Standalone compilation does not have the benefit of having the cake and 
> > being able to eat it. It has to be all or nothing, as we do not have the 
> > ability to separate the host and GPU code we pull in via headers, nor can 
> > we provide a GPU-side overloads. In a way injecting llvm-libc path is a 
> > crude attempt to do that by 

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> We don't want to have one attribute per debug format, because that's not 
> going to scale.

LLVM supports outputting a total of 2 debug info formats.

> So how do we validate that this attribute should be exposed to users *now* 
> before we know what the story is for other debug formats?

Do you have any other debug info formats in mind? To my knowledge LLVM doesn't 
support lowering all of the the debug info metadata into PDB because there are 
some corners where it is less expressive than DWARF.
I believe that the closest equivalent to this feature in PDB is a feature 
called "just my code". A web search came up with 
https://learn.microsoft.com/en-us/visualstudio/debugger/just-my-code?view=vs-2022
 and it seems like it's powered by an external database of function names. 
Would be interesting to hear from someone with knowledge and an interest in PDB 
about this.

Would you say that we shouldn't create this attribute if it cannot be supported 
on Windows? Or would documenting that the attribute only has an effect on DWARF 
platforms be sufficient?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146777: [clang] Preliminary fat-lto-object support

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 508773.
paulkirth retitled this revision from "[clang] Preliminary fat-lot-object 
support" to "[clang] Preliminary fat-lto-object support".
paulkirth added a comment.
Herald added a subscriber: inglorion.

Fix typo in title


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/embed-lto-fatlto.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fatlto-objects.c

Index: clang/test/Driver/fatlto-objects.c
===
--- /dev/null
+++ clang/test/Driver/fatlto-objects.c
@@ -0,0 +1,10 @@
+// REQUIRES: x86_64-linux
+// RUN: %clang -target x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// CHECK-CC: -cc1
+// CHECK-CC: -emit-obj
+// CHECK-CC: -ffat-lto-objects
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1
+// CHECK-CC-NOLTO: -emit-obj
+// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -421,7 +421,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fwhole-program' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
-// CHECK-WARNING-DAG: optimization flag '-ffat-lto-objects' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fmerge-constants' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-small-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-ftree-dce' is not supported
Index: clang/test/CodeGen/embed-lto-fatlto.c
===
--- /dev/null
+++ clang/test/CodeGen/embed-lto-fatlto.c
@@ -0,0 +1,9 @@
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+//
+// CHECK: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}
+// CHECK: !{{[0-9]+}} = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+
+int test(void) {
+  return 0xabcd;
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7276,6 +7276,14 @@
 }
   }
 
+  if (IsUsingLTO && Args.getLastArg(options::OPT_ffat_lto_objects)) {
+assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-flto=") + (LTOMode == LTOK_Thin ? "thin" : "full")));
+CmdArgs.push_back("-flto-unit");
+CmdArgs.push_back("-ffat-lto-objects");
+  }
+
   if (Args.hasArg(options::OPT_forder_file_instrumentation)) {
  CmdArgs.push_back("-forder-file-instrumentation");
  // Enable order file instrumentation when ThinLTO is not on. When ThinLTO is
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4641,8 +4641,13 @@
   }
   case phases::Backend: {
 if (isUsingLTO() && TargetDeviceOffloadKind == Action::OFK_None) {
-  types::ID Output =
-  Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC;
+  types::ID Output;
+  if (Args.hasArg(options::OPT_S))
+Output = types::TY_LTO_IR;
+  else if (Args.hasArg(options::OPT_ffat_lto_objects))
+Output = types::TY_PP_Asm;
+  else
+Output = types::TY_LTO_BC;
   return C.MakeAction(Input, Output);
 }
 if (isUsingLTO(/* IsOffload */ true) &&
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
+#include "llvm/Bitcode/EmbedBitcodePass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -973,7 +974,13 @@
 MPM.addPass(InstrProfiling(*Options, false));
   });
 
-if (IsThinLTO) {
+if (CodeGenOpts.OptimizationLevel == 0) {
+  MPM = PB.buildO0DefaultPipeline(Level, IsLTO || IsThinLTO);
+} else if (CodeGenOpts.FatLTO

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

[Paul Robinson via Phabricator via cfe-commits]

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

One entirely optional suggestion on the test. LGTM.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1563
+
+  assert(PreviousBitfield->isBitField());
+

jmmartinez wrote:
> probinson wrote:
> > Is this true for cases like
> > ```
> > struct nonadjacent {
> >   char a : 8;
> >   char : 0;
> >   int b;
> >   char d : 8;
> > };
> > ```
> > where the field `d` has a predecessor that is not a bitfield? (This might 
> > be my ignorance of how Decls are put together, but asserting that `advance` 
> > is guaranteed to get you a bitfield just seems a little odd.)
> In that case the assert is never reached.
> 
> When emiting the debug-info for `d`, when looking at the metadata generated 
> for the previous field the function should exit early on this condition:
> ```
>   if (!PreviousMDField || !PreviousMDField->isBitField() ||
>   PreviousMDField->getSizeInBits() == 0)
> return nullptr;
> ```
Ah, you are right. Thanks!



Comment at: clang/test/CodeGen/debug-info-bitfield-0-struct.c:40
+struct SecondDuplicate {
+  // BOTH-DAG: ![[SECONDD:[0-9]+]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "SecondDuplicate", file: !{{[0-9]+}}, line: 
{{[0-9]+}}, size: 64, elements: ![[SECONDD_ELEMENTS:[0-9]+]])
+

Maybe use something like `SECONDDUP` instead of `SECONDD` which can look like a 
typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144870

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

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

I've removed the special-casing that I added for namespace scope statements. If 
I make those changes at all, I'll raise them as a follow up diff.




Comment at: clang/unittests/Format/FormatTest.cpp:21953
-  verifyFormat("std::sort(v.begin(), v.end(),\n"
-   "  [](const auto &someLongArgumentName, const auto "
-   "&someOtherLongArgumentName) {\n"

MyDeveloperDay wrote:
> I would think the point here is to test someLongArgumentName not to use a 
> small variable like 'v'
These changes have been moved to D146995 now, but I'll respond here: the long 
variable names were (presumably) being used to cause a line break. Since I've 
reduced the column limit from 100 to 60, `v` is now sufficient (and less noisy 
to read).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146042

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

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 508765.
jp4a50 added a comment.

Re-enable OuterScope lambda body indentation of lambdas in namespace scope 
statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146042

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21967,9 +21967,9 @@
   verifyFormat("void test() {\n"
"  (\n"
"  []() -> auto {\n"
-   "int b = 32;\n"
-   "return 3;\n"
-   "  },\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
"  foo, bar)\n"
"  .foo();\n"
"}",
@@ -21983,17 +21983,82 @@
"  .bar();\n"
"}",
Style);
-  Style = getGoogleStyle();
-  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
-  verifyFormat("#define A   \\\n"
-   "  [] {  \\\n"
-   "xxx(\\\n"
-   "x); \\\n"
-   "  }",
-   Style);
-  // TODO: The current formatting has a minor issue that's not worth fixing
-  // right now whereby the closing brace is indented relative to the signature
-  // instead of being aligned. This only happens with macros.
+  verifyFormat("#define A  \\\n"
+   "  [] { \\\n"
+   "xxx(   \\\n"
+   "x);\\\n"
+   "  }",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  aFunction(1, b(c(foo, bar, baz, [](d) {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  })));\n"
+   "}\n",
+   Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  1, b(c(\n"
+   " [](d) -> Foo {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  },\n"
+   " foo, Bar{},\n"
+   " [] {\n"
+   "auto g = h();\n"
+   "return g;\n"
+   "  },\n"
+   " baz)));\n"
+   "}\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  aFunction(1, b(c(foo, Bar{}, baz, [](d) -> Foo {\n"
+   "auto f = e(\n"
+   "foo,\n"
+   "[&] {\n"
+   "  auto g = h();\n"
+   "  return g;\n"
+   "},\n"
+   "qux,\n"
+   "[&] -> Bar {\n"
+   "  auto i = j();\n"
+   "  return i;\n"
+   "});\n"
+   "return f;\n"
+   "  })));\n"
+   "}\n",
+   Style);
+  verifyFormat("Namespace::Foo::Foo(\n"
+   "LongClassName bar, AnotherLongClassName baz)\n"
+   ": baz{baz}, func{[&] {\n"
+   "  auto qux = bar;\n"
+   "  return aFunkyFunctionCall(qux);\n"
+   "}} {}\n",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  1, b(c(foo, Bar{}, baz,\n"
+   " [](d) -> Foo\n"
+   "  {\n"
+   "auto f = e(\n"
+   "[&]\n"
+   "{\n"
+   "  auto g = h();\n"
+   "  return g;\n"
+   "},\n"
+   "qux,\n"
+   "[&] -> Bar\n"
+   "{\n"
+   "  auto i = j();\n"
+   "  return i;\n"
+   "});\n"
+   "return f;\n"
+   "  })));\n"
+   "}\n",
+   Style);
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> Why should this feature be limited to DWARF? Don't we have the same kind of 
> stepping behavior issue with PDB files, for example?

That's not what I was trying to say. Yes, `DW_AT_trampoline` is a DWARF 
feature. But the "target function" LLVM IR feature is not necessarily limited 
to lowering into DWARF. If someone wants to implement lowering of the LLVM IR 
feature to PDB, and PDB supports some equivalent constructs, they can certainly 
do so.

I just wanted to point out that the goal of this patch is to give Clang users 
access to an already existing LLVM (& DWARF) feature that is being used by 
other frontends (flang) and supported by other debuggers. While I agree with 
David's general point that the feature is somewhat limited in its design, most 
of the design choices ultimately come from the DWARF specification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:21952
"}",
Style);
+  verifyFormat("void test() {\n"

MyDeveloperDay wrote:
> I feel like lots of these test changes could be made on their own, BEFORE 
> changing the code, I would feel more comfortable about that I think
Pulled them out into D146995.



Comment at: clang/unittests/Format/FormatTest.cpp:21916
 
   // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(60);

HazardyKnusperkeks wrote:
> jp4a50 wrote:
> > MyDeveloperDay wrote:
> > > jp4a50 wrote:
> > > > MyDeveloperDay wrote:
> > > > > Why are you changing existing tests
> > > > Sorry about the rather large diff here.
> > > > 
> > > > I have made the following changes to existing tests (all for what I 
> > > > think are good reasons):
> > > > 
> > > >   - Reduced the column limit and then reduced the verbosity of the code 
> > > > samples in the test to make them more readable. Before the column limit 
> > > > was 100 and the code samples were very long. I found this made them 
> > > > unnecessarily difficult to read, especially because some of the lines 
> > > > of the code sample would wrap before a newline in the code itself, 
> > > > which was confusing.
> > > >   - Changed a number of tests written in the form `EXPECT_EQ(code, 
> > > > format(otherCode))` to simply `verifyFormat(code)` because the latter 
> > > > is much shorter and easier to read. If you can think of a good reason 
> > > > to favour the former over the latter then let me know but it just 
> > > > seemed like unnecessary duplication to me.
> > > >   - Some of the tests weren't syntactically valid C++ e.g. the tests at 
> > > > line 21939 which I have changed to have a valid (rather than 
> > > > incomplete) trailing return type.
> > > >   - The macro test actually captured a bug so I had to change the code 
> > > > sample there to reflect the now fixed behaviour.
> > > > 
> > > > I also added one or two more tests at the bottom to capture more 
> > > > complex cases like when more than one argument to a function call is a 
> > > > lambda. 
> > > > 
> > > > 
> > > refactoring the tests is one thing, but not at the same time you are 
> > > modifying how they are formatted. Could it be a separate commit so we can 
> > > actually see what is changing?
> > > 
> > > I don't deny what is there before doesn't look 100% correct, but I've 
> > > always worked on the Beyoncé rule  (If you liked it you should have put a 
> > > test on it), meaning... someone put a test there to assert some form of 
> > > formatting behaviour, you are now changing that to be your desired 
> > > behaviour, that break the rule (IMHO)
> > > 
> > > we have to agree what was there before wasn't correct, but your changes 
> > > don't give me that confidence, 
> > > refactoring the tests is one thing, but not at the same time you are 
> > > modifying how they are formatted. Could it be a separate commit so we can 
> > > actually see what is changing?
> > 
> > Sure! I assume that this implies two separate Phabricator reviews/diffs? Or 
> > is there somehow a way to show two "commits" within a single review? Sorry, 
> > I'm not used to using Phrabricator.
> > 
> > > I don't deny what is there before doesn't look 100% correct, but I've 
> > > always worked on the Beyoncé rule  (If you liked it you should have put a 
> > > test on it), meaning... someone put a test there to assert some form of 
> > > formatting behaviour, you are now changing that to be your desired 
> > > behaviour, that break the rule (IMHO)
> > 
> > The macro example I was referring to as a "bug" actually had a comment 
> > below it which seemed to acknowledge an issue with the previous 
> > implementation. Now I'm not 100% sure that that comment was referring to 
> > the behaviour in the macro example but what does seem clear is that the 
> > behaviour shown in the macro test case is not desirable in the case of 
> > using `OuterScope`. I just can't see how that behaviour could be justified 
> > as being intentional/desirable. If you wanted, I could reach out to the 
> > original author of that test and ask them since they still work at the same 
> > company.
> > 
> > > we have to agree what was there before wasn't correct, but your changes 
> > > don't give me that confidence, 
> > 
> > I'd appreciate if you could elaborate on this - perhaps you're referring 
> > mainly to the macro example but I'm not sure.
> Each commit gets its own differential, there are no multiple commit reviews 
> like pull requests. You can add a parent or child commit to show the 
> dependency, if any.
Pulled out test refactoring (excluding functional changes) into D146995.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146042

___
c

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 508762.
jp4a50 added a comment.

Rebase diff on top of D146995 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146042

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21959,17 +21959,19 @@
"  }\n"
"}",
Style);
-  verifyFormat("std::sort(v.begin(), v.end(),\n"
-   "  [](const auto &foo, const auto &bar) {\n"
-   "  return foo.baz < bar.baz;\n"
-   "});\n",
+  verifyFormat("void test() {\n"
+   "  std::sort(v.begin(), v.end(),\n"
+   "[](const auto &foo, const auto &bar) {\n"
+   "return foo.baz < bar.baz;\n"
+   "  });\n"
+   "}\n",
Style);
   verifyFormat("void test() {\n"
"  (\n"
"  []() -> auto {\n"
-   "int b = 32;\n"
-   "return 3;\n"
-   "  },\n"
+   "int b = 32;\n"
+   "return 3;\n"
+   "  },\n"
"  foo, bar)\n"
"  .foo();\n"
"}",
@@ -21983,17 +21985,82 @@
"  .bar();\n"
"}",
Style);
-  Style = getGoogleStyle();
-  Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
-  verifyFormat("#define A   \\\n"
-   "  [] {  \\\n"
-   "xxx(\\\n"
-   "x); \\\n"
-   "  }",
-   Style);
-  // TODO: The current formatting has a minor issue that's not worth fixing
-  // right now whereby the closing brace is indented relative to the signature
-  // instead of being aligned. This only happens with macros.
+  verifyFormat("#define A  \\\n"
+   "  [] { \\\n"
+   "xxx(   \\\n"
+   "x);\\\n"
+   "  }",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  aFunction(1, b(c(foo, bar, baz, [](d) {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  })));\n"
+   "}\n",
+   Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  1, b(c(\n"
+   " [](d) -> Foo {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  },\n"
+   " foo, Bar{},\n"
+   " [] {\n"
+   "auto g = h();\n"
+   "return g;\n"
+   "  },\n"
+   " baz)));\n"
+   "}\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  aFunction(1, b(c(foo, Bar{}, baz, [](d) -> Foo {\n"
+   "auto f = e(\n"
+   "foo,\n"
+   "[&] {\n"
+   "  auto g = h();\n"
+   "  return g;\n"
+   "},\n"
+   "qux,\n"
+   "[&] -> Bar {\n"
+   "  auto i = j();\n"
+   "  return i;\n"
+   "});\n"
+   "return f;\n"
+   "  })));\n"
+   "}\n",
+   Style);
+  verifyFormat("Namespace::Foo::Foo(\n"
+   "LongClassName bar, AnotherLongClassName baz)\n"
+   ": baz{baz}, func{[&] {\n"
+   "auto qux = bar;\n"
+   "return aFunkyFunctionCall(qux);\n"
+   "  }} {}\n",
+   Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  1, b(c(foo, Bar{}, baz,\n"
+   " [](d) -> Foo\n"
+   "  {\n"
+   "auto f = e(\n"
+   "[&]\n"
+   "{\n"
+   "  auto g = h();\n"
+   "  return g;\n"
+   "},\n"
+  

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea added a comment.

Nit: the diffs would be easier to analyze with "-fno-discard-value-names".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145265

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> I guess it was implemented there first/ported to clang with the fortran effort

Yeah my understanding is that the LLVM feature was added for flang, and so I'm 
not sure what the targeted debugger is, I believe there are some non-GDB/LLDB 
debuggers targeting the HPC market that might implement this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D146595#4225259 , @augusto2112 
wrote:

> In D146595#4225170 , @aaron.ballman 
> wrote:
>
>> In D146595#4225132 , @aprantl 
>> wrote:
>>
>>> In D146595#4225048 , @dblaikie 
>>> wrote:
>>>
 I know this is a bit of a redirection/scope creep/etc - but I'd quite like 
 to see a solution that is likely to be usable for the "std::function" 
 problem (stepping into std::function should allow you to reach the 
 underlying function - but lldb currently skips any call to a 
 std-namespaced function, I think, so you step right over the whole op() 
 call) that could also cover the Swift needs. Though perhaps they're just 
 sufficiently different problems that there is no generalizing here.
>>>
>>> This patch aims at exposing an existing LLVM IR & DWARF feature in clang. 
>>> Having a generic solution for the std::function problem is definitely 
>>> worthwhile investigating, but I'm not sure it needs to prevent supporting 
>>> the existing mechanism in clang.
>>
>> Why should this feature be limited to DWARF? Don't we have the same kind of 
>> stepping behavior issue with PDB files, for example?
>
> There's already support to threading trampoline names from IR to DWARF. If 
> PDB supports the same attribute in some form there's nothing stopping someone 
> to adding support to thread the attribute from IR to PDB as well.

We don't want to have one attribute per debug format, because that's not going 
to scale. So how do we validate that this attribute should be exposed to users 
*now* before we know what the story is for other debug formats? The argument 
"this is for compiler-generated code" doesn't really address the concern I have 
here -- once the attribute exists, people will start using it if it does useful 
things. The problem this attribute solves is pretty general to a lot of 
different kinds of library facilities (at least in C++, a bit less so in C 
IMO), so it seems that hypothetical situation is plausible. We can document 
that this only works with DWARF, but that's still not actually solving the 
problem users have.

I guess what I'm trying to say is: this feels like a specific solution to a 
general problem and that makes me worried we're painting ourselves into a 
corner where we're going to need to deprecate this attribute and add the 
general one in the future. How likely do you think that is (might be more of a 
question for @dblaikie from the debug info side of things) and do you agree 
that it seems like users would want this functionality for their own libraries?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D146973#4225300 , @tschuett wrote:

> Could you hide the amdgpu and nvptx somewhere libc here `clang 
> -print-resource-dir` in two different directories?  One for AMD, one for 
> NVPTX.

So, right now this header is installed from the `libc` projects. So is there a 
good way to communicate the resource directory as an install target? Also I 
think just calling it `gpu` would be sufficient, I'd like these to be common 
between the GPUs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146995: [clang-format] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:21978
Style);
-  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
-"  return promise.then(\n"

The refactored version of these test cases has been moved up to immediately 
follow the setting of `OuterScope` so that the result can be compared with the 
identical code sample above which doesn't use `OuterScope`. It was odd that 
these weren't grouped together before.



Comment at: clang/unittests/Format/FormatTest.cpp:21994
+   Style);
   // TODO: The current formatting has a minor issue that's not worth fixing
   // right now whereby the closing brace is indented relative to the signature

This is the comment that I believe is calling out the seemingly broken 
formatting behaviour in the test case directly above it. I think it's pretty 
clear that the code in that test case should not be formatted that way when 
`OuterScope` is set (or even without it - the formatting just doesn't make any 
sense really).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146995

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Could you hide the amdgpu and nvptx somewhere here `clang -print-resource-dir` 
in two different directories?  One for AMD, one for NVPTX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146995: [clang-format] Refactor unit tests for "LambdaBodyIndentation: OuterScope" option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 created this revision.
Herald added a project: All.
jp4a50 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The changes:

- make the tests more concise
- fix invalid C++ in the code samples
- ensure line breaks in tests' code samples correspond to line breaks in the 
test code itself for the avoidance of confusion


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146995

Files:
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21914,51 +21914,59 @@
LLVMWithBeforeLambdaBody);
 
   // Lambdas with different indentation styles.
-  Style = getLLVMStyleWithColumns(100);
-  EXPECT_EQ("SomeResult doSomething(SomeObject promise) {\n"
-"  return promise.then(\n"
-"  [this, &someVariable, someObject = "
-"std::mv(s)](std::vector evaluated) mutable {\n"
-"return someObject.startAsyncAction().then(\n"
-"[this, &someVariable](AsyncActionResult result) "
-"mutable { result.processMore(); });\n"
-"  });\n"
-"}\n",
-format("SomeResult doSomething(SomeObject promise) {\n"
-   "  return promise.then([this, &someVariable, someObject = "
-   "std::mv(s)](std::vector evaluated) mutable {\n"
-   "return someObject.startAsyncAction().then([this, "
-   "&someVariable](AsyncActionResult result) mutable {\n"
-   "  result.processMore();\n"
-   "});\n"
-   "  });\n"
-   "}\n",
-   Style));
+  Style = getLLVMStyleWithColumns(60);
+  verifyFormat("Result doSomething(Promise promise) {\n"
+   "  return promise.then(\n"
+   "  [this, obj = std::move(s)](int bar) mutable {\n"
+   "return someObject.startAsyncAction().then(\n"
+   "[this, &obj](Result result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style);
   Style.LambdaBodyIndentation = FormatStyle::LBI_OuterScope;
-  verifyFormat("test() {\n"
-   "  ([]() -> {\n"
+  verifyFormat("Result doSomething(Promise promise) {\n"
+   "  return promise.then(\n"
+   "  [this, obj = std::move(s)](int bar) mutable {\n"
+   "return obj.startAsyncAction().then(\n"
+   "[this, &obj](Result result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style);
+  verifyFormat("Result doSomething(Promise promise) {\n"
+   "  return promise.then([this, obj = std::move(s)] {\n"
+   "return obj.startAsyncAction().then(\n"
+   "[this, &obj](Result result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"
+   "  });\n"
+   "}\n",
+   Style);
+  verifyFormat("void test() {\n"
+   "  ([]() -> auto {\n"
"int b = 32;\n"
"return 3;\n"
"  }).foo();\n"
"}",
Style);
-  verifyFormat("test() {\n"
-   "  []() -> {\n"
+  verifyFormat("void test() {\n"
+   "  []() -> auto {\n"
"int b = 32;\n"
"return 3;\n"
"  }\n"
"}",
Style);
   verifyFormat("std::sort(v.begin(), v.end(),\n"
-   "  [](const auto &someLongArgumentName, const auto "
-   "&someOtherLongArgumentName) {\n"
-   "  return someLongArgumentName.someMemberVariable < "
-   "someOtherLongArgumentName.someMemberVariable;\n"
-   "});",
+   "  [](const auto &foo, const auto &bar) {\n"
+   "  return foo.baz < bar.baz;\n"
+   "});\n",
Style);
-  verifyFormat("test() {\n"
+  verifyFormat("void test() {\n"
"  (\n"
-   "  []() -> {\n"
+   "  []() -> auto {\n"
"int b = 32;\n"
"return 3;\n"
"  },\n"
@@ -21966,8 +21974,8 @@
"  .foo();\n"
"}",
Style);
-  verifyFormat("test() {\n"
-   "  ([]() -> {\n"
+  verifyFormat("void test() {\n"
+   "  ([]() -> auto {\n"
"int b = 32;\n"
"return 3;\n"
"  })\n"
@@ -21975,51 +21983,14 @@
 

[PATCH] D146986: Downgrade reserved module identifier error into a warning

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D146986#4225192 , @aaron.ballman 
wrote:

> In D146986#4225121 , @dblaikie 
> wrote:
>
>> From the discussion on the issue:
>>
>>> Do we want this loosening of the restriction to apply to *only* `std` and 
>>> the same followed by a number, or to any reserved identifier used in a 
>>> module? e.g.,
>>>
>>>   module std; // error today, but will become a warning
>>>   module _Test; // error today, but do we want this to become a warning as 
>>> well?
>>>
>>> my thinking is we probably want all of these to be warnings because it'd be 
>>> hard to explain why `std` is reserved but with a warning while `_Test` is 
>>> reserved but with an error.
>>
>> Yeah, I'd treat them equally - while we could subset the reserved names and 
>> allow implementations to only use a subset (while leaving the rest as an 
>> error for both implementations and consumers alike) that doesn't feel in 
>> keeping with the purpose of these names - to be usable by /someone/ and so 
>> necessary to allow them to be used.
>>
>> (hmm - there's some discussion in the description about the fact that this 
>> error was already suppressed in "system headers" - why was that suppression 
>> inadequate for system implementation modules? (& does that suppression for 
>> reserved names risk being over-broad, since every third party library 
>> installed on a system is generally considered a "system header", even if 
>> they aren't part of the implementation?))
>
> We currently use line markers to "enter" a system header and that's quite 
> fragile. I mentioned we could use `#pragma clang system_header`, but 
> @ChuanqiXu  didn't think that was appropriate because these are not headers, 
> they're modules, and we should have some separation between "system headers" 
> and "system modules". 
> (https://github.com/llvm/llvm-project/issues/61446#issuecomment-1473029776) 
> As for being over-broad, it might be, but this is the approach we usually 
> take (anything that's a "system header" is considered special and gets less 
> diagnostics because the user isn't typically able to change the contents of 
> the header file anyway).

Presumably adding an alias for `#pragma clang system_header` called 
`system_module` wouldn't be too hard? (though the pragma is also being removed 
from libc++ soon, I think, in favor of `-isystem` usage, so maybe that's a sign 
the pragma's not a great way to do)

Presumably the line marker issue would be less significant for a module? Since 
there's no complex line marking, inclusion, etc, going on - it's just where the 
actual .cppm file is located/how it's found? (though yeah, that might get weird 
for building .pcms - since you're going to name the source file directly on the 
command line, it's not going to be found via any isystem lookup, etc... )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146986

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

tra wrote:
> sivachandra wrote:
> > jhuber6 wrote:
> > > tra wrote:
> > > > jhuber6 wrote:
> > > > > tra wrote:
> > > > > > Ensuring the right include order will be tricky. Interaction 
> > > > > > between the headers provided by llvm-libc vs the system headers 
> > > > > > will be interesting if we end up mixing the headers. It may be less 
> > > > > > problematic compared to the C++ standard library, but I doubt that 
> > > > > > mixing two implementations would work well here, either. 
> > > > > > 
> > > > > > So, the major question I have -- does adding include path here 
> > > > > > guarantees that we're not picking up the host headers? I do not see 
> > > > > > any changes that would exclude system headers from include paths.
> > > > > > If we do have include paths leading to both llvm-libc and the host 
> > > > > > headers, what's expected to happen if user code ends up including a 
> > > > > > header that's not present in  llvm-libc? Is that possible?
> > > > > > 
> > > > > Right now I'm just kind of relying on an expectation that since this 
> > > > > will be the first `c-isystem` path set, then it will pull in these 
> > > > > libraries first if they exist. It's not captured by the tests, but 
> > > > > compiling with `-v` shows this path being used first in my 
> > > > > experience. So, theoretically, if there is an implementation of said 
> > > > > header in this location, it will be picked up before anything else. 
> > > > > Otherwise it'll just search the other standard locations.
> > > > I think this will be a problem. We're cross-compiling here and for that 
> > > > to work reliably we need to make sure that only target headers are in 
> > > > effect. The problem is that we likely just do not have sufficiently 
> > > > complete set of headers for the GPU. Do we? I have no idea what exactly 
> > > > llvm-libc provides and whether it is sufficient for normal user code to 
> > > > cross-compile for a GPU. 
> > > > 
> > > > It would be interesting to try to compile some C++ code which would 
> > > > include commonly used, but generally target-agnostic, headers like 
> > > >   , etc and check whether we end up pulling 
> > > > in any system headers. Then check what happens if we do not have system 
> > > > headers available at all.
> > > No, it's definitely not complete. Some headers work on the GPU, most 
> > > break in some way or another. The only ones `llvm-libc` provides 
> > > currently is `string.h` and `ctype.h`. But, I figured this wouldn't be a 
> > > problem since it would just go to the system headers anyway if we didn't 
> > > provide them. So we are merely replacing maybe broken with probably works.
> > > 
> > > I was talking with Johannes and he brings up other issues about the idea 
> > > of host-device compatibility between these headers. Since, fundamentally, 
> > > right now `libc` generates its own headers and needs to generate its own 
> > > headers to function. But there can be a problem when migrating data 
> > > between the host and the device is the headers on the host differ 
> > > somewhat to those on the device. I'm not sure what a good overall 
> > > solution to that problem is.
> > Normally, one should not expect target and host headers to be compatible. 
> > So, if you are building for the host, you should use host headers and if 
> > you are building for the target, you should use target headers. Does 
> > general GPU build not follow this approach? May be there are some common 
> > headers but I do not expect them to be from the standard libraries.
> We can generally assume that the GPU and the host do have largely identical 
> types. At least the subset of the types we expect to exchange between host 
> and GPU.
> CUDA compilation cheats, and allows the host to provide most of the headers, 
> with clang and CUDA SDK providing a subset of GPU-side overloads. This way, 
> if GPU-side functions do implicitly rely on the code nominally provided for 
> the host by the host headers, but if we need to code-gen it, we can only do 
> so for a subset that's actually compileable for the GPU -- either constexpr 
> functions, lambdas or `__device__` overloads provided by us.
> 
> Standalone compilation does not have the benefit of having the cake and being 
> able to eat it. It has to be all or nothing, as we do not have the ability to 
> separate the host and GPU code we pull in via headers, nor can we provide a 
> GPU-side overloads. In a way injecting llvm-libc path is a crude attempt to 
> do that by providing GPU-side implementations before we get to include 
> host-side ehaders that would provide the host versions. While it may 
> sometimes work, I do not think it's a robust solution.
> 
> The only sound(-ish) 

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Augusto Noronha via Phabricator via cfe-commits]

augusto2112 added a comment.

In D146595#4225170 , @aaron.ballman 
wrote:

> In D146595#4225132 , @aprantl wrote:
>
>> In D146595#4225048 , @dblaikie 
>> wrote:
>>
>>> I know this is a bit of a redirection/scope creep/etc - but I'd quite like 
>>> to see a solution that is likely to be usable for the "std::function" 
>>> problem (stepping into std::function should allow you to reach the 
>>> underlying function - but lldb currently skips any call to a std-namespaced 
>>> function, I think, so you step right over the whole op() call) that could 
>>> also cover the Swift needs. Though perhaps they're just sufficiently 
>>> different problems that there is no generalizing here.
>>
>> This patch aims at exposing an existing LLVM IR & DWARF feature in clang. 
>> Having a generic solution for the std::function problem is definitely 
>> worthwhile investigating, but I'm not sure it needs to prevent supporting 
>> the existing mechanism in clang.
>
> Why should this feature be limited to DWARF? Don't we have the same kind of 
> stepping behavior issue with PDB files, for example?

There's already support to threading trampoline names from IR to DWARF. If PDB 
supports the same attribute in some form there's nothing stopping someone to 
adding support to thread the attribute from IR to PDB as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Can we default to freestanding on, or just document that freestanding is a good 
idea, instead of hacking with include behaviour directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D145545: [clang][Interp] Fix local variable (destructor) management in loop bodies

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:338
+this->emitDestructors();
+this->Ctx->emitDestroy(*Idx, SourceInfo{});
+  }

Should we be setting `Idx = std::nullopt;` after this so that the `LocalScope` 
destructor does not also emit a destroy for the same `Idx`?



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:199-200
+bool ByteCodeStmtGen::visitUnscopedCompoundStmt(const Stmt *S) {
+  if (isa(S))
+return true;
+

Errr, I'm surprised it isn't UB to call this with anything but a `CompoundStmt` 
given the function name.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:209
+
+  return this->visitStmt(S);
+}

It's a bit of a surprise that we visit the entire body of the compound 
statement before we visit the compound statement itself. I was thinking the 
compound statement could potentially have prologue work it needs to do, but now 
I'm second-guessing that. But this design still feels a little bit off... it's 
basically doing a post-order traversal just for this one node, and I wonder if 
we want something more general like a `preVisitFoo()` followed by `visitFoo()` 
followed by `postVisitFoo()` that applies to any AST node.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:324-328
+  LocalScope Scope(this);
+  if (!this->visitUnscopedCompoundStmt(Body))
 return false;
+
+  Scope.emitDestructors();

`AutoScope` and some curly braces to delimit the scope object lifetime?



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:345
   LoopScope LS(this, EndLabel, CondLabel);
+  LocalScope Scope(this);
 

Similar question here and elsewhere. The concern I have with this form is that 
it's pretty easy to accidentally miss that we've emitted the destructors in 
later code, whereas using the RAII object makes that impossible.


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

https://reviews.llvm.org/D145545

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

[PATCH] D146908: [clang][MinGW] Add asan link flags before other libs and objects

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun updated this revision to Diff 508751.
alvinhochun added a comment.

Trying to fix the test...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146908

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-sanitizers.c


Index: clang/test/Driver/mingw-sanitizers.c
===
--- clang/test/Driver/mingw-sanitizers.c
+++ clang/test/Driver/mingw-sanitizers.c
@@ -1,13 +1,18 @@
-// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-I686 %s
-// ASAN-I686: "{{.*}}libclang_rt.asan_dynamic-i386.dll.a"
-// ASAN-I686: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
-// ASAN-I686: "--require-defined" "___asan_seh_interceptor"
-// ASAN-I686: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
-
-// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 2>&1 | 
FileCheck --check-prefix=ASAN-X86_64 %s
-// ASAN-X86_64: "{{.*}}libclang_rt.asan_dynamic-x86_64.dll.a"
+// RUN: echo -n > %t.a
+// RUN: %clang -target i686-windows-gnu %s -### -fsanitize=address -lcomponent 
%/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-I686 -DINPUT=%/t.a %s
+// RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=address 
-lcomponent %/t.a 2>&1 | FileCheck --check-prefixes=ASAN-ALL,ASAN-X86_64 
-DINPUT=%/t.a %s
+//
+// ASAN-ALL-NOT:"-l{{[^"]+"]}}"
+// ASAN-ALL-NOT:"[[INPUT]]"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic-i386.dll.a"
+// ASAN-I686:   "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a"
+// ASAN-I686:   "--require-defined" "___asan_seh_interceptor"
+// ASAN-I686:   "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-i386.a" "--no-whole-archive"
+// ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic-x86_64.dll.a"
 // ASAN-X86_64: "{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a"
 // ASAN-X86_64: "--require-defined" "__asan_seh_interceptor"
-// ASAN-X86_64: "--whole-archive" 
"{{.*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-X86_64: "--whole-archive" 
"{{[^"]*}}libclang_rt.asan_dynamic_runtime_thunk-x86_64.a" "--no-whole-archive"
+// ASAN-ALL:"-lcomponent"
+// ASAN-ALL:"[[INPUT]]"
 
 // RUN: %clang -target x86_64-windows-gnu %s -### -fsanitize=vptr
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -200,6 +200,29 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
   Args.AddLastArg(CmdArgs, options::OPT_Z_Flag);
 
+  // Add asan flags before other libs and objects. Making asan_dynamic the 
first
+  // import lib allows asan to be initialized as early as possible to increase
+  // its instrumentation coverage to include other user DLLs which has not been
+  // built with asan.
+  if (Sanitize.needsAsanRt() && !Args.hasArg(options::OPT_nostdlib) &&
+  !Args.hasArg(options::OPT_nodefaultlibs)) {
+// MinGW always links against a shared MSVCRT.
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic", ToolChain::FT_Shared));
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--require-defined");
+CmdArgs.push_back(TC.getArch() == llvm::Triple::x86
+  ? "___asan_seh_interceptor"
+  : "__asan_seh_interceptor");
+// Make sure the linker consider all object files from the dynamic
+// runtime thunk.
+CmdArgs.push_back("--whole-archive");
+CmdArgs.push_back(
+TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
+CmdArgs.push_back("--no-whole-archive");
+  }
+
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_mdll)) {
   CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("dllcrt2.o")));
@@ -292,24 +315,6 @@
   if (Args.hasArg(options::OPT_pthread))
 CmdArgs.push_back("-lpthread");
 
-  if (Sanitize.needsAsanRt()) {
-// MinGW always links against a shared MSVCRT.
-CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dynamic",
-ToolChain::FT_Shared));
-CmdArgs.push_back(
-TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
-CmdArgs.push_back("--require-defined");
-CmdArgs.push_back(TC.getArch() == llvm::Triple::x86
-  ? "___asan_seh_interceptor"
-  : "__asan_seh_interceptor");
-// Make sure the linker consider all object files from the dynamic
-// runtime thunk.
-CmdArgs.push_back("--whole-archive");
-CmdArgs.push_back(
-  

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp added inline comments.



Comment at: clang/test/SemaCXX/dllexport.cpp:437
 
+class Base {};
+class __declspec(dllexport) ExportedClass {};

hans wrote:
> Is this one used somewhere?
It's not. Thanks for finding it.



Comment at: clang/test/SemaCXX/dllexport.cpp:439
+class __declspec(dllexport) ExportedClass {};
+class __declspec(dllimport) ImportedClass {};
+

hans wrote:
> Just to double check: this case (non-template base class) worked before this 
> patch too, right?
Right. I just added the case for good measure. Our case is about allowing a 
dll{ex,im}ported class that is called into existence, so to speak (i.e. 
instantiated), with template arguments that cause it to have non-external 
linkage. There is no question about the linkage of the non-template base 
classes. 


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

https://reviews.llvm.org/D146338

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

sivachandra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > jhuber6 wrote:
> > > > tra wrote:
> > > > > Ensuring the right include order will be tricky. Interaction between 
> > > > > the headers provided by llvm-libc vs the system headers will be 
> > > > > interesting if we end up mixing the headers. It may be less 
> > > > > problematic compared to the C++ standard library, but I doubt that 
> > > > > mixing two implementations would work well here, either. 
> > > > > 
> > > > > So, the major question I have -- does adding include path here 
> > > > > guarantees that we're not picking up the host headers? I do not see 
> > > > > any changes that would exclude system headers from include paths.
> > > > > If we do have include paths leading to both llvm-libc and the host 
> > > > > headers, what's expected to happen if user code ends up including a 
> > > > > header that's not present in  llvm-libc? Is that possible?
> > > > > 
> > > > Right now I'm just kind of relying on an expectation that since this 
> > > > will be the first `c-isystem` path set, then it will pull in these 
> > > > libraries first if they exist. It's not captured by the tests, but 
> > > > compiling with `-v` shows this path being used first in my experience. 
> > > > So, theoretically, if there is an implementation of said header in this 
> > > > location, it will be picked up before anything else. Otherwise it'll 
> > > > just search the other standard locations.
> > > I think this will be a problem. We're cross-compiling here and for that 
> > > to work reliably we need to make sure that only target headers are in 
> > > effect. The problem is that we likely just do not have sufficiently 
> > > complete set of headers for the GPU. Do we? I have no idea what exactly 
> > > llvm-libc provides and whether it is sufficient for normal user code to 
> > > cross-compile for a GPU. 
> > > 
> > > It would be interesting to try to compile some C++ code which would 
> > > include commonly used, but generally target-agnostic, headers like 
> > >   , etc and check whether we end up pulling 
> > > in any system headers. Then check what happens if we do not have system 
> > > headers available at all.
> > No, it's definitely not complete. Some headers work on the GPU, most break 
> > in some way or another. The only ones `llvm-libc` provides currently is 
> > `string.h` and `ctype.h`. But, I figured this wouldn't be a problem since 
> > it would just go to the system headers anyway if we didn't provide them. So 
> > we are merely replacing maybe broken with probably works.
> > 
> > I was talking with Johannes and he brings up other issues about the idea of 
> > host-device compatibility between these headers. Since, fundamentally, 
> > right now `libc` generates its own headers and needs to generate its own 
> > headers to function. But there can be a problem when migrating data between 
> > the host and the device is the headers on the host differ somewhat to those 
> > on the device. I'm not sure what a good overall solution to that problem is.
> Normally, one should not expect target and host headers to be compatible. So, 
> if you are building for the host, you should use host headers and if you are 
> building for the target, you should use target headers. Does general GPU 
> build not follow this approach? May be there are some common headers but I do 
> not expect them to be from the standard libraries.
We can generally assume that the GPU and the host do have largely identical 
types. At least the subset of the types we expect to exchange between host and 
GPU.
CUDA compilation cheats, and allows the host to provide most of the headers, 
with clang and CUDA SDK providing a subset of GPU-side overloads. This way, if 
GPU-side functions do implicitly rely on the code nominally provided for the 
host by the host headers, but if we need to code-gen it, we can only do so for 
a subset that's actually compileable for the GPU -- either constexpr functions, 
lambdas or `__device__` overloads provided by us.

Standalone compilation does not have the benefit of having the cake and being 
able to eat it. It has to be all or nothing, as we do not have the ability to 
separate the host and GPU code we pull in via headers, nor can we provide a 
GPU-side overloads. In a way injecting llvm-libc path is a crude attempt to do 
that by providing GPU-side implementations before we get to include host-side 
ehaders that would provide the host versions. While it may sometimes work, I do 
not think it's a robust solution.

The only sound(-ish) ways out I see are:
- implement sufficiently complete set of headers for the GPU.
- provide GPU-side overloads which would allow us to pull in system headers and 
augment t

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

[Wolfgang Pieb via Phabricator via cfe-commits]

wolfgangp updated this revision to Diff 508747.
wolfgangp marked an inline comment as done.
wolfgangp added a comment.

Added 2 test cases that check that dll{ex,im}ported classes that are 
instantiated with classes with internal linkage as template arguments are not 
exported or imported.

Had to place them into separate files, since they are negative tests and did 
not fit well into dllexport.cpp and dllimport.cpp


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

https://reviews.llvm.org/D146338

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-unique-external.cpp
  clang/test/CodeGenCXX/dllimport-unique-external.cpp
  clang/test/SemaCXX/dllexport.cpp
  clang/test/SemaCXX/dllimport.cpp

Index: clang/test/SemaCXX/dllimport.cpp
===
--- clang/test/SemaCXX/dllimport.cpp
+++ clang/test/SemaCXX/dllimport.cpp
@@ -5,9 +5,9 @@
 // RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify -std=c++17 -Wunsupported-dll-base-class-template -DGNU %s
 // RUN: %clang_cc1 -triple i686-windows-itanium   -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DWI %s
 // RUN: %clang_cc1 -triple x86_64-windows-itanium -fsyntax-only -fms-extensions -verify -std=c++17 -Wunsupported-dll-base-class-template -DWI %s
-// RUN: %clang_cc1 -triple x86_64-scei-ps4-fsyntax-only -fdeclspec  -verify -std=c++11 -Wunsupported-dll-base-class-template -DWI -DPS %s
-// RUN: %clang_cc1 -triple x86_64-scei-ps4-fsyntax-only -fdeclspec  -verify -std=c++17 -Wunsupported-dll-base-class-template -DWI -DPS %s
-// RUN: %clang_cc1 -triple x86_64-sie-ps5 -fsyntax-only -fdeclspec  -verify -std=c++17 -Wunsupported-dll-base-class-template -DWI -DPS %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps4-fsyntax-only -fdeclspec  -verify -std=c++11 -Wunsupported-dll-base-class-template -DPS %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps4-fsyntax-only -fdeclspec  -verify -std=c++17 -Wunsupported-dll-base-class-template -DPS %s
+// RUN: %clang_cc1 -triple x86_64-sie-ps5 -fsyntax-only -fdeclspec  -verify -std=c++17 -Wunsupported-dll-base-class-template -DPS %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Imported {};
@@ -60,7 +60,7 @@
 // expected-note@+2{{previous attribute is here}}
 #endif
 __declspec(dllimport) extern int ExternGlobalDeclInit; // expected-note{{previous declaration is here}}
-#if defined(MS) || defined(WI)
+#if defined(MS) || defined(WI) || defined(PS)
 // expected-warning@+4{{'ExternGlobalDeclInit' redeclared without 'dllimport' attribute: 'dllexport' attribute added}}
 #else
 // expected-warning@+2{{'ExternGlobalDeclInit' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
@@ -71,7 +71,7 @@
 // expected-note@+2{{previous attribute is here}}
 #endif
 __declspec(dllimport) int GlobalDeclInit; // expected-note{{previous declaration is here}}
-#if defined(MS) || defined(WI)
+#if defined(MS) || defined(WI) || defined(PS)
 // expected-warning@+4{{'GlobalDeclInit' redeclared without 'dllimport' attribute: 'dllexport' attribute added}}
 #else
 // expected-warning@+2{{'GlobalDeclInit' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
@@ -82,7 +82,7 @@
 // expected-note@+2{{previous attribute is here}}
 #endif
 int *__attribute__((dllimport)) GlobalDeclChunkAttrInit; // expected-note{{previous declaration is here}}
-#if defined(MS) || defined(WI)
+#if defined(MS) || defined(WI) || defined(PS)
 // expected-warning@+4{{'GlobalDeclChunkAttrInit' redeclared without 'dllimport' attribute: 'dllexport' attribute added}}
 #else
 // expected-warning@+2{{'GlobalDeclChunkAttrInit' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
@@ -93,7 +93,7 @@
 // expected-note@+2{{previous attribute is here}}
 #endif
 int GlobalDeclAttrInit __attribute__((dllimport)); // expected-note{{previous declaration is here}}
-#if defined(MS) || defined(WI)
+#if defined(MS) || defined(WI) || defined(PS)
 // expected-warning@+4{{'GlobalDeclAttrInit' redeclared without 'dllimport' attribute: 'dllexport' attribute added}}
 #else
 // expected-warning@+2{{'GlobalDeclAttrInit' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
@@ -184,7 +184,7 @@
 #endif
 template 
 __declspec(dllimport) extern int ExternVarTmplDeclInit; // expected-note{{previous declaration is here}}
-#if defined(MS) || defined(WI)
+#if defined(MS) || defined(WI) || defined(PS)
 // expected-warning@+5{{'ExternVarTmplDeclInit' redeclared without 'dllimport' attribute: 'dllexport' attribute added}}
 #else
 // expected-warning@+3{{'ExternVarTmplDeclInit' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
@@ -197,7 +197,7 @@
 #endif
 template 
 __declspec(dllimport) int VarTmplDeclInit; // expected-note{{previous declaration is here}}
-#if defined(MS) || de

[PATCH] D137379: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added a comment.

In D137379#4225000 , @manojgupta 
wrote:

> This is firing even in checked length codes, is that expected?

Yes, it is expected.  The unsafe buffer analysis is syntax-based.  The analysis 
warns operations that do not follow the buffer-safe programming model we are 
suggesting.  The programming model prohibits pointer arithmetic.  In the model, 
pointer arithmetic and buffer access can be done using hardened libc++ 
facilities such as `std::span`.

More information about the analysis and the programming model can be found at 
https://discourse.llvm.org/t/rfc-c-buffer-hardening/65734.

To suppress the warning, you can either turn the analysis off using 
`-Wno-unsafe-buffer-usage` or put code in a pair of opt-out pragmas `#pragma 
clang unsafe_buffer_usage begin` & `#pragma clang unsafe_buffer_usage end`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137379

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

[PATCH] D146986: Downgrade reserved module identifier error into a warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D146986#4225121 , @dblaikie wrote:

> From the discussion on the issue:
>
>> Do we want this loosening of the restriction to apply to *only* `std` and 
>> the same followed by a number, or to any reserved identifier used in a 
>> module? e.g.,
>>
>>   module std; // error today, but will become a warning
>>   module _Test; // error today, but do we want this to become a warning as 
>> well?
>>
>> my thinking is we probably want all of these to be warnings because it'd be 
>> hard to explain why `std` is reserved but with a warning while `_Test` is 
>> reserved but with an error.
>
> Yeah, I'd treat them equally - while we could subset the reserved names and 
> allow implementations to only use a subset (while leaving the rest as an 
> error for both implementations and consumers alike) that doesn't feel in 
> keeping with the purpose of these names - to be usable by /someone/ and so 
> necessary to allow them to be used.
>
> (hmm - there's some discussion in the description about the fact that this 
> error was already suppressed in "system headers" - why was that suppression 
> inadequate for system implementation modules? (& does that suppression for 
> reserved names risk being over-broad, since every third party library 
> installed on a system is generally considered a "system header", even if they 
> aren't part of the implementation?))

We currently use line markers to "enter" a system header and that's quite 
fragile. I mentioned we could use `#pragma clang system_header`, but @ChuanqiXu 
 didn't think that was appropriate because these are not headers, they're 
modules, and we should have some separation between "system headers" and 
"system modules". 
(https://github.com/llvm/llvm-project/issues/61446#issuecomment-1473029776) As 
for being over-broad, it might be, but this is the approach we usually take 
(anything that's a "system header" is considered special and gets less 
diagnostics because the user isn't typically able to change the contents of the 
header file anyway).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146986

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D146595#4225132 , @aprantl wrote:

> In D146595#4225048 , @dblaikie 
> wrote:
>
>> I know this is a bit of a redirection/scope creep/etc - but I'd quite like 
>> to see a solution that is likely to be usable for the "std::function" 
>> problem (stepping into std::function should allow you to reach the 
>> underlying function - but lldb currently skips any call to a std-namespaced 
>> function, I think, so you step right over the whole op() call) that could 
>> also cover the Swift needs. Though perhaps they're just sufficiently 
>> different problems that there is no generalizing here.
>
> This patch aims at exposing an existing LLVM IR & DWARF feature in clang.

Existing LLVM IR feature? *goes to check history* Ah, I see - hadn't realized 
at least at the IR level this was already implemented. (Anyone got experience 
with how this works in GCC/GDB then? I guess it was implemented there 
first/ported to clang with the fortran effort? So that might answer some of the 
questions about how we should implement it in LLVM and Clang, and about the 
precedent for behavior of the corner cases?)

> Having a generic solution for the std::function problem is definitely 
> worthwhile investigating, but I'm not sure it needs to prevent supporting the 
> existing mechanism in clang.
>
> I understand that this is nowhere near as flexible as a source-level 
> solution, but in case you weren't aware of this, LLDB implements a custom 
> step through plan for std::function:
>
> https://github.com/llvm/llvm-project/blob/fd059ea7ec044198fd75bb2b3aa30734bcace33e/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp#L368

Ah, might've been an overreach/misassumption on my part - I thought we came 
across this somewhere, but it might've been `std::make_unique` or some other 
standard library code that didn't have this special casing (can imagine various 
`emplace` functions, etc, where you might want to step into the ctor too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146840: [AMDGPU] Replace target feature for global fadd32

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec requested changes to this revision.
rampitec added a comment.
This revision now requires changes to proceed.

You cannot just enable it on gfx908 which does not have return version of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146840

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

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

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

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:4343
+   WhitespaceSensitiveMacros == R.WhitespaceSensitiveMacros &&
+   Macros == R.Macros;
   }

I put a lot of effort into bringing the stuff sorted. And now a change which 
was completely transparent to me, because of the missing clang-format tag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144170

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D146595#4225132 , @aprantl wrote:

> In D146595#4225048 , @dblaikie 
> wrote:
>
>> I know this is a bit of a redirection/scope creep/etc - but I'd quite like 
>> to see a solution that is likely to be usable for the "std::function" 
>> problem (stepping into std::function should allow you to reach the 
>> underlying function - but lldb currently skips any call to a std-namespaced 
>> function, I think, so you step right over the whole op() call) that could 
>> also cover the Swift needs. Though perhaps they're just sufficiently 
>> different problems that there is no generalizing here.
>
> This patch aims at exposing an existing LLVM IR & DWARF feature in clang. 
> Having a generic solution for the std::function problem is definitely 
> worthwhile investigating, but I'm not sure it needs to prevent supporting the 
> existing mechanism in clang.

Why should this feature be limited to DWARF? Don't we have the same kind of 
stepping behavior issue with PDB files, for example?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

[Michael Buch via Phabricator via cfe-commits]

Michael137 marked an inline comment as not done.
Michael137 added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

dblaikie wrote:
> Michael137 wrote:
> > probinson wrote:
> > > This doesn't become `Foo` ?
> > The name stays as `Foo>` but the type of the template parameter 
> > becomes `BarInt`. So the dwarf would look like:
> > ```
> > DW_TAG_structure_type
> >   DW_AT_name  ("Foo >")
> > 
> >   DW_TAG_template_type_parameter
> > DW_AT_type(0x0095 "BarInt")
> > DW_AT_name("T")
> > 
> > ```
> > Will add the parameter metadata to the test
> Hmm, that seems buggy - why doesn't `Foo >` become `Foo`? 
> (is the preferred name ever used in the DW_AT_name? I'd have thought it 
> should be unless it's explicitly avoided to ensure type 
> compatibility/collapsing with debug info built without this feature enabled?)
I suspect it's because the name is constructed using the clang TypePrinter. And 
we turn off `PrintingPolicy::UsePreferredNames` by default to avoid divergence 
from GCC.

Will double check though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145803

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

[PATCH] D144358: [clang][github] Added checking for completeness of lvalue in conditional operator [#59718]

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D144358#4225139 , @Aditya-pixel 
wrote:

> Oh, okay, Ill change the patch now. Also, it seems my local directory is out 
> of sync. Should I always update it before submitting a patch, or is it 
> alright without updating it?

It's usually good to rebase each time you update the patch, but it's not a 
requirement either (sometimes it's easier to rebase at the end instead of while 
the patch is under review). I've landed the changes for you in 
dedd7b6548f4a37f4f691aa0cd3a709756b7e794 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144358

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

[PATCH] D144358: [clang][github] Added checking for completeness of lvalue in conditional operator [#59718]

[Aaron Ballman 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 rGdedd7b6548f4: Added checking for completeness of lvalue in 
conditional operator (authored by Aditya-pixel, committed by aaron.ballman).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D144358?vs=508742&id=508743#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144358

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/incomplete-decl.c


Index: clang/test/Sema/incomplete-decl.c
===
--- clang/test/Sema/incomplete-decl.c
+++ clang/test/Sema/incomplete-decl.c
@@ -1,31 +1,51 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c,expected %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=cxx,expected %s
 
-struct foo; // expected-note 5 {{forward declaration of 'struct foo'}}
+
+
+struct foo; // c-note 5 {{forward declaration of 'struct foo'}} \
+   cxx-note 3 {{forward declaration of 'foo'}}
 
 void b;  // expected-error {{variable has incomplete type 'void'}}
-struct foo f; // expected-error{{tentative definition has type 'struct foo' 
that is never completed}}
+struct foo f; // c-error {{tentative definition has type 'struct foo' that is 
never completed}} \
+ cxx-error {{variable has incomplete type 'struct foo'}}
 
 static void c; // expected-error {{variable has incomplete type 'void'}}
-static struct foo g;  // expected-warning {{tentative definition of variable 
with internal linkage has incomplete non-array type 'struct foo'}} \
-expected-error{{tentative definition has type 'struct foo' that is never 
completed}}
+static struct foo g;  // c-warning {{tentative definition of variable with 
internal linkage has incomplete non-array type 'struct foo'}} \
+ c-error {{tentative definition has type 'struct foo' 
that is never completed}} \
+ cxx-error {{variable has incomplete type 'struct 
foo'}}
 
-extern void d;
+extern void d; // cxx-error {{variable has incomplete type 'void'}}
 extern struct foo e;
 
-int ary[]; // expected-warning {{tentative array definition assumed to have 
one element}}
-struct foo bary[]; // expected-error {{array has incomplete element type 
'struct foo'}}
+int ary[]; // c-warning {{tentative array definition assumed to have one 
element}} \
+  cxx-error {{definition of variable with array type needs an 
explicit size or an initializer}}
+struct foo bary[]; // c-error {{array has incomplete element type 'struct 
foo'}} \
+  cxx-error {{definition of variable with array type needs 
an explicit size or an initializer}}
 
 void func(void) {
-  int ary[]; // expected-error{{definition of variable with array type needs 
an explicit size or an initializer}}
+  int ary[]; // expected-error {{definition of variable with array type needs 
an explicit size or an initializer}}
   void b; // expected-error {{variable has incomplete type 'void'}}
   struct foo f; // expected-error {{variable has incomplete type 'struct foo'}}
 }
 
-int h[]; // expected-warning {{tentative array definition assumed to have one 
element}}
-int (*i)[] = &h+1; // expected-error {{arithmetic on a pointer to an 
incomplete type 'int[]'}}
+int h[]; // c-warning {{tentative array definition assumed to have one 
element}} \
+cxx-error {{definition of variable with array type needs an 
explicit size or an initializer}}
+int (*i)[] = &h+1; // c-error {{arithmetic on a pointer to an incomplete type 
'int[]'}}
 
 struct bar j = {1}; // expected-error {{variable has incomplete type 'struct 
bar'}} \
-expected-note {{forward declaration of 'struct bar'}}
-struct bar k;
+   c-note {{forward declaration of 'struct bar'}} \
+   cxx-note 2 {{forward declaration of 'bar'}}
+
+struct bar k; // cxx-error {{variable has incomplete type 'struct bar'}}
 struct bar { int a; };
 
+struct x y; //c-note 2 {{forward declaration of 'struct x'}} \
+  cxx-error {{variable has incomplete type 'struct x'}} \
+  cxx-note {{forward declaration of 'x'}}
+void foo() {
+  (void)(1 ? y : y); // c-error 2 {{incomplete type 'struct x' where a 
complete type is required}}
+}
+struct x{
+  int a;
+};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14396,6 +14396,9 @@
 static void CheckConditionalOperand(Sema &S, Expr *E, QualType T,
 SourceLocation CC, bool &ICContext) {
   E = E->IgnoreParenImpCasts();
+  // Diagnose incomplete type for second or third operand in C.
+  if (!S.getL

[clang] dedd7b6 - Added checking for completeness of lvalue in conditional operator

[Aaron Ballman via cfe-commits]

Author: Aditya Singh
Date: 2023-03-27T14:47:08-04:00
New Revision: dedd7b6548f4a37f4f691aa0cd3a709756b7e794

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

LOG: Added checking for completeness of lvalue in conditional operator

Given:
```
struct x y;
int main(void)
{
(void)(1 ? y : y);
}
struct x {int i;};
```
The conditional operator(?:) requires the second and third operands to
be of compatible types. To be compatible, they also need to be
complete (however, both can be void). Therefore, the expected response
from clang after running the above code as a C program should be error
dialogue pointing out that both the types are incomplete hence
incompatible, but the code compiled without any errors.

The patch ensures the completeness in the CheckCondtionalOperand
function present in llvm-project/clang/lib/Sema/SemaChecking.cpp.

Fixes https://github.com/llvm/llvm-project/issues/59718
Differential Revision: https://reviews.llvm.org/D144358

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/incomplete-decl.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b2efa99313255..295532a9bfeca 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -186,6 +186,9 @@ Improvements to Clang's diagnostics
   by prioritizing ``-Wunreachable-code-fallthrough``.
 - Clang now correctly diagnoses statement attributes 
``[[clang::always_inine]]`` and
   ``[[clang::noinline]]`` when used on a statement with dependent call 
expressions.
+- Clang now checks for completeness of the second and third arguments in the
+  conditional operator.
+  (`#59718 `_)
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index a3f2452b53d0c..c8b42519c88dc 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14396,6 +14396,9 @@ static void CheckConditionalOperator(Sema &S, 
AbstractConditionalOperator *E,
 static void CheckConditionalOperand(Sema &S, Expr *E, QualType T,
 SourceLocation CC, bool &ICContext) {
   E = E->IgnoreParenImpCasts();
+  // Diagnose incomplete type for second or third operand in C.
+  if (!S.getLangOpts().CPlusPlus && E->getType()->isRecordType())
+S.RequireCompleteExprType(E, diag::err_incomplete_type);
 
   if (auto *CO = dyn_cast(E))
 return CheckConditionalOperator(S, CO, CC, T);

diff  --git a/clang/test/Sema/incomplete-decl.c 
b/clang/test/Sema/incomplete-decl.c
index 954d4ab0c672f..bf2890bba9911 100644
--- a/clang/test/Sema/incomplete-decl.c
+++ b/clang/test/Sema/incomplete-decl.c
@@ -1,31 +1,51 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c,expected %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify=cxx,expected %s
 
-struct foo; // expected-note 5 {{forward declaration of 'struct foo'}}
+
+
+struct foo; // c-note 5 {{forward declaration of 'struct foo'}} \
+   cxx-note 3 {{forward declaration of 'foo'}}
 
 void b;  // expected-error {{variable has incomplete type 'void'}}
-struct foo f; // expected-error{{tentative definition has type 'struct foo' 
that is never completed}}
+struct foo f; // c-error {{tentative definition has type 'struct foo' that is 
never completed}} \
+ cxx-error {{variable has incomplete type 'struct foo'}}
 
 static void c; // expected-error {{variable has incomplete type 'void'}}
-static struct foo g;  // expected-warning {{tentative definition of variable 
with internal linkage has incomplete non-array type 'struct foo'}} \
-expected-error{{tentative definition has type 'struct foo' that is never 
completed}}
+static struct foo g;  // c-warning {{tentative definition of variable with 
internal linkage has incomplete non-array type 'struct foo'}} \
+ c-error {{tentative definition has type 'struct foo' 
that is never completed}} \
+ cxx-error {{variable has incomplete type 'struct 
foo'}}
 
-extern void d;
+extern void d; // cxx-error {{variable has incomplete type 'void'}}
 extern struct foo e;
 
-int ary[]; // expected-warning {{tentative array definition assumed to have 
one element}}
-struct foo bary[]; // expected-error {{array has incomplete element type 
'struct foo'}}
+int ary[]; // c-warning {{tentative array definition assumed to have one 
element}} \
+  cxx-error {{definition of variable with array type needs an 
explicit size or an initializer}}
+struct foo bary[]; // c-error {{array has incomplete element type 'struct 
foo'}} \
+  cxx-error {{definition of varia

[PATCH] D146924: [clang] Add support for dollar sign in ud_suffix of numeric constants

[Anna Arad via Phabricator via cfe-commits]

annara added a comment.

Thanks for all the input!

I'll work on @cor3ntin points and take a look at the crash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146924

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D146595#4225048 , @dblaikie wrote:

> I know this is a bit of a redirection/scope creep/etc - but I'd quite like to 
> see a solution that is likely to be usable for the "std::function" problem 
> (stepping into std::function should allow you to reach the underlying 
> function - but lldb currently skips any call to a std-namespaced function, I 
> think, so you step right over the whole op() call) that could also cover the 
> Swift needs. Though perhaps they're just sufficiently different problems that 
> there is no generalizing here.

This patch aims at exposing an existing LLVM IR & DWARF feature in clang. 
Having a generic solution for the std::function problem is definitely 
worthwhile investigating, but I'm not sure it needs to prevent supporting the 
existing mechanism in clang.

I understand that this is nowhere near as flexible as a source-level solution, 
but in case you weren't aware of this, LLDB implements a custom step through 
plan for std::function:

https://github.com/llvm/llvm-project/blob/fd059ea7ec044198fd75bb2b3aa30734bcace33e/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp#L368


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

jhuber6 wrote:
> tra wrote:
> > jhuber6 wrote:
> > > tra wrote:
> > > > Ensuring the right include order will be tricky. Interaction between 
> > > > the headers provided by llvm-libc vs the system headers will be 
> > > > interesting if we end up mixing the headers. It may be less problematic 
> > > > compared to the C++ standard library, but I doubt that mixing two 
> > > > implementations would work well here, either. 
> > > > 
> > > > So, the major question I have -- does adding include path here 
> > > > guarantees that we're not picking up the host headers? I do not see any 
> > > > changes that would exclude system headers from include paths.
> > > > If we do have include paths leading to both llvm-libc and the host 
> > > > headers, what's expected to happen if user code ends up including a 
> > > > header that's not present in  llvm-libc? Is that possible?
> > > > 
> > > Right now I'm just kind of relying on an expectation that since this will 
> > > be the first `c-isystem` path set, then it will pull in these libraries 
> > > first if they exist. It's not captured by the tests, but compiling with 
> > > `-v` shows this path being used first in my experience. So, 
> > > theoretically, if there is an implementation of said header in this 
> > > location, it will be picked up before anything else. Otherwise it'll just 
> > > search the other standard locations.
> > I think this will be a problem. We're cross-compiling here and for that to 
> > work reliably we need to make sure that only target headers are in effect. 
> > The problem is that we likely just do not have sufficiently complete set of 
> > headers for the GPU. Do we? I have no idea what exactly llvm-libc provides 
> > and whether it is sufficient for normal user code to cross-compile for a 
> > GPU. 
> > 
> > It would be interesting to try to compile some C++ code which would include 
> > commonly used, but generally target-agnostic, headers like  
> >  , etc and check whether we end up pulling in any 
> > system headers. Then check what happens if we do not have system headers 
> > available at all.
> No, it's definitely not complete. Some headers work on the GPU, most break in 
> some way or another. The only ones `llvm-libc` provides currently is 
> `string.h` and `ctype.h`. But, I figured this wouldn't be a problem since it 
> would just go to the system headers anyway if we didn't provide them. So we 
> are merely replacing maybe broken with probably works.
> 
> I was talking with Johannes and he brings up other issues about the idea of 
> host-device compatibility between these headers. Since, fundamentally, right 
> now `libc` generates its own headers and needs to generate its own headers to 
> function. But there can be a problem when migrating data between the host and 
> the device is the headers on the host differ somewhat to those on the device. 
> I'm not sure what a good overall solution to that problem is.
Normally, one should not expect target and host headers to be compatible. So, 
if you are building for the host, you should use host headers and if you are 
building for the target, you should use target headers. Does general GPU build 
not follow this approach? May be there are some common headers but I do not 
expect them to be from the standard libraries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D145803: [clang][DebugInfo] Emit DW_AT_type of preferred name if available

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/preferred_name.cpp:49
+
+Foo> varFooInt;
+

Michael137 wrote:
> probinson wrote:
> > This doesn't become `Foo` ?
> The name stays as `Foo>` but the type of the template parameter 
> becomes `BarInt`. So the dwarf would look like:
> ```
> DW_TAG_structure_type
>   DW_AT_name  ("Foo >")
> 
>   DW_TAG_template_type_parameter
> DW_AT_type(0x0095 "BarInt")
> DW_AT_name("T")
> 
> ```
> Will add the parameter metadata to the test
Hmm, that seems buggy - why doesn't `Foo >` become `Foo`? (is 
the preferred name ever used in the DW_AT_name? I'd have thought it should be 
unless it's explicitly avoided to ensure type compatibility/collapsing with 
debug info built without this feature enabled?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145803

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

[PATCH] D146986: Downgrade reserved module identifier error into a warning

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

From the discussion on the issue:

> Do we want this loosening of the restriction to apply to *only* `std` and the 
> same followed by a number, or to any reserved identifier used in a module? 
> e.g.,
>
>   module std; // error today, but will become a warning
>   module _Test; // error today, but do we want this to become a warning as 
> well?
>
> my thinking is we probably want all of these to be warnings because it'd be 
> hard to explain why `std` is reserved but with a warning while `_Test` is 
> reserved but with an error.

Yeah, I'd treat them equally - while we could subset the reserved names and 
allow implementations to only use a subset (while leaving the rest as an error 
for both implementations and consumers alike) that doesn't feel in keeping with 
the purpose of these names - to be usable by /someone/ and so necessary to 
allow them to be used.

(hmm - there's some discussion in the description about the fact that this 
error was already suppressed in "system headers" - why was that suppression 
inadequate for system implementation modules? (& does that suppression for 
reserved names risk being over-broad, since every third party library installed 
on a system is generally considered a "system header", even if they aren't part 
of the implementation?))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146986

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > Ensuring the right include order will be tricky. Interaction between the 
> > > headers provided by llvm-libc vs the system headers will be interesting 
> > > if we end up mixing the headers. It may be less problematic compared to 
> > > the C++ standard library, but I doubt that mixing two implementations 
> > > would work well here, either. 
> > > 
> > > So, the major question I have -- does adding include path here guarantees 
> > > that we're not picking up the host headers? I do not see any changes that 
> > > would exclude system headers from include paths.
> > > If we do have include paths leading to both llvm-libc and the host 
> > > headers, what's expected to happen if user code ends up including a 
> > > header that's not present in  llvm-libc? Is that possible?
> > > 
> > Right now I'm just kind of relying on an expectation that since this will 
> > be the first `c-isystem` path set, then it will pull in these libraries 
> > first if they exist. It's not captured by the tests, but compiling with 
> > `-v` shows this path being used first in my experience. So, theoretically, 
> > if there is an implementation of said header in this location, it will be 
> > picked up before anything else. Otherwise it'll just search the other 
> > standard locations.
> I think this will be a problem. We're cross-compiling here and for that to 
> work reliably we need to make sure that only target headers are in effect. 
> The problem is that we likely just do not have sufficiently complete set of 
> headers for the GPU. Do we? I have no idea what exactly llvm-libc provides 
> and whether it is sufficient for normal user code to cross-compile for a GPU. 
> 
> It would be interesting to try to compile some C++ code which would include 
> commonly used, but generally target-agnostic, headers like   
> , etc and check whether we end up pulling in any system headers. 
> Then check what happens if we do not have system headers available at all.
No, it's definitely not complete. Some headers work on the GPU, most break in 
some way or another. The only ones `llvm-libc` provides currently is `string.h` 
and `ctype.h`. But, I figured this wouldn't be a problem since it would just go 
to the system headers anyway if we didn't provide them. So we are merely 
replacing maybe broken with probably works.

I was talking with Johannes and he brings up other issues about the idea of 
host-device compatibility between these headers. Since, fundamentally, right 
now `libc` generates its own headers and needs to generate its own headers to 
function. But there can be a problem when migrating data between the host and 
the device is the headers on the host differ somewhat to those on the device. 
I'm not sure what a good overall solution to that problem is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 marked an inline comment as done.
jp4a50 added a comment.

All actionable comments have been addressed.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidth
+  : Style.DesignatedInitializerIndentWidth;
+  NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;

owenpan wrote:
> owenpan wrote:
> > Using -1 to mean `ContinuationIndentWidth` is unnecessary and somewhat 
> > confusing IMO.
> Please disregard my comment above.
Just to make sure we are on the same page, does this mean that you are happy 
with the approach of using `-1` as a default value to indicate that 
`ContinuationIndentWidth` should be used?

I did initially consider using `std::optional` and using empty 
optional to indicate that `ContinuationIndentWidth` should be used but I saw 
that `PPIndentWidth` was using `-1` to default to using `IndentWidth` so I 
followed that precedent.



Comment at: clang/unittests/Format/FormatTest.cpp:4828
+   ".yy = 2,\n"
+   ".zz = 3};\n",
+   Style);

MyDeveloperDay wrote:
> jp4a50 wrote:
> > MyDeveloperDay wrote:
> > > Can we fix the brace positioning too
> > What would you expect the brace positioning to be here? I noticed that, 
> > with this configuration at least, the closing brace goes at the end of the 
> > same line (as the last initializer) when there is no trailing comma but 
> > goes on a new line if there is a trailing comma.
> part of me would like an option to not have to supply the trailing comma, 
I agree that it's slightly odd behaviour but I think it's pretty orthogonal to 
this change and would deserve it's own diff.

I think it's also pretty debatable what the behaviour *should* be. Should the 
`AlignAfterOpenBracket` option dictate the formatting of braces like this? 
Oddly enough, specifying `AlwaysBreak` (as I have done in this test) does 
indeed result in breaking after the `{` but changing it to `BlockIndent` 
doesn't put the closing `}` on a newline. Should we instead add a new 
`AlignAfterOpenBrace` analogue of `AlignAfterOpenBracket`?

Would you like me to to raise a separate issue to track this (if it doesn't 
already exist)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: echristo.
tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

jhuber6 wrote:
> tra wrote:
> > Ensuring the right include order will be tricky. Interaction between the 
> > headers provided by llvm-libc vs the system headers will be interesting if 
> > we end up mixing the headers. It may be less problematic compared to the 
> > C++ standard library, but I doubt that mixing two implementations would 
> > work well here, either. 
> > 
> > So, the major question I have -- does adding include path here guarantees 
> > that we're not picking up the host headers? I do not see any changes that 
> > would exclude system headers from include paths.
> > If we do have include paths leading to both llvm-libc and the host headers, 
> > what's expected to happen if user code ends up including a header that's 
> > not present in  llvm-libc? Is that possible?
> > 
> Right now I'm just kind of relying on an expectation that since this will be 
> the first `c-isystem` path set, then it will pull in these libraries first if 
> they exist. It's not captured by the tests, but compiling with `-v` shows 
> this path being used first in my experience. So, theoretically, if there is 
> an implementation of said header in this location, it will be picked up 
> before anything else. Otherwise it'll just search the other standard 
> locations.
I think this will be a problem. We're cross-compiling here and for that to work 
reliably we need to make sure that only target headers are in effect. The 
problem is that we likely just do not have sufficiently complete set of headers 
for the GPU. Do we? I have no idea what exactly llvm-libc provides and whether 
it is sufficient for normal user code to cross-compile for a GPU. 

It would be interesting to try to compile some C++ code which would include 
commonly used, but generally target-agnostic, headers like   
, etc and check whether we end up pulling in any system headers. 
Then check what happens if we do not have system headers available at all.



Comment at: clang/test/Driver/gpu-libc-headers.c:14
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// CHECK-HEADERS: "-cc1"{{.*}}"-c-isystem" "{{.*}}include/llvm-libc"
+

jhuber6 wrote:
> tra wrote:
> > I think here we want to test for not just the presence of 
> > `include/llvm-libc`, but also that it's in the correct position relative to 
> > other include paths. 
> > 
> > We may want something similar to what we have in 
> > clang/test/Driver/hip-include-path.hip
> Yeah, I wasn't sure if there was a good way to guarantee a certain path since 
> those can change based on the system. Maybe `--sysroot`?
I do not have a good answer. 

@echristo -- when we need to cross-compile for some target, who/where/how tells 
clang where to get target-specific headers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 508740.
jp4a50 added a comment.

Address minor review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4820,6 +4820,34 @@
"[3] = cc,\n"
"[4] = dd,\n"
"[5] = ee};");
+
+  auto Style = getLLVMStyleWithColumns(60);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  verifyFormat("auto s = SomeStruct{\n"
+   ".xx = 1,\n"
+   ".yy = 2,\n"
+   ".zz = 3};\n",
+   Style);
+  Style.DesignatedInitializerIndentWidth = 2;
+  verifyFormat("auto s = SomeStruct{\n"
+   "  .xx = 1,\n"
+   "  .yy = 2,\n"
+   "  .zz = 3};\n",
+   Style);
+  verifyFormat("auto s = someFunctionCall(\n"
+   ", ,\n"
+   "SomeStruct{\n"
+   "  .xx = 1,\n"
+   "  .yy = 2,\n"
+   "  .zz = 3});\n",
+   Style);
+  Style.ContinuationIndentWidth = 8;
+  Style.DesignatedInitializerIndentWidth = -1; // Use ContinuationIndentWidth.
+  verifyFormat("auto s = SomeStruct{\n"
+   ".xx = 1,\n"
+   ".yy = 2,\n"
+   ".zz = 3};\n",
+   Style);
 }
 
 TEST_F(FormatTest, NestedStaticInitializers) {
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -245,6 +245,10 @@
   SpacesBeforeTrailingComments, 1234u);
   CHECK_PARSE("IndentWidth: 32", IndentWidth, 32u);
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
+  CHECK_PARSE("DesignatedInitializerIndentWidth: 34",
+  DesignatedInitializerIndentWidth, 34);
+  CHECK_PARSE("DesignatedInitializerIndentWidth: -1",
+  DesignatedInitializerIndentWidth, -1);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
   Style.QualifierAlignment = FormatStyle::QAS_Right;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -902,6 +902,8 @@
 IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
 IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
 IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
+IO.mapOptional("DesignatedInitializerIndentWidth",
+   Style.DesignatedInitializerIndentWidth);
 IO.mapOptional("DisableFormat", Style.DisableFormat);
 IO.mapOptional("EmptyLineAfterAccessModifier",
Style.EmptyLineAfterAccessModifier);
@@ -1367,6 +1369,7 @@
   LLVMStyle.ContinuationIndentWidth = 4;
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;
+  LLVMStyle.DesignatedInitializerIndentWidth = -1;
   LLVMStyle.DisableFormat = false;
   LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
   LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1656,13 +1656,20 @@
 CurrentState.NestedBlockIndent);
   if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) ||
   opensProtoMessageField(Current, Style)) {
+const FormatToken *NextNoComment = Current.getNextNonComment();
 if (Current.opensBlockOrBlockTypeList(Style)) {
   NewIndent = Style.IndentWidth +
   std::min(State.Column, CurrentState.NestedBlockIndent);
+} else if (NextNoComment &&
+   NextNoComment->is(TT_DesignatedInitializerPeriod)) {
+  const auto DesignatedInitializerIndentWidth =
+  Style.DesignatedInitializerIndentWidth < 0
+  ? Style.ContinuationIndentWidt

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

[Oliver Stöneberg via Phabricator via cfe-commits]

firewave added a comment.

In D137205#4225006 , @Febbe wrote:

> Yes, I agree, while I can't understand, why someone still wants to use only 
> c++11 I can totally understand, that a single Software Engineer can't do 
> anything about this, when the corporation does not permit it. 
> But it should not be removed, since this warning can still show some pitfalls 
> in performance critical code. A way in c++11 to fix the warning, is to create 
> a functor, instead of a lambda.

Well, compatibility with older platforms. And I personally have lots of 
quarrels with modern (and more recently even early) C++, but let's not get into 
that. Being able to move within captures is not one of them though...

I still think being able to tune that check for certain parts would be helpful 
- especially if it would not be fixable with a fix-it. It should also help with 
applying these findings to an existing code base as you could enable it 
incrementally (especially if you need to introduce something like functors).
Recent Clang additions like `misc-const-correctness` or `-Wunsafe-buffer-usage` 
would have profited from being just a bit more granular as they produce such a 
flood of warnings even for small code bases which makes it quite troublesome to 
adopt the code for those warnings as some of them are not just applying the 
fix-it but also need to be looked at in the bigger picture to see if they might 
not impact things negatively in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D146801: [clang] Fix consteval initializers of temporaries

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM! Precommit CI is still falling over because of the libcxx bot. That issue 
is being actively investigated, but we don't have an ETA on it being resolved. 
I think these changes are sufficiently safe to land and watch for post-commit 
failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146801

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added a comment.

I do not have the commit access, can you commit on my behalf @nickdesaulniers


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

https://reviews.llvm.org/D145726

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

[PATCH] D146234: [clang] Fix crash when handling nested immediate invocations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this LGTM, but I'm holding off on signing off until @shafik is 
satisfied with the part he was asking about. You should add a release note for 
the fix, too.




Comment at: clang/lib/Sema/SemaExpr.cpp:17891-17894
+  // It is possible that some subexpression of current immediate invocation
+  // was handled from another expression evaluation context. Do not handle
+  // current immediate invocation if some of its subexpressions failed
+  // before.

Minor grammar nits.



Comment at: clang/lib/Sema/SemaExpr.cpp:17973
 
-  /// When we have more then 1 ImmediateInvocationCandidates we need to check
-  /// for nested ImmediateInvocationCandidates. when we have only 1 we only
-  /// need to remove ReferenceToConsteval in the immediate invocation.
-  if (Rec.ImmediateInvocationCandidates.size() > 1) {
+  /// When we have more then 1 ImmediateInvocationCandidates or previously
+  /// failed immediate invocations, we need to check for nested





Comment at: clang/lib/Sema/SemaExpr.cpp:17979
+  if (Rec.ImmediateInvocationCandidates.size() > 1 ||
+  SemaRef.FailedImmediateInvocations.size()) {
 

Fznamznon wrote:
> cor3ntin wrote:
> > Shouln't we clear `FailedImmediateInvocations` at the end of a full 
> > expression to avoid going there if there was an error in another expression
> The idea sounds reasonable, but I'm not sure it can work with current 
> algorithm of handling immediate invocations. Immediate invocations are 
> handled when expression evaluation context is popped, so we need to remember 
> previously failed immediate invocations until then, even though they could 
> happen in two different full expressions. 
> 
> ```
> consteval foo() { ... }
> void bar() {
>   int a = foo(/* there might be a failed immediate invocation attached to 
> initializer context */); // this is a full-expression
>   int b = foo(); // this is another full-expression
> } // This is where immediate invocations that appear in function context are 
> processed
> ```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146234

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

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/OHOS.cpp:140
   getFilePaths().clear();
-  std::string CandidateLibPath = getArchSpecificLibPath();
-  if (getVFS().exists(CandidateLibPath))
-getFilePaths().push_back(CandidateLibPath);
+  for (const auto &CandidateLibPath : getArchSpecificLibPaths()) {
+if (getVFS().exists(CandidateLibPath))

delete braces


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

https://reviews.llvm.org/D146686

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

[PATCH] D146686: [Driver] Fix rpath for compiler-rt

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The new test can be placed in `arch-specific-libdir-rpath.c`. One or two 
additional RUN lines seem sufficient, no need to duplicate too many.

Also, use `--target=` for new tests and avoid `^//$`.


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

https://reviews.llvm.org/D146686

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

[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I know this is a bit of a redirection/scope creep/etc - but I'd quite like to 
see a solution that is likely to be usable for the "std::function" problem 
(stepping into std::function should allow you to reach the underlying function 
- but lldb currently skips any call to a std-namespaced function, I think, so 
you step right over the whole op() call) that could also cover the Swift needs. 
Though perhaps they're just sufficiently different problems that there is no 
generalizing here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[garvit gupta via Phabricator via cfe-commits]

garvitgupta08 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:976-985
+  bool IsInputTyAsm = false;
+  for (const auto &II : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

nickdesaulniers wrote:
> garvitgupta08 wrote:
> > nickdesaulniers wrote:
> > > Thinking about this more, does the issue still exist if the user passed 
> > > .c and .s/.S files together?
> > > 
> > > i.e. `$ clang ... -fno-integrated-as -gdwarf-4 foo.s main.c`?
> > Yes, the error will still be thrown for c/cpp files.
> So this patch is an incomplete fix then? Is there somewhere else we can move 
> this logic then so that it's only applied for individual files and not 
> multiple inputs?
No. I misunderstood your question. I thought you were asking what will happen 
in case of multiple inputs without this fix. I answered according to this. 

The current patch fixes the error in case of multiple inputs as well. No error 
will be thrown for c/cpp files. I hope this answers your original question.


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

https://reviews.llvm.org/D145726

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

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1230
+  llvm::sys::path::append(P, "llvm-libc");
+  CmdArgs.push_back("-c-isystem");
+  CmdArgs.push_back(Args.MakeArgString(P));

tra wrote:
> Ensuring the right include order will be tricky. Interaction between the 
> headers provided by llvm-libc vs the system headers will be interesting if we 
> end up mixing the headers. It may be less problematic compared to the C++ 
> standard library, but I doubt that mixing two implementations would work well 
> here, either. 
> 
> So, the major question I have -- does adding include path here guarantees 
> that we're not picking up the host headers? I do not see any changes that 
> would exclude system headers from include paths.
> If we do have include paths leading to both llvm-libc and the host headers, 
> what's expected to happen if user code ends up including a header that's not 
> present in  llvm-libc? Is that possible?
> 
Right now I'm just kind of relying on an expectation that since this will be 
the first `c-isystem` path set, then it will pull in these libraries first if 
they exist. It's not captured by the tests, but compiling with `-v` shows this 
path being used first in my experience. So, theoretically, if there is an 
implementation of said header in this location, it will be picked up before 
anything else. Otherwise it'll just search the other standard locations.



Comment at: clang/test/Driver/gpu-libc-headers.c:14
+// RUN: FileCheck %s --check-prefix=CHECK-HEADERS
+// CHECK-HEADERS: "-cc1"{{.*}}"-c-isystem" "{{.*}}include/llvm-libc"
+

tra wrote:
> I think here we want to test for not just the presence of 
> `include/llvm-libc`, but also that it's in the correct position relative to 
> other include paths. 
> 
> We may want something similar to what we have in 
> clang/test/Driver/hip-include-path.hip
Yeah, I wasn't sure if there was a good way to guarantee a certain path since 
those can change based on the system. Maybe `--sysroot`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146973

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

[PATCH] D145726: Fix assembler error when -g and -gdwarf-* is passed with -fno-integrated-as.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:976-985
+  bool IsInputTyAsm = false;
+  for (const auto &II : Inputs) {
+CmdArgs.push_back(II.getFilename());
+StringRef BaseInput = StringRef(II.getBaseInput());
+types::ID InputType = types::lookupTypeForExtension(
+llvm::sys::path::extension(BaseInput).drop_front());
+if (InputType == types::TY_Asm || InputType == types::TY_PP_Asm)

garvitgupta08 wrote:
> nickdesaulniers wrote:
> > Thinking about this more, does the issue still exist if the user passed .c 
> > and .s/.S files together?
> > 
> > i.e. `$ clang ... -fno-integrated-as -gdwarf-4 foo.s main.c`?
> Yes, the error will still be thrown for c/cpp files.
So this patch is an incomplete fix then? Is there somewhere else we can move 
this logic then so that it's only applied for individual files and not multiple 
inputs?


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

https://reviews.llvm.org/D145726

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