[PATCH] D132913: [HLSL] Preserve vec3 for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae created this revision.
python3kgae added reviewers: bogner, beanz, pow2clk.
Herald added a subscriber: Anastasia.
Herald added a project: All.
python3kgae requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Preserve vec3 for HLSL by set -fpreserve-vec3-type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132913

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenHLSL/float3.hlsl


Index: clang/test/CodeGenHLSL/float3.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/float3.hlsl
@@ -0,0 +1,11 @@
+// RUN: %clang --driver-mode=dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// Make sure float3 is not changed into float4.
+// CHECK:<3 x float> @"?foo@@YAT?$__vector@M$02@__clang@@T12@@Z"(<3 x float> 
noundef %a)
+// CHECK:%[[A_ADDR:.+]] = alloca <3 x float>, align 16
+// CHECK-NEXT:store <3 x float> %a, ptr %[[A_ADDR]], align 16
+// CHECK-NEXT:%[[V:[0-9]+]] = load <3 x float>, ptr %[[A_ADDR]], align 16
+// CHECK-NEXT:ret <3 x float> %[[V]]
+float3 foo(float3 a) {
+  return a;
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3520,6 +3520,8 @@
   // Add the default headers if dxc_no_stdinc is not set.
   if (!Args.hasArg(options::OPT_dxc_no_stdinc))
 CmdArgs.push_back("-finclude-default-header");
+  // Preserve vec3 for HLSL.
+  CmdArgs.push_back("-fpreserve-vec3-type");
 }
 
 static void RenderARCMigrateToolOptions(const Driver &D, const ArgList &Args,


Index: clang/test/CodeGenHLSL/float3.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/float3.hlsl
@@ -0,0 +1,11 @@
+// RUN: %clang --driver-mode=dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// Make sure float3 is not changed into float4.
+// CHECK:<3 x float> @"?foo@@YAT?$__vector@M$02@__clang@@T12@@Z"(<3 x float> noundef %a)
+// CHECK:%[[A_ADDR:.+]] = alloca <3 x float>, align 16
+// CHECK-NEXT:store <3 x float> %a, ptr %[[A_ADDR]], align 16
+// CHECK-NEXT:%[[V:[0-9]+]] = load <3 x float>, ptr %[[A_ADDR]], align 16
+// CHECK-NEXT:ret <3 x float> %[[V]]
+float3 foo(float3 a) {
+  return a;
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3520,6 +3520,8 @@
   // Add the default headers if dxc_no_stdinc is not set.
   if (!Args.hasArg(options::OPT_dxc_no_stdinc))
 CmdArgs.push_back("-finclude-default-header");
+  // Preserve vec3 for HLSL.
+  CmdArgs.push_back("-fpreserve-vec3-type");
 }
 
 static void RenderARCMigrateToolOptions(const Driver &D, const ArgList &Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Jack Huang via Phabricator via cfe-commits]

jackhong12 updated this revision to Diff 456563.
jackhong12 added a comment.

Annotate `*` as UnaryOperator instead of `PointerOrReference`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

Files:
  clang/lib/Format/TokenAnnotator.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
@@ -120,6 +120,17 @@
   Tokens = annotate("int i = int{42} * 2;");
   EXPECT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("delete[] *ptr;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] **ptr;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] *(ptr);");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10535,6 +10535,11 @@
"} &&ptr = {};",
Style);
 
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("delete[] *ptr;", Style);
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2372,6 +2372,12 @@
 !PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
+const FormatToken *Prev = PrevToken;
+if (Prev->is(tok::r_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::l_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::kw_delete))
+  return TT_UnaryOperator;
+
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -120,6 +120,17 @@
   Tokens = annotate("int i = int{42} * 2;");
   EXPECT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
+
+  Tokens = annotate("delete[] *ptr;");
+  EXPECT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] **ptr;");
+  EXPECT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
+  Tokens = annotate("delete[] *(ptr);");
+  EXPECT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10535,6 +10535,11 @@
"} &&ptr = {};",
Style);
 
+  Style.PointerAlignment = FormatStyle::PAS_Right;
+  verifyFormat("delete[] *ptr;", Style);
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2372,6 +2372,12 @@
 !PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
+const FormatToken *Prev = PrevToken;
+if (Prev->is(tok::r_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::l_square) && (Prev = Prev->getPreviousNonComment()) &&
+Prev->is(tok::kw_delete))
+  return TT_UnaryOperator;
+
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {

[PATCH] D131992: [Support] compression proposal for a enum->spec->impl approach

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

(I was out of town last week so I did not respond then.)

In D131992#3748306 , @dblaikie wrote:

> @MaskRay I think this change is probably the best point of 
> comparison/demonstration of this patch direction (taking some minor liberties 
> with this patch to simplify things somewhat in ways that have already been 
> discussed/seem quite reasonable - specifically having `getCompresisonSpec` 
> return a reference and the enum having either no "unknown" value, or 
> `getCompressionSpec` asserting on the unknown value):

I am sorry to disagree... As previously mentioned this is an expression problem.
When we maintain no state in the compression algorithms, the OO style has more 
boilerplate in llvm/lib/Support and in call sites, so I prefer the FP style 
(free function style) instead.

(Adding a compression algorithm has a significant barrier. I think we tend to 
add functions more than adding types, so FP style wins here.)

>   // removing the hardcoded zlib compression kind parameter in favor of what 
> the code would look like in the near future once it's parameterized on a 
> compression kind
>   CompressionImplementation *Compressor = 
> getCompressionSpec(CompressionAlgorithmKind)->Implementation;
>   bool DoCompression = Compress && DoInstrProfNameCompression && Compressor;
>   if (DoCompression) {
> Compressor->compress(arrayRefFromStringRef(FilenamesStr),
> CompressedStr,
> Compressor->BestSizeLevel);
>   }
>
> I think a reasonable speculation about what the alternative would look like 
> in the a non-OO design would be something like:
>
>   bool DoCompression = Compress && DoInstrProfNameCompression && 
> isAvailable(CompressionAlgorithmKind);
>   if (DoCompression) {
> compress(CompressionAlgorithmKind, arrayRefFromStringRef(FilenamesStr), 
> CompressedStr, getBestSizeLevel(CompressionAlgorithmKind));

Most call sites do not specify the compression level, so 
`getBestSizeLevel(CompressionAlgorithmKind)` is omitted.

> And having these three correlated calls (`isAvailable`, `compress`, and 
> `getBestSizeLevel`) all made independently/have to have the same compression 
> algorithm kind passed to them independently seems more error prone & 
> redundant (not in an actual execution/runtime efficiency perspective - I 
> don't think any of these different designs would have materially different 
> runtime performance - but in terms of "duplicate work, and work that could 
> diverge/become inconsistent" - essentially the duplicate work/repeated switch 
> statements, one in each `compression::*` generic entry point seems like a 
> code smell to me that points towards a design that doesn't have that 
> duplication) rather than tying these calls together and making the lookup 
> happen once and then using the features after that. Also exposing the 
> compression algorithm along with the availability in one operation (not 
> exposing compress/decompress/size APIs when the algorithm isn't available) 
> seems to have similar benefits.

In the FP style (free function style), `CompressionAlgorithmKind` is passed in 
multiple places. It is as if, in the OO style, the `Compressor` object is 
passed in multiple places.

`CompressionAlgorithmKind` is an scoped enum and there is hardly another 
variable of the same type in the scope of `CompressionAlgorithmKind`. So I do 
not see how the FP style is more error-prone.
To me, not checking `isAvailable(CompressionAlgorithmKind)` is similar to not 
checking the nullness of `Compressor`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131992

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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2379
+Prev->is(tok::kw_delete))
+  return TT_UnaryOperator;
+

Why unary here, doesn’t that make it a multiply?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Jack Huang via Phabricator via cfe-commits]

jackhong12 added a comment.

In this case, I think it's dereferencing a pointer instead of multiplying two 
numbers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

In D125418#3756223 , @efriedma wrote:

> There's no way the calling convention can change based on whether you're 
> calling a function vs. a function pointer.  I can't explain why MSVC is 
> generating different code.  I think we should just ignore it, at least for 
> now.

It's OK for me to ignore the difference but I think the main thing is not 
function or function pointer. It's how to generate the exit thunkwhen return 
with structure size value > 16.
https://godbolt.org/z/MWv4YaKdK
Three different way to call extern function, with three kind of exit thunks. 
All of them are keep the return value, not move the return value' point to the 
first argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

[Nathan James via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:66
+  if (needsParensAfterUnaryNegation(Condition)) {
+Diag << FixItHint::CreateInsertion(Condition->getBeginLoc(), "!(")
+ << FixItHint::CreateInsertion(

JonasToth wrote:
> did you consider comma expressions?
> 
> `if (myDirtyCode(), myCondition && yourCondition)`. It seems to me, that the 
> transformation would be incorrect.
Comma operator is a binary operator, so the transformation would wrap the whole 
expression in parens.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:63
+void Process(bool A, bool B) {
+  if (A && B) {
+// Long processing.

JonasToth wrote:
> if this option is false, the transformation would be `if(!(A && B))`, right?
> 
> should demorgan rules be applied or at least be mentioned here? I think 
> transforming to `if (!A || !B)` is at least a viable option for enough users.
Once this is in, I plan to merge some common code with the 
simplify-boolean-expr logic for things like demorgan processing. Right now the 
transformation happens, the simplify boolean suggests a demorgan transformation 
of you run the output through clang tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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

[PATCH] D132916: [clang] Explicitly set the EmulatedTLS codegen option

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: aaron.ballman, rjmccall, efriedma.
Herald added a subscriber: ormris.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang.

Set the EmulatedTLS option based on `Triple::hasDefaultEmulatedTLS()`
if the user didn't specify it; set `ExplicitEmulatedTLS` to true
in `llvm::TargetOptions` and set `EmulatedTLS` to Clang's
opinion of what the default or preference is.

This avoids any risk of deviance between the two.

This affects one check of `getCodeGenOpts().EmulatedTLS` in
`shouldAssumeDSOLocal` in CodeGenModule, but as that check only
is done for `TT.isWindowsGNUEnvironment()`, and
`hasDefaultEmulatedTLS()` returns false for such environments
it doesn't make any current testable difference.

Some mingw distributions carry a downstream patch, that enables
emulated TLS by default for mingw targets in `hasDefaultEmulatedTLS()`

- and for such cases, this patch does make a difference and fixes the

detection of emulated TLS, if it is implicitly enabled.

This is an alternative to D132848 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132916

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1492,13 +1492,8 @@
 F.Filename, SA);
   }
 
-  // TODO: Consider removing marshalling annotations from f[no_]emulated_tls.
-  //  That would make it easy to generate the option only **once** if it was
-  //  explicitly set to non-default value.
-  if (Opts.ExplicitEmulatedTLS) {
-GenerateArg(
-Args, Opts.EmulatedTLS ? OPT_femulated_tls : OPT_fno_emulated_tls, SA);
-  }
+  GenerateArg(
+  Args, Opts.EmulatedTLS ? OPT_femulated_tls : OPT_fno_emulated_tls, SA);
 
   if (Opts.FPDenormalMode != llvm::DenormalMode::getIEEE())
 GenerateArg(Args, OPT_fdenormal_fp_math_EQ, Opts.FPDenormalMode.str(), SA);
@@ -1862,9 +1857,9 @@
 Opts.LinkBitcodeFiles.push_back(F);
   }
 
-  if (Args.getLastArg(OPT_femulated_tls) ||
-  Args.getLastArg(OPT_fno_emulated_tls)) {
-Opts.ExplicitEmulatedTLS = true;
+  if (!Args.getLastArg(OPT_femulated_tls) &&
+  !Args.getLastArg(OPT_fno_emulated_tls)) {
+Opts.EmulatedTLS = T.hasDefaultEmulatedTLS();
   }
 
   if (Arg *A = Args.getLastArg(OPT_ftlsmodel_EQ)) {
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -422,7 +422,7 @@
   CodeGenOpts.UniqueBasicBlockSectionNames;
   Options.TLSSize = CodeGenOpts.TLSSize;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
-  Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;
+  Options.ExplicitEmulatedTLS = true;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
   Options.StackUsageOutput = CodeGenOpts.StackUsageOutput;
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -88,7 +88,6 @@
 CODEGENOPT(EmitGcovNotes , 1, 0) ///< Emit coverage "notes" files, aka 
GCNO.
 CODEGENOPT(EmitOpenCLArgMetadata , 1, 0) ///< Emit OpenCL kernel arg metadata.
 CODEGENOPT(EmulatedTLS   , 1, 0) ///< Set by default or 
-f[no-]emulated-tls.
-CODEGENOPT(ExplicitEmulatedTLS , 1, 0) ///< Set if -f[no-]emulated-tls is used.
 /// Embed Bitcode mode (off/all/bitcode/marker).
 ENUM_CODEGENOPT(EmbedBitcode, EmbedBitcodeKind, 2, Embed_Off)
 /// Inline asm dialect, -masm=(att|intel)


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1492,13 +1492,8 @@
 F.Filename, SA);
   }
 
-  // TODO: Consider removing marshalling annotations from f[no_]emulated_tls.
-  //  That would make it easy to generate the option only **once** if it was
-  //  explicitly set to non-default value.
-  if (Opts.ExplicitEmulatedTLS) {
-GenerateArg(
-Args, Opts.EmulatedTLS ? OPT_femulated_tls : OPT_fno_emulated_tls, SA);
-  }
+  GenerateArg(
+  Args, Opts.EmulatedTLS ? OPT_femulated_tls : OPT_fno_emulated_tls, SA);
 
   if (Opts.FPDenormalMode != llvm::DenormalMode::getIEEE())
 GenerateArg(Args, OPT_fdenormal_fp_math_EQ, Opts.FPDenormalMode.str(), SA);
@@ -1862,9 +1857,9 @@
 Opts.LinkBitcodeFiles.push_back(F);
   }
 
-  if (Args.getLastArg(OPT_femulated_tls) ||
-  Args.getLastArg(OPT_fno_emulated_tls)) {
-Opts.Expli

[PATCH] D132918: [clang] Fix a crash in constant evaluation

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: usaxena95, ilya-biryukov.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was showing up in our internal crash collector. I have no idea how
to test it out though, open for suggestions if there are easy paths but
otherwise I'd move forward with the patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132918

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4794,6 +4794,11 @@
   Result = APValue((const FieldDecl *)nullptr);
   return true;
 }
+// Can't access properties of an incomplete type.
+if (!RD->hasDefinition()) {
+  Result = APValue();
+  return false;
+}
 Result = APValue(APValue::UninitStruct(), RD->getNumBases(),
  std::distance(RD->field_begin(), RD->field_end()));
 


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4794,6 +4794,11 @@
   Result = APValue((const FieldDecl *)nullptr);
   return true;
 }
+// Can't access properties of an incomplete type.
+if (!RD->hasDefinition()) {
+  Result = APValue();
+  return false;
+}
 Result = APValue(APValue::UninitStruct(), RD->getNumBases(),
  std::distance(RD->field_begin(), RD->field_end()));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 created this revision.
usaxena95 added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
usaxena95 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132919

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -337,7 +337,7 @@
 "folding-ranges",
 cat(Features),
 desc("Enable preview of FoldingRanges feature"),
-init(false),
+init(true),
 Hidden,
 };
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -337,7 +337,7 @@
 "folding-ranges",
 cat(Features),
 desc("Enable preview of FoldingRanges feature"),
-init(false),
+init(true),
 Hidden,
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132920: [clang] Silence a false positive GCC -Wunused-but-set-parameter warning with constexpr

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: bkramer.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang.

This fixes the following warning:

  In file included from 
../tools/clang/lib/Tooling/Transformer/Transformer.cpp:9:
  ../tools/clang/include/clang/Tooling/Transformer/Transformer.h: In 
instantiation of ‘llvm::Error clang::tooling::detail::populateMetadata(const 
clang::transformer::RewriteRuleWith&, size_t, const 
clang::ast_matchers::MatchFinder::MatchResult&, 
clang::tooling::TransformerResult&) [with T = void; size_t = long unsigned 
int]’:
  ../tools/clang/include/clang/Tooling/Transformer/Transformer.h:179:34:   
required from ‘void 
clang::tooling::detail::WithMetadataImpl::onMatchImpl(const 
clang::ast_matchers::MatchFinder::MatchResult&) [with T = void]’
  ../tools/clang/include/clang/Tooling/Transformer/Transformer.h:156:8:   
required from here
  ../tools/clang/include/clang/Tooling/Transformer/Transformer.h:120:25: 
warning: parameter ‘SelectedCase’ set but not used [-Wunused-but-set-parameter]
120 |  size_t SelectedCase,
|  ~~~^~~~

The issue is fixed in GCC 10 and later, but this silences the noisy
warning in older versions. See 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85827
for more details about the bug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132920

Files:
  clang/include/clang/Tooling/Transformer/Transformer.h


Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -120,6 +120,11 @@
  size_t SelectedCase,
  const ast_matchers::MatchFinder::MatchResult &Match,
  TransformerResult &Result) {
+  // Silence a false positive GCC -Wunused-but-set-parameter warning in 
constexpr
+  // cases, by marking SelectedCase as used. See
+  // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85827 for details. The issue 
is
+  // fixed in GCC 10.
+  (void)SelectedCase;
   if constexpr (!std::is_void_v) {
 auto Metadata = Rule.Metadata[SelectedCase]->eval(Match);
 if (!Metadata)


Index: clang/include/clang/Tooling/Transformer/Transformer.h
===
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -120,6 +120,11 @@
  size_t SelectedCase,
  const ast_matchers::MatchFinder::MatchResult &Match,
  TransformerResult &Result) {
+  // Silence a false positive GCC -Wunused-but-set-parameter warning in constexpr
+  // cases, by marking SelectedCase as used. See
+  // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85827 for details. The issue is
+  // fixed in GCC 10.
+  (void)SelectedCase;
   if constexpr (!std::is_void_v) {
 auto Metadata = Rule.Metadata[SelectedCase]->eval(Match);
 if (!Metadata)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126498: [clangd] Fix hover crashing on integral or enumeral casts

[Georg Kotheimer via Phabricator via cfe-commits]

gkll requested review of this revision.
gkll marked an inline comment as done.
gkll added a comment.

Mhm, I think the change here has gone under the radar 🙈


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

https://reviews.llvm.org/D126498

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

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Trass3r via Phabricator via cfe-commits]

Trass3r added a comment.

Whenever I tried this option in the past it crashed clangd.
I recommend doing some more testing before flipping the switch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132919

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

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:340
 desc("Enable preview of FoldingRanges feature"),
-init(false),
+init(true),
 Hidden,

i think we should just retire the flag altogether, ATM this is only preventing 
clangdlspserver from announcing the capability.

the only benefit of keeping it as a flag is, we can turn it off quickly if its 
crashing constantly. but we should fix those crashes instead. so do you mind:
- moving it to be near other retiredflags (around line 320)
- always announce foldingrangeprovider capability and bind the method in 
clangdlspserver.cpp
- get rid of the option inside clangdserver.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132919

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

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D132919#3757685 , @Trass3r wrote:

> Whenever I tried this option in the past it crashed clangd.
> I recommend doing some more testing before flipping the switch.

we've a new implementation of folding ranges based on pseudo parsing now. hence 
no more crashes 🎉


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132919

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

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 456584.
usaxena95 marked an inline comment as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132919

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,8 @@
 RetiredFlag CrossFileRename("cross-file-rename");
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
+RetiredFlag FoldingRanges("folding-ranges");
+
 
 opt LimitResults{
 "limit-results",
@@ -333,14 +335,6 @@
 CommaSeparated,
 };
 
-opt FoldingRanges{
-"folding-ranges",
-cat(Features),
-desc("Enable preview of FoldingRanges feature"),
-init(false),
-Hidden,
-};
-
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -903,7 +897,6 @@
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
-  Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -43,6 +43,7 @@
 # CHECK-NEXT:  "clangd.applyTweak"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:  "foldingRangeProvider": true,
 # CHECK-NEXT:  "hoverProvider": true,
 # CHECK-NEXT:  "implementationProvider": true,
 # CHECK-NEXT:  "inlayHintProvider": true,
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -161,9 +161,6 @@
 /// fetch system include path.
 std::vector QueryDriverGlobs;
 
-/// Enable preview of FoldingRanges feature.
-bool FoldingRanges = false;
-
 // Whether the client supports folding only complete lines.
 bool LineFoldingOnly = false;
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -584,6 +584,7 @@
   {"callHierarchyProvider", true},
   {"clangdInlayHintsProvider", true},
   {"inlayHintProvider", true},
+  {"foldingRangeProvider", true},
   };
 
   {
@@ -613,8 +614,6 @@
  CodeAction::INFO_KIND}}}
   : llvm::json::Value(true);
 
-  if (Opts.FoldingRanges)
-ServerCaps["foldingRangeProvider"] = true;
 
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
@@ -1618,8 +1617,7 @@
   Bind.method("clangd/inlayHints", this, &ClangdLSPServer::onClangdInlayHints);
   Bind.method("textDocument/inlayHint", this, &ClangdLSPServer::onInlayHint);
   Bind.method("$/memoryUsage", this, &ClangdLSPServer::onMemoryUsage);
-  if (Opts.FoldingRanges)
-Bind.method("textDocument/foldingRange", this, 
&ClangdLSPServer::onFoldingRange);
+  Bind.method("textDocument/foldingRange", this, 
&ClangdLSPServer::onFoldingRange);
   Bind.command(ApplyFixCommand, this, &ClangdLSPServer::onCommandApplyEdit);
   Bind.command(ApplyTweakCommand, this, &ClangdLSPServer::onCommandApplyTweak);
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,8 @@
 RetiredFlag CrossFileRename("cross-file-rename");
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
+RetiredFlag FoldingRanges("folding-ranges");
+
 
 opt LimitResults{
 "limit-results",
@@ -333,14 +335,6 @@
 CommaSeparated,
 };
 
-opt FoldingRanges{
-"folding-ranges",
-cat(Features),
-desc("Enable preview of FoldingRanges feature"),
-init(false),
-Hidden,
-};
-
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -903,7 +897,6 @@
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
-  Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-param

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[Jack Huang via Phabricator via cfe-commits]

jackhong12 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2381-2385
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {
   return TT_BinaryOperator;
 }

The `*` token in `delete *x;` will be annotated as UnaryOperator (dereferencing 
a pointer). 

In the case `delete[] *x;`, there is a `]` before `*`. So, it will be annotated 
as BinaryOperator by this rule.

I think both `*` here should be UnaryOperator. Therefore, I add a new rule to 
match the `delete[]` pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132919

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

[clang-tools-extra] c338735 - [clangd] Enable folding ranges by default.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-08-30T12:04:59+02:00
New Revision: c338735020033e49fe36369ce3e49e316b6d2da0

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

LOG: [clangd] Enable folding ranges by default.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index e0129d99d9706..449ac9e3a85bd 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -584,6 +584,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   {"callHierarchyProvider", true},
   {"clangdInlayHintsProvider", true},
   {"inlayHintProvider", true},
+  {"foldingRangeProvider", true},
   };
 
   {
@@ -613,8 +614,6 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
  CodeAction::INFO_KIND}}}
   : llvm::json::Value(true);
 
-  if (Opts.FoldingRanges)
-ServerCaps["foldingRangeProvider"] = true;
 
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
@@ -1618,8 +1617,7 @@ void ClangdLSPServer::bindMethods(LSPBinder &Bind,
   Bind.method("clangd/inlayHints", this, &ClangdLSPServer::onClangdInlayHints);
   Bind.method("textDocument/inlayHint", this, &ClangdLSPServer::onInlayHint);
   Bind.method("$/memoryUsage", this, &ClangdLSPServer::onMemoryUsage);
-  if (Opts.FoldingRanges)
-Bind.method("textDocument/foldingRange", this, 
&ClangdLSPServer::onFoldingRange);
+  Bind.method("textDocument/foldingRange", this, 
&ClangdLSPServer::onFoldingRange);
   Bind.command(ApplyFixCommand, this, &ClangdLSPServer::onCommandApplyEdit);
   Bind.command(ApplyTweakCommand, this, &ClangdLSPServer::onCommandApplyTweak);
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 6be0c930ded47..13eed0784c589 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -161,9 +161,6 @@ class ClangdServer {
 /// fetch system include path.
 std::vector QueryDriverGlobs;
 
-/// Enable preview of FoldingRanges feature.
-bool FoldingRanges = false;
-
 // Whether the client supports folding only complete lines.
 bool LineFoldingOnly = false;
 

diff  --git a/clang-tools-extra/clangd/test/initialize-params.test 
b/clang-tools-extra/clangd/test/initialize-params.test
index 8387fb9b504be..e7810c66f44a2 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -43,6 +43,7 @@
 # CHECK-NEXT:  "clangd.applyTweak"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:  "foldingRangeProvider": true,
 # CHECK-NEXT:  "hoverProvider": true,
 # CHECK-NEXT:  "implementationProvider": true,
 # CHECK-NEXT:  "inlayHintProvider": true,

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 76cfaf7a35dfe..11c5acb71d978 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,8 @@ RetiredFlag 
CollectMainFileRefs("collect-main-file-refs");
 RetiredFlag CrossFileRename("cross-file-rename");
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
+RetiredFlag FoldingRanges("folding-ranges");
+
 
 opt LimitResults{
 "limit-results",
@@ -333,14 +335,6 @@ list TweakList{
 CommaSeparated,
 };
 
-opt FoldingRanges{
-"folding-ranges",
-cat(Features),
-desc("Enable preview of FoldingRanges feature"),
-init(false),
-Hidden,
-};
-
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -903,7 +897,6 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
-  Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;



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

[PATCH] D132919: [clangd] Enable folding ranges by default.

[Utkarsh Saxena 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 rGc33873502003: [clangd] Enable folding ranges by default. 
(authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132919

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,8 @@
 RetiredFlag CrossFileRename("cross-file-rename");
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
+RetiredFlag FoldingRanges("folding-ranges");
+
 
 opt LimitResults{
 "limit-results",
@@ -333,14 +335,6 @@
 CommaSeparated,
 };
 
-opt FoldingRanges{
-"folding-ranges",
-cat(Features),
-desc("Enable preview of FoldingRanges feature"),
-init(false),
-Hidden,
-};
-
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -903,7 +897,6 @@
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
-  Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -43,6 +43,7 @@
 # CHECK-NEXT:  "clangd.applyTweak"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
+# CHECK-NEXT:  "foldingRangeProvider": true,
 # CHECK-NEXT:  "hoverProvider": true,
 # CHECK-NEXT:  "implementationProvider": true,
 # CHECK-NEXT:  "inlayHintProvider": true,
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -161,9 +161,6 @@
 /// fetch system include path.
 std::vector QueryDriverGlobs;
 
-/// Enable preview of FoldingRanges feature.
-bool FoldingRanges = false;
-
 // Whether the client supports folding only complete lines.
 bool LineFoldingOnly = false;
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -584,6 +584,7 @@
   {"callHierarchyProvider", true},
   {"clangdInlayHintsProvider", true},
   {"inlayHintProvider", true},
+  {"foldingRangeProvider", true},
   };
 
   {
@@ -613,8 +614,6 @@
  CodeAction::INFO_KIND}}}
   : llvm::json::Value(true);
 
-  if (Opts.FoldingRanges)
-ServerCaps["foldingRangeProvider"] = true;
 
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
@@ -1618,8 +1617,7 @@
   Bind.method("clangd/inlayHints", this, &ClangdLSPServer::onClangdInlayHints);
   Bind.method("textDocument/inlayHint", this, &ClangdLSPServer::onInlayHint);
   Bind.method("$/memoryUsage", this, &ClangdLSPServer::onMemoryUsage);
-  if (Opts.FoldingRanges)
-Bind.method("textDocument/foldingRange", this, 
&ClangdLSPServer::onFoldingRange);
+  Bind.method("textDocument/foldingRange", this, 
&ClangdLSPServer::onFoldingRange);
   Bind.command(ApplyFixCommand, this, &ClangdLSPServer::onCommandApplyEdit);
   Bind.command(ApplyTweakCommand, this, &ClangdLSPServer::onCommandApplyTweak);
 


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,8 @@
 RetiredFlag CrossFileRename("cross-file-rename");
 RetiredFlag ClangTidyChecks("clang-tidy-checks");
 RetiredFlag InlayHints("inlay-hints");
+RetiredFlag FoldingRanges("folding-ranges");
+
 
 opt LimitResults{
 "limit-results",
@@ -333,14 +335,6 @@
 CommaSeparated,
 };
 
-opt FoldingRanges{
-"folding-ranges",
-cat(Features),
-desc("Enable preview of FoldingRanges feature"),
-init(false),
-Hidden,
-};
-
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -903,7 +897,6 @@
 Opts.StaticIndex = PAI.get();
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
-  Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/test/initialize-params.test
===

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 456608.
melver added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130888

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize-metadata.c

Index: clang/test/Driver/fsanitize-metadata.c
===
--- /dev/null
+++ clang/test/Driver/fsanitize-metadata.c
@@ -0,0 +1,34 @@
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=all \
+// RUN:   -fno-experimental-sanitize-metadata=all %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: -fexperimental-sanitize-metadata
+
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=bad_arg %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-INVALID %s
+// CHECK-INVALID: error: unsupported argument 'bad_arg' to option '-fexperimental-sanitize-metadata='
+
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=covered %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-COVERED %s
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=atomics \
+// RUN:   -fno-experimental-sanitize-metadata=atomics \
+// RUN:   -fexperimental-sanitize-metadata=covered %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-COVERED %s
+// CHECK-COVERED: "-fexperimental-sanitize-metadata=covered"
+// CHECK-COVERED-NOT: "-fexperimental-sanitize-metadata=atomics"
+
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=atomics %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ATOMICS %s
+// CHECK-ATOMICS: "-fexperimental-sanitize-metadata=atomics"
+
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=covered,atomics %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ALL %s
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=covered \
+// RUN:   -fexperimental-sanitize-metadata=atomics %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ALL %s
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=all %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ALL %s
+// CHECK-ALL: "-fexperimental-sanitize-metadata=covered"
+// CHECK-ALL: "-fexperimental-sanitize-metadata=atomics"
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -100,6 +100,11 @@
   CoverageTraceStores = 1 << 17,
 };
 
+enum BinaryMetadataFeature {
+  BinaryMetadataCovered = 1 << 0,
+  BinaryMetadataAtomics = 1 << 1,
+};
+
 /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
 /// invalid components. Returns a SanitizerMask.
 static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
@@ -110,6 +115,11 @@
 static int parseCoverageFeatures(const Driver &D, const llvm::opt::Arg *A,
  bool DiagnoseErrors);
 
+/// Parse -f(no-)?sanitize-metadata= flag values, diagnosing any invalid
+/// components. Returns OR of members of \c BinaryMetadataFeature enumeration.
+static int parseBinaryMetadataFeatures(const Driver &D, const llvm::opt::Arg *A,
+   bool DiagnoseErrors);
+
 /// Produce an argument string from ArgList \p Args, which shows how it
 /// provides some sanitizer kind from \p Mask. For example, the argument list
 /// "-fsanitize=thread,vptr -fsanitize=address" with mask \c NeedsUbsanRt
@@ -834,6 +844,22 @@
 DiagnoseErrors);
   }
 
+  // Parse -f(no-)?sanitize-metadata.
+  for (const auto *Arg :
+   Args.filtered(options::OPT_fexperimental_sanitize_metadata_EQ,
+ options::OPT_fno_experimental_sanitize_metadata_EQ)) {
+if (Arg->getOption().matches(
+options::OPT_fexperimental_sanitize_metadata_EQ)) {
+  Arg->claim();
+  BinaryMetadataFeatures |=
+  parseBinaryMetadataFeatures(D, Arg, DiagnoseErrors);
+} else {
+  Arg->claim();
+  BinaryMetadataFeatures &=
+  ~parseBinaryMetadataFeatures(D, Arg, DiagnoseErrors);
+}
+  }
+
   SharedRuntime =
   Args.hasFlag(options::OPT_shared_libsan, options::OPT_static_libsan,
TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia() ||
@@ -1095,6 +1121,17 @@
   addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-coverage-ignorelist=",
 CoverageIgnorelistFiles);
 
+  // Translate available BinaryMetadataFeatures to corresponding clang-cc1
+  // flags. Does not depend on any other sanitizers.
+  const std::pair BinaryMetadataFlags[] = {
+  std::make

[PATCH] D132920: [clang] Silence a false positive GCC -Wunused-but-set-parameter warning with constexpr

[Benjamin Kramer via Phabricator via cfe-commits]

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

thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132920

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

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Yeah, this makes sense to me if we factor out the commonalities. I agree that 
it's nicer to have a single option vs. having to specify every single native 
tool individually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131052

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

[PATCH] D132853: [clang] Fix -Warray-bound interaction with -fstrict-flex-arrays=1

[Aaron Ballman via Phabricator via cfe-commits]

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

Thank you for the updated description! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132853

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

[PATCH] D132918: [clang] Fix a crash in constant evaluation

[Utkarsh Saxena via Phabricator via cfe-commits]

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

Thanks. This looks reasonable. 
As discussed offline, this verifiably fixes the crash but it is hard to come up 
with a reduced reproducer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132918

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

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

[Aaron Ballman via Phabricator via cfe-commits]

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

This LGTM but I'd appreciate it if @dblaikie could also give it a once over so 
we have more than one sets of eyes on the changes in case I've missed something.




Comment at: clang/test/CodeGenCXX/exception-spec-decay.cpp:1
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions %s -triple=i686-unknown-linux 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %stdcxx_98- -fcxx-exceptions -fexceptions 
-Wno-dynamic-exception-spec %s -triple=i686-unknown-linux -emit-llvm -o - | 
FileCheck %s
 typedef int Array[10];

MaskRay wrote:
> aaron.ballman wrote:
> > Should we drop the `%stdcxx_98-` entirely from tests and not have any 
> > `-std` flag (e.g., no such flags tells lit to run the test in all language 
> > modes, eventually)?
> The proposal is probably clean if we the majority of tests work with C++98, 
> but I think we have accrued many tests which don't work with C++98 so we need 
> directives like `%stdcxx_11-`.
> 
> Since C++98 is actually uncommon now. I prefer explicit `%stdcxx_98-` to 
> indicate a test works with C++98.
> Since C++98 is actually uncommon now. I prefer explicit %stdcxx_98- to 
> indicate a test works with C++98.

Heheh, there's still a *ton* of C++98 code out there, but I'm okay being 
explicit just the same (it makes the tests easier to read).



Comment at: clang/test/CodeGenCXX/override-layout.cpp:1-9
+// RUN: %clang_cc1 -std=c++14 -w -fdump-record-layouts-simple %s > %t.layouts
+// RUN: %clang_cc1 -std=c++14 -w -fdump-record-layouts-simple %s > %t.before
+// RUN: %clang_cc1 -std=c++14 -w -DPACKED= -DALIGNED16= 
-fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after
 // RUN: diff -u %t.before %t.after
-// RUN: FileCheck %s < %t.after
+// RUN: FileCheck --check-prefixes=CHECK,PRE17 %s < %t.after
+
+// RUN: %clang_cc1 -std=c++17 -w -fdump-record-layouts-simple %s > %t.layouts

MaskRay wrote:
> aaron.ballman wrote:
> > Pre 14? Post 17?
> Unfortunately, C++17 and C++20 have different behaviors. I haven't 
> investigated why it is the case.
Ah, that's fine to do in a follow-up then. Thanks!



Comment at: clang/test/Layout/ms-x86-vtordisp.cpp:1-3
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fms-extensions -emit-llvm-only 
-triple i686-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 \
 // RUN:| FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fms-extensions -emit-llvm-only 
-triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>/dev/null \

MaskRay wrote:
> aaron.ballman wrote:
> > Is this test specific to C++14?
> This is similar to the previous -fdump-record-layouts test that the dump 
> order is different across 14, 17, 20. I do now know whether there is 
> something which should be improved to add the coverage.
> 
> For now I assume it is not this patch's responsibility to address it.
Agreed, thanks for the explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131464

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

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan via Phabricator via cfe-commits]

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

This is a valid HTML5 tag. Previously it triggered a Clang error (`HTML start 
tag prematurely ended, expected attribute name or '>'`) since Clang was 
treating `/>` as a text token. This was happening because after lexing the 
closing quote (`"`) the lexer state was reset to "Normal" while the tag was not 
actually closed yet: `>` was not yet parsed at that point.

rdar://91464292


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132932

Files:
  clang/lib/AST/CommentLexer.cpp
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -62,6 +62,21 @@
 /// 
 int test_html11(int);
 
+/// Aaa bbb
+int test_html_img1(int);
+
+/// Aaa bbb
+int test_html_img2(int);
+
+/// Aaa bbb
+int test_html_img3(int);
+
+/// Aaa bbb
+int test_html_img3(int);
+
+/// Aaa bbb
+int test_html_img4(int);
+
 /// Meow
 int test_html_nesting1(int);
 
Index: clang/lib/AST/CommentLexer.cpp
===
--- clang/lib/AST/CommentLexer.cpp
+++ clang/lib/AST/CommentLexer.cpp
@@ -701,7 +701,7 @@
 
   C = *BufferPtr;
   if (!isHTMLIdentifierStartingCharacter(C) &&
-  C != '=' && C != '\"' && C != '\'' && C != '>') {
+  C != '=' && C != '\"' && C != '\'' && C != '>' && C != '/') {
 State = LS_Normal;
 return;
   }


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -62,6 +62,21 @@
 /// 
 int test_html11(int);
 
+/// Aaa bbb
+int test_html_img1(int);
+
+/// Aaa bbb
+int test_html_img2(int);
+
+/// Aaa bbb
+int test_html_img3(int);
+
+/// Aaa bbb
+int test_html_img3(int);
+
+/// Aaa bbb
+int test_html_img4(int);
+
 /// Meow
 int test_html_nesting1(int);
 
Index: clang/lib/AST/CommentLexer.cpp
===
--- clang/lib/AST/CommentLexer.cpp
+++ clang/lib/AST/CommentLexer.cpp
@@ -701,7 +701,7 @@
 
   C = *BufferPtr;
   if (!isHTMLIdentifierStartingCharacter(C) &&
-  C != '=' && C != '\"' && C != '\'' && C != '>') {
+  C != '=' && C != '\"' && C != '\'' && C != '>' && C != '/') {
 State = LS_Normal;
 return;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/test/Sema/warn-documentation.cpp:78
+/// Aaa bbb
+int test_html_img4(int);
+

Could you also add these tests to 
clang/test/Index/comment-to-html-xml-conversion.cpp to show that the whole tag 
is captured correctly? (for example, the ">" is correctly captured as a part of 
the tag, not as plain text)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

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

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 456624.
wyt added a comment.

Replace use of virtual functions with sfinea and CRTP to call transfer 
functions defined by the user.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> &BlockStates;
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [&PostVisitStmt, &Context](
-   const CFGStmt &Stmt,
-   const TypeErasedDataflowAnalysisState &State) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [&PostVisitCFG, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto &BlockStates = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[&PostVisitStmt](ASTContext &Context, const CFGElement &Element,
+ const TypeErasedDataflowAnalysisState &State) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [&VerifyResults](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [&Results,
-   &Annotations](const clang::CFGStmt &Stmt,
+   &Annotations](const clang::CFGElement &Element,
  const TypeErasedDataflowAnalysisState &State) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend

[clang] 31dd025 - [clang] Fix -Warray-bound interaction with -fstrict-flex-arrays=1

[via cfe-commits]

Author: serge-sans-paille
Date: 2022-08-30T14:49:37+02:00
New Revision: 31dd0255aeaab7f2270abd0454e835842cfdcbb0

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

LOG: [clang] Fix -Warray-bound interaction with -fstrict-flex-arrays=1

The test to check if an array was a FAM in the context of array bound checking
and strict-flex-arrays=1 was inverted.

As a by product, improve test coverage.

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index bb4ca8c1e4209..e2e3d64fcade7 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15852,7 +15852,7 @@ static bool IsTailPaddedMemberArray(Sema &S, const 
llvm::APInt &Size,
   if (StrictFlexArraysLevel >= 2 && Size != 0)
 return false;
 
-  if (StrictFlexArraysLevel == 1 && Size.ule(1))
+  if (StrictFlexArraysLevel == 1 && Size.uge(2))
 return false;
 
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing

diff  --git a/clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp 
b/clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
index 225a804b3171c..7136973a8ab8e 100644
--- a/clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
+++ b/clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fstrict-flex-arrays=2 %s
+// RUN: %clang_cc1 -verify=relaxed -fstrict-flex-arrays=1 %s
+// RUN: %clang_cc1 -verify=relaxed,strict -fstrict-flex-arrays=2 %s
 
 // We cannot know for sure the size of a flexible array.
 struct t {
@@ -9,16 +10,26 @@ void test(t *s2) {
   s2->a[2] = 0; // no-warning
 }
 
-// Under -fstrict-flex-arrays `a` is not a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is not a flexible array
+struct t0 {
+  int f;
+  int a[10]; // relaxed-note {{array 'a' declared here}}
+};
+void test0(t0 *s2) {
+  s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the 
array (which contains 10 elements)}}
+}
+
+
+// Under -fstrict-flex-arrays=2 `a` is not a flexible array, but it is under 
-fstrict-flex-arrays=1
 struct t1 {
   int f;
-  int a[1]; // expected-note {{array 'a' declared here}}
+  int a[1]; // strict-note {{array 'a' declared here}}
 };
 void test1(t1 *s2) {
-  s2->a[2] = 0; // expected-warning {{array index 2 is past the end of the 
array (which contains 1 element)}}
+  s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array 
(which contains 1 element)}}
 }
 
-// Under -fstrict-flex-arrays `a` is a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is a flexible array.
 struct t2 {
   int f;
   int a[0];



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

[PATCH] D132853: [clang] Fix -Warray-bound interaction with -fstrict-flex-arrays=1

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG31dd0255aeaa: [clang] Fix -Warray-bound interaction with 
-fstrict-flex-arrays=1 (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132853

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp


Index: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
===
--- clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
+++ clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fstrict-flex-arrays=2 %s
+// RUN: %clang_cc1 -verify=relaxed -fstrict-flex-arrays=1 %s
+// RUN: %clang_cc1 -verify=relaxed,strict -fstrict-flex-arrays=2 %s
 
 // We cannot know for sure the size of a flexible array.
 struct t {
@@ -9,16 +10,26 @@
   s2->a[2] = 0; // no-warning
 }
 
-// Under -fstrict-flex-arrays `a` is not a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is not a flexible array
+struct t0 {
+  int f;
+  int a[10]; // relaxed-note {{array 'a' declared here}}
+};
+void test0(t0 *s2) {
+  s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the 
array (which contains 10 elements)}}
+}
+
+
+// Under -fstrict-flex-arrays=2 `a` is not a flexible array, but it is under 
-fstrict-flex-arrays=1
 struct t1 {
   int f;
-  int a[1]; // expected-note {{array 'a' declared here}}
+  int a[1]; // strict-note {{array 'a' declared here}}
 };
 void test1(t1 *s2) {
-  s2->a[2] = 0; // expected-warning {{array index 2 is past the end of the 
array (which contains 1 element)}}
+  s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array 
(which contains 1 element)}}
 }
 
-// Under -fstrict-flex-arrays `a` is a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is a flexible array.
 struct t2 {
   int f;
   int a[0];
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15852,7 +15852,7 @@
   if (StrictFlexArraysLevel >= 2 && Size != 0)
 return false;
 
-  if (StrictFlexArraysLevel == 1 && Size.ule(1))
+  if (StrictFlexArraysLevel == 1 && Size.uge(2))
 return false;
 
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing


Index: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
===
--- clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
+++ clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fstrict-flex-arrays=2 %s
+// RUN: %clang_cc1 -verify=relaxed -fstrict-flex-arrays=1 %s
+// RUN: %clang_cc1 -verify=relaxed,strict -fstrict-flex-arrays=2 %s
 
 // We cannot know for sure the size of a flexible array.
 struct t {
@@ -9,16 +10,26 @@
   s2->a[2] = 0; // no-warning
 }
 
-// Under -fstrict-flex-arrays `a` is not a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is not a flexible array
+struct t0 {
+  int f;
+  int a[10]; // relaxed-note {{array 'a' declared here}}
+};
+void test0(t0 *s2) {
+  s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the array (which contains 10 elements)}}
+}
+
+
+// Under -fstrict-flex-arrays=2 `a` is not a flexible array, but it is under -fstrict-flex-arrays=1
 struct t1 {
   int f;
-  int a[1]; // expected-note {{array 'a' declared here}}
+  int a[1]; // strict-note {{array 'a' declared here}}
 };
 void test1(t1 *s2) {
-  s2->a[2] = 0; // expected-warning {{array index 2 is past the end of the array (which contains 1 element)}}
+  s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array (which contains 1 element)}}
 }
 
-// Under -fstrict-flex-arrays `a` is a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is a flexible array.
 struct t2 {
   int f;
   int a[0];
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15852,7 +15852,7 @@
   if (StrictFlexArraysLevel >= 2 && Size != 0)
 return false;
 
-  if (StrictFlexArraysLevel == 1 && Size.ule(1))
+  if (StrictFlexArraysLevel == 1 && Size.uge(2))
 return false;
 
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132874: [clang] Don't emit debug vtable information for consteval functions

[Luke Nihlen via Phabricator via cfe-commits]

luken-google marked an inline comment as done.
luken-google added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1761
 
-  if (Method->isVirtual()) {
+  if (Method->isVirtual() && !Method->isConsteval()) {
 if (Method->isPure())

shafik wrote:
> It is not clear to me if this is the core issue or not. Can you explain this 
> a little better.
Thanks for the feedback!

Sure, here's what I'm thinking. This code calls 
ItaniumVTableContext::getMethodVTableIndex(GlobalDecl) on line 1771 below. That 
method asserts in VTableBuilder.cpp:2281 because it tries to look up a vtable 
entry for the consteval method and fails to do so. Any consteval methods are 
prevented from gaining vtable entries by the logic in 
VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD):

```
bool VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD) {
  return MD->isVirtual() && !MD->isConsteval();
}
```

This call is peppered throughout the VTableBuilder code, but the relevant call 
for our purposes is in ItaniumVTableBuilder::AddMethods() in 
VTableBuilder.cpp:1492, which causes any consteval method to be skipped when 
the code is iterating through all virtual member functions and adding them to 
the vtable.

My fix mirrors the logic in VTableContextBase::hasVtableSlot here, because the 
code assumes that if the method is virtual it has a vtable index, which 
consteval functions don't. Writing this, I've convinced myself it's better to 
just call that method directly, so I've refactored the code to avoid the 
duplicate logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132874

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

[PATCH] D132874: [clang] Don't emit debug vtable information for consteval functions

[Luke Nihlen via Phabricator via cfe-commits]

luken-google updated this revision to Diff 456641.
luken-google added a comment.

Change to method call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132874

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-linux-gnu 
-std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-obj -debug-info-kind=constructor -std=c++20 %s -o -
 
 namespace PR50787 {
 // This code would previously cause a crash.
@@ -71,3 +72,23 @@
   return function(Item{'a'}, Item{'a'});
 }
 } // namespace Issue58871
+
+namespace Issue55065 {
+struct Base {
+  consteval virtual int Get() const = 0;
+};
+
+struct Derived : Base {
+  consteval int Get() const override {
+return 42;
+  }
+};
+
+int foo() {
+  constexpr Derived a;
+
+  auto val = a.Get();
+  return val;
+}
+} // namespace Issue55065
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -26,6 +26,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1758,7 +1759,7 @@
   llvm::DISubprogram::DISPFlags SPFlags = llvm::DISubprogram::SPFlagZero;
   int ThisAdjustment = 0;
 
-  if (Method->isVirtual()) {
+  if (VTableContextBase::hasVtableSlot(Method)) {
 if (Method->isPure())
   SPFlags |= llvm::DISubprogram::SPFlagPureVirtual;
 else
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -188,10 +188,11 @@
   and `DR1734 
`_.
 - Class member variables are now in scope when parsing a ``requires`` clause. 
Fixes
   `GH55216 `_.
-
 - Correctly set expression evaluation context as 'immediate function context' 
in
   consteval functions.
   This fixes `GH51182 `
+- Fixes an assert crash caused by looking up missing vtable information on 
``consteval``
+  virtual functions. Fixes `GH55065 
`_.
 
 
 C++2b Feature Support


Index: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
===
--- clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
+++ clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-obj -debug-info-kind=constructor -std=c++20 %s -o -
 
 namespace PR50787 {
 // This code would previously cause a crash.
@@ -71,3 +72,23 @@
   return function(Item{'a'}, Item{'a'});
 }
 } // namespace Issue58871
+
+namespace Issue55065 {
+struct Base {
+  consteval virtual int Get() const = 0;
+};
+
+struct Derived : Base {
+  consteval int Get() const override {
+return 42;
+  }
+};
+
+int foo() {
+  constexpr Derived a;
+
+  auto val = a.Get();
+  return val;
+}
+} // namespace Issue55065
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -26,6 +26,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1758,7 +1759,7 @@
   llvm::DISubprogram::DISPFlags SPFlags = llvm::DISubprogram::SPFlagZero;
   int ThisAdjustment = 0;
 
-  if (Method->isVirtual()) {
+  if (VTableContextBase::hasVtableSlot(Method)) {
 if (Method->isPure())
   SPFlags |= llvm::DISubprogram::SPFlagPureVirtual;
 else
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -188,10 +188,11 @@
   and `DR1734 `_.
 - Class member variables are now in scope when parsing a ``requires`` clause. Fixes
   `GH55216 `_.
-
 - Correctly set expression evaluation context a

[PATCH] D132821: [clang][Parse] Fix crash when emitting template diagnostic

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGef1bb11a34de: [clang][Parse] Fix crash when emitting 
template diagnostic (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D132821?vs=456389&id=456643#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132821

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/test/Parser/cxx-concept-declaration.cpp


Index: clang/test/Parser/cxx-concept-declaration.cpp
===
--- clang/test/Parser/cxx-concept-declaration.cpp
+++ clang/test/Parser/cxx-concept-declaration.cpp
@@ -1,7 +1,12 @@
 
 // Support parsing of concepts
 // Disabled for now.
-// expected-no-diagnostics
 
-// RUN:  %clang_cc1 -std=c++14 -x c++ -verify %s
-// template concept C1 = true;
+// RUN:  %clang_cc1 -std=c++20 -x c++ -verify %s
+template concept C1 = true;
+
+template
+concept C = true;
+
+template
+class C {}; //expected-error{{identifier followed by '<' indicates a 
class template specialization but 'C' refers to a concept}}
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -775,7 +775,8 @@
 def err_template_spec_syntax_non_template : Error<
   "identifier followed by '<' indicates a class template specialization but "
   "%0 %select{does not refer to a template|refers to a function template|"
-  "|refers to a variable template||refers to a concept}1">;
+  "|refers to a variable template|||"
+  "refers to a concept}1">;
 def err_id_after_template_in_nested_name_spec : Error<
   "expected template name after 'template' keyword in nested name specifier">;
 def err_unexpected_template_in_unqualified_id : Error<
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -84,6 +84,8 @@
 - Fix assert that triggers a crash during template name lookup when a type was
   incomplete but was not also a TagType. This fixes
   `Issue 57387 `_.
+- Fix a crash when emitting a concept-related diagnostic. This fixes
+  `Issue 57415 `_.
 
 
 Improvements to Clang's diagnostics


Index: clang/test/Parser/cxx-concept-declaration.cpp
===
--- clang/test/Parser/cxx-concept-declaration.cpp
+++ clang/test/Parser/cxx-concept-declaration.cpp
@@ -1,7 +1,12 @@
 
 // Support parsing of concepts
 // Disabled for now.
-// expected-no-diagnostics
 
-// RUN:  %clang_cc1 -std=c++14 -x c++ -verify %s
-// template concept C1 = true;
+// RUN:  %clang_cc1 -std=c++20 -x c++ -verify %s
+template concept C1 = true;
+
+template
+concept C = true;
+
+template
+class C {}; //expected-error{{identifier followed by '<' indicates a class template specialization but 'C' refers to a concept}}
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -775,7 +775,8 @@
 def err_template_spec_syntax_non_template : Error<
   "identifier followed by '<' indicates a class template specialization but "
   "%0 %select{does not refer to a template|refers to a function template|"
-  "|refers to a variable template||refers to a concept}1">;
+  "|refers to a variable template|||"
+  "refers to a concept}1">;
 def err_id_after_template_in_nested_name_spec : Error<
   "expected template name after 'template' keyword in nested name specifier">;
 def err_unexpected_template_in_unqualified_id : Error<
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -84,6 +84,8 @@
 - Fix assert that triggers a crash during template name lookup when a type was
   incomplete but was not also a TagType. This fixes
   `Issue 57387 `_.
+- Fix a crash when emitting a concept-related diagnostic. This fixes
+  `Issue 57415 `_.
 
 
 Improvements to Clang's diagnostics
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ef1bb11 - [clang][Parse] Fix crash when emitting template diagnostic

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-08-30T15:11:38+02:00
New Revision: ef1bb11a34de2822514878b99b442575a022a658

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

LOG: [clang][Parse] Fix crash when emitting template diagnostic

This was passing a 6 to the diagnostic engine, which the diagnostic
message didn't handle.

Add the new value to the diagnosic message, remove an unused value and
add a test.

This fixes https://github.com/llvm/llvm-project/issues/57415

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/test/Parser/cxx-concept-declaration.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 92b52b8998b4f..54060deca5c30 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -84,6 +84,8 @@ Bug Fixes
 - Fix assert that triggers a crash during template name lookup when a type was
   incomplete but was not also a TagType. This fixes
   `Issue 57387 `_.
+- Fix a crash when emitting a concept-related diagnostic. This fixes
+  `Issue 57415 `_.
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 18adb21e2be08..84504b15764b1 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -775,7 +775,8 @@ def warn_cxx14_compat_template_template_param_typename : 
Warning<
 def err_template_spec_syntax_non_template : Error<
   "identifier followed by '<' indicates a class template specialization but "
   "%0 %select{does not refer to a template|refers to a function template|"
-  "|refers to a variable template||refers to a concept}1">;
+  "|refers to a variable template|||"
+  "refers to a concept}1">;
 def err_id_after_template_in_nested_name_spec : Error<
   "expected template name after 'template' keyword in nested name specifier">;
 def err_unexpected_template_in_unqualified_id : Error<

diff  --git a/clang/test/Parser/cxx-concept-declaration.cpp 
b/clang/test/Parser/cxx-concept-declaration.cpp
index 41bf53aed36e5..9248e26b9cbeb 100644
--- a/clang/test/Parser/cxx-concept-declaration.cpp
+++ b/clang/test/Parser/cxx-concept-declaration.cpp
@@ -1,7 +1,12 @@
 
 // Support parsing of concepts
 // Disabled for now.
-// expected-no-diagnostics
 
-// RUN:  %clang_cc1 -std=c++14 -x c++ -verify %s
-// template concept C1 = true;
+// RUN:  %clang_cc1 -std=c++20 -x c++ -verify %s
+template concept C1 = true;
+
+template
+concept C = true;
+
+template
+class C {}; //expected-error{{identifier followed by '<' indicates a 
class template specialization but 'C' refers to a concept}}



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

[PATCH] D132713: [clang-tidy] Skip union-like classes in use-equals-default

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:220
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
-  // Skip unions since constructors with empty bodies behave differently
-  // in comparison with structs/classes.
+  // Skip union-like classes since constructors with empty bodies behave
+  // differently in comparison with structs/classes.





Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:225
+  anyOf(isUnion(),
+has(fieldDecl(isImplicit(), hasType(cxxRecordDecl(isUnion()));
 

Why is "isImplicit" needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132713

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

[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp:2
 // RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index 
%t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index 
-extra-arg=-std=c++17 %t
 

You shouldn't need to add an extra RUN line, check_clang_tidy already loops 
over all language standards.



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

https://reviews.llvm.org/D132654

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa updated this revision to Diff 456646.
yihanaa added a comment.

Use custom seam checking on __builtin_assume_aligned.

C++ not allow implicit cast from volatile void * to const void *, but C allowed 
and only emit a warning. This is a general case, volatile in 
__builtin_assume_aligned just use to ignore generate 'call void 
@__ubsan_handle_alignment_assumption' in CodeGen. So, I use 
GatherArgumentsForCall to handle the common case(e.g. emit diag message) except 
1st arg, we ignore the implicit cast from user-written-type to 'const void *' 
on the 1st arg, because CodeGen need user-written-type to generate correct 
TypeDescriptor (this class in compiler-rt/UBSan), then, clang will emit cast 
instruction which cast type from user-written-type to VoidPtr in CodeGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/catch-alignment-assumption-array.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c

Index: clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
===
--- clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -26,3 +26,9 @@
 void *ignore_volatiles(volatile void * x) {
   return __builtin_assume_aligned(x, 1);
 }
+
+// CHECK-LABEL: ignore_array_volatiles
+void *ignore_array_volatiles() {
+  volatile int arr[] = {1};
+  return __builtin_assume_aligned(arr, 4);
+}
Index: clang/test/CodeGen/catch-alignment-assumption-array.c
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-array.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'char *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 31, i32 35 }, {{.*}}* @[[CHAR]] }
+
+void *caller(void) {
+  char str[] = "";
+  // CHECK:   define{{.*}}
+  // CHECK-NEXT:  entry:
+  // CHECK-NEXT:%[[STR:.*]] = alloca [1 x i8], align 1
+  // CHECK-NEXT:%[[BITCAST:.*]] = bitcast [1 x i8]* %[[STR]] to i8*
+  // CHECK-NEXT:call void @llvm.memset.p0i8.i64(i8* align 1 %[[BITCAST]], i8 0, i64 1, i1 false)
+  // CHECK-NEXT:%[[ARRAYDECAY:.*]] = getelementptr inbounds [1 x i8], [1 x i8]* %[[STR]], i64 0, i64 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64
+  // CHECK-SANITIZE-NEXT:   %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 0
+  // CHECK-SANITIZE-NEXT:   %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT_DUP:.*]] = ptrtoint i8* %[[ARRAYDECAY]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT:   br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_ALIGNMENT_ASSUMPTION:[^,]+]],{{.*}} !nosanitize
+  // CHECK-SANITIZE:  [[HANDLER_ALIGNMENT_ASSUMPTION]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_alignment_assumption_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-RECOVER-NEXT:   call void @__ubsan_handle_alignment_assumption(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1, i64 0){{.*}}, !nosanitize
+  // CHECK-SANITIZE-TRAP-NEXT:  call void @llvm.ubsantrap(i8 23){{.*}}, !nosanitize
+  // CHECK-SANITIZE-UNREACHABLE-NEXT:   unreachable, !nosanitize
+  // CHECK-SANITIZE:  [[CONT]]:
+  // CHECK-

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

In D131979#3756474 , @rjmccall wrote:

> In D131979#3756067 , @yihanaa wrote:
>
>> In D131979#3756006 , @rjmccall 
>> wrote:
>>
>>> In D131979#3753358 , @yihanaa 
>>> wrote:
>>>
 In D131979#3752208 , @rjmccall 
 wrote:

> From the test case, it looks like the builtin just ignores pointers to 
> volatile types, which should be preserved by the conversions you're now 
> doing in Sema.  That is, you should be able to just check 
> `Ptr->getType()->castAs()->getPointeeType().isVolatile()`.
>
> It would be different if it ignored pointers loaded out of volatile 
> l-values or something, but that's explicitly not what it's doing.

 Thanks for your tips John, form the test case global constants and 
 compiler-rt ubsan class TypeDescriptor, it looks like pass an type 
 description to __ubsan_handle_alignment_assumption, so, we have to use 
 getSubExprAsWritten to get the origin(user written arg type) type 
 description of 1st arg, but not 'const void *', for example,   in 
 catch-alignment-assumption-builtin_assume_aligned-three-params.cpp, the 
 1st arg type description is 'char **', but if we don't use 
 getSubExprAsWritten, all the type description will be 'const void *'
>>>
>>> The version of this patch which changed the builtin to use custom 
>>> type-checking didn't need to worry about this because doing that will stop 
>>> Sema from converting to `const void *`.  You seem to have taken all of that 
>>>  code out and reverted to an earlier approach, though, and I don't see 
>>> anything in this review explaining why.
>>>
>>> Also, please don't ping for review on Monday when you submitted a new 
>>> version of the patch on Saturday.  Not only is that a very short period to 
>>> be demanding review during, but a lot of reviewers keep up with review as 
>>> part of their normal workday and cannot be expected to review over the 
>>> weekend.
>>
>> Thanks for your reply John, the patch of the previous version which will 
>> emit an error when 1st arg is volatile-qualified, we're  worried about the 
>> shift to emit an unconditional error on `volatile`. So, I'll try to separate 
>> that into its own patch and just fix the bug in this one. and I'm so sorry 
>> about have 'pin' too often, this is due to time zone, I will pay attention 
>> to it later, and thanks for your tips!
>
> Hmm.  I think you can take the previous version of the patch and either (1) 
> remove the place where it explicitly emits an error or (2) make it only emit 
> an error in C++ mode.
>
> I think the type descriptor for something that starts as an array reference 
> should probably be a pointer, i.e. you should apply array decay and then 
> derive the type from that.  That's one of the things that'll just fall out 
> naturally if you do what the previous version of the patch was doing.

Thanks for your suggestions John. As far as I know,  ‘warning: passing 
'volatile char *' to parameter of type 'const void *' discards qualifiers’ is a 
common case, so I use GatherArgumentsForCall to handle these common cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

Now that parameter checking is generic, I think it would be better to use both 
common seam checking and custom sema checking, like Diff 456142  
(https://reviews.llvm.org/differential/diff/456142/), what do you think about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki updated this revision to Diff 456662.
yusuke-kadowaki marked 9 inline comments as done.
yusuke-kadowaki added a comment.

- Fix the newline condition
- Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,106 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+  
+  verifyFormat("#include \"a.h\"  // Align these\n"
+   "\n"
+   "#include \"aa.h\" // two comments\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // But do not align this because there are two lines without comments above\n",
+   Style);
+  
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line about b\n"
+   "   long b;",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line 1 about b\n"
+"// line 2 about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line 1 about b\n"
+   "   // line 2 about b\n"
+   "   long b;",
+   Style));   
+}
+
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
 " */",
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20041,7 +20041,6 @@
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
-  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParameter

[PATCH] D129488: [Sema] Fix evaluation context of immediate functions in default arguments

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 456663.
ilya-biryukov added a comment.

- Special-case the std::source_location::current calls, delay until codegen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/SemaCXX/source_location.cpp
  clang/test/SemaCXX/source_location_consteval.cpp

Index: clang/test/SemaCXX/source_location_consteval.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/source_location_consteval.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+namespace std {
+class source_location {
+  struct __impl;
+
+public:
+  static consteval source_location current(const __impl *__p = __builtin_source_location()) noexcept {
+source_location __loc;
+__loc.__m_impl = __p;
+return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_impl ? __m_impl->_M_line : 0; }
+  constexpr unsigned int column() const noexcept { return __m_impl ? __m_impl->_M_column : 0; }
+  constexpr const char *file() const noexcept { return __m_impl ? __m_impl->_M_file_name : ""; }
+  constexpr const char *function() const noexcept { return __m_impl ? __m_impl->_M_function_name : ""; }
+
+private:
+  // Note: The type name "std::source_location::__impl", and its constituent
+  // field-names are required by __builtin_source_location().
+  struct __impl {
+const char *_M_file_name;
+const char *_M_function_name;
+unsigned _M_line;
+unsigned _M_column;
+  };
+  const __impl *__m_impl = nullptr;
+
+public:
+  using public_impl_alias = __impl;
+};
+} // namespace std
+
+using SL = std::source_location;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+constexpr SL get_sl(SL l = SL::current()) { return l; }
+SL get_sl_not_const(SL l = SL::current()) { return l; }
+
+#line 700 "CheckDefaultArg.h"
+constexpr SL l = get_sl();
+static_assert(l.line() == 700);
+static_assert(is_equal(l.file(), "CheckDefaultArg.h"));
+
+
+consteval SL get_sl_rec_consteval(SL l = get_sl()) { return l; }
+constexpr SL get_sl_rec_constexpr(SL l = get_sl()) { return l; }
+
+static_assert(get_sl_rec_constexpr().line() == __LINE__);
+static_assert(get_sl_rec_consteval().line() == __LINE__);
+
+int test() {
+  static_assert(is_equal(get_sl().function(), __PRETTY_FUNCTION__));
+  static_assert(get_sl().line() ==  __LINE__);
+  return get_sl().line() + get_sl_not_const().line();
+}
Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++20 -fcxx-exceptions -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -13,7 +13,7 @@
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
+  static consteval source_location current(const __impl *__p = __builtin_source_location()) noexcept {
 source_location __loc;
 __loc.__m_impl = __p;
 return __loc;
@@ -364,8 +364,8 @@
 template 
 void func_template_tests() {
   constexpr auto P = test_func_template(42);
-  //static_assert(is_equal(P.first.function(), __func__), "");
-  //static_assert(!is_equal(P.second.function(), __func__), "");
+  static_assert(is_equal(P.first.function(), __PRETTY_FUNCTION__), "");
+  static_assert(!is_equal(P.second.function(), __PRETTY_FUNCTION__), "");
 }
 template void func_template_tests();
 
Index: clang/test/CodeGenCXX/builtin-source-location.cpp
===
--- clang/test/CodeGenCXX/builtin-source-location.cpp
+++ clang/test/CodeGenCXX/builtin-source-location.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -no-opaque-pointers -std=c++2a -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
+// RUN: %clang_cc1 -no-opaque-pointers -std=c++20 -fblocks %s -triple x86_64-unknown-unknown -emit-llvm -o %t.ll
 
 // This needs to be performed before #line directives which alter filename
-// RUN: %clang_cc1 -no-opaque-pointers -fno-file-reproducible -fmacro-prefix-map=%p=/UNLIKELY/PATH -emit-llvm -o - %s | Fi

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:785
+  - Consecutive
+  - AcrossEmptyLines
+  - AcrossComments

MyDeveloperDay wrote:
> may be AcrossEmptyLines should be a number (to mean the number of empty lines)
We need to introduce a new struct to do that since AlignConsecutiveStyle is 
shared with some options and not possible to be changed. Plus I think more than 
two empty lines are formatted to one empty line anyway.



Comment at: clang/docs/ClangFormatStyleOptions.rst:794
+
+AlignConsecutiveMacros: AcrossEmptyLines
+

MyDeveloperDay wrote:
> Should this say `AlignedConsecutuveCommets`
No. This part is a documentation about AlignConsecutiveStyle type, which is 
appended automatically after all the options that take AlignConsecutiveStyle 
type.



Comment at: clang/docs/ClangFormatStyleOptions.rst:797
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > why do we need Enabled?
> > > 
> > > isn't it just
> > > 
> > > ```
> > > false:
> > > AcrossEmptyLines: false
> > > AcrossComments: false
> > > 
> > > true:
> > > AcrossEmptyLines: true
> > > AcrossComments: true
> > > ```
> > The struct is a bit older. And we need `Enabled`, one might wish to align 
> > only really consecutive comments, as the option right now does. (Same for 
> > macros, assignments, etc..)
> I'm still not sure I understand, plus are those use cases you describe 
> tested, I couldn't see that. 
> 
> If feels like what you are saying is that AlignConsecutiveStyle is used 
> elsewhere and it has options that don't pertain to aligning comments? 
> 
> I couldn't tell if feel like this documentation is describing something other 
> than AligningTrailingComments, if I'm confused users could be too? 
As for the Enabled option,
Enabled: true
AcrossEmptyLines: false

is supposed to work in the same way as the old style AlignTrailingComments. So 
the tests of AlignTrailingComments are the test cases.

For the documentation, I also thought it was a bit confusing when I first saw 
it because the description of the option and the style is kind of connected. 
But as I also mentioned above, this part is automatically append and it affects 
all the other options that have AlignConsecutiveStyle if we change. So I think 
it should be another patch even if we are to modify it.



Comment at: clang/lib/Format/Format.cpp:649
 IO.mapOptional("AlignOperands", Style.AlignOperands);
-IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
 IO.mapOptional("AllowAllArgumentsOnNextLine",

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > you can't remove an option, otherwise you'll break everyones 
> > > > .clang-format
> > > That's not correct. We have done it:
> > > D108752 -> D108882 -> D127390
> > > 
> > > You can remove (and in this case should), but you still need to parse it 
> > > and act accordingly. Which is done as far as I can see.
> > sorry thats what I meant, you can remove it, but you have to make it turn 
> > on the correct new style that keeps exactly the old user case, and you 
> > can't remove it from the configuration parsing otherwise anyone who has it 
> > in their .clang-format is immediately broken with an unknown option.
> > 
> > to be honest this is an annoyance for introducing new features, which at 
> > some point I'd like to drop, you can't have a new option which is not read
> > 
> > For me, when we introduced new languages, or new features like InsertBraces 
> > etc.. I want to put it in my .clang-format even though other people they 
> > aren't using a version that uses it. (becuase it won't impact them), i.e. 
> > it won't add braces or correct QualifierOrder that doesn't bother me
> > 
> Do you disagree that it actually is parsed?
> 
> But that compatibility parsing has nothing to do with ignoring unknown (new) 
> options.
I confirmed the old style AlignTrailingComments works and it's also tested with 
CHECK_PARSE if I understand correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D132941: [DO NOT SUBMIT] [Sema] Delay evaluation of std::source_location::current() in default arguments

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

They must be evaluated in the context where default argument is actually
used during a call, not in a parameter list where default argument is specified.

Fixes #56379.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132941

Files:
  clang/include/clang/AST/CurrentSourceLocExprScope.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/SemaCXX/source_location.cpp
  clang/test/SemaCXX/source_location_consteval.cpp

Index: clang/test/SemaCXX/source_location_consteval.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/source_location_consteval.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+namespace std {
+class source_location {
+  struct __impl;
+
+public:
+  static consteval source_location current(const __impl *__p = __builtin_source_location()) noexcept {
+source_location __loc;
+__loc.__m_impl = __p;
+return __loc;
+  }
+  constexpr source_location() = default;
+  constexpr source_location(source_location const &) = default;
+  constexpr unsigned int line() const noexcept { return __m_impl ? __m_impl->_M_line : 0; }
+  constexpr unsigned int column() const noexcept { return __m_impl ? __m_impl->_M_column : 0; }
+  constexpr const char *file() const noexcept { return __m_impl ? __m_impl->_M_file_name : ""; }
+  constexpr const char *function() const noexcept { return __m_impl ? __m_impl->_M_function_name : ""; }
+
+private:
+  // Note: The type name "std::source_location::__impl", and its constituent
+  // field-names are required by __builtin_source_location().
+  struct __impl {
+const char *_M_file_name;
+const char *_M_function_name;
+unsigned _M_line;
+unsigned _M_column;
+  };
+  const __impl *__m_impl = nullptr;
+
+public:
+  using public_impl_alias = __impl;
+};
+} // namespace std
+
+using SL = std::source_location;
+
+constexpr bool is_equal(const char *LHS, const char *RHS) {
+  while (*LHS != 0 && *RHS != 0) {
+if (*LHS != *RHS)
+  return false;
+++LHS;
+++RHS;
+  }
+  return *LHS == 0 && *RHS == 0;
+}
+
+constexpr SL get_sl(SL l = SL::current()) { return l; }
+SL get_sl_not_const(SL l = SL::current()) { return l; }
+
+#line 700 "CheckDefaultArg.h"
+constexpr SL l = get_sl();
+static_assert(l.line() == 700);
+static_assert(is_equal(l.file(), "CheckDefaultArg.h"));
+
+int test() {
+  static_assert(is_equal(get_sl().function(), __PRETTY_FUNCTION__));
+  static_assert(get_sl().line() ==  __LINE__);
+  return get_sl().line() + get_sl_not_const().line();
+}
Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -364,8 +364,8 @@
 template 
 void func_template_tests() {
   constexpr auto P = test_func_template(42);
-  //static_assert(is_equal(P.first.function(), __func__), "");
-  //static_assert(!is_equal(P.second.function(), __func__), "");
+  static_assert(is_equal(P.first.function(), __PRETTY_FUNCTION__), "");
+  static_assert(!is_equal(P.second.function(), __PRETTY_FUNCTION__), "");
 }
 template void func_template_tests();
 
Index: clang/test/CodeGenCXX/builtin-source-location.cpp
===
--- clang/test/CodeGenCXX/builtin-source-location.cpp
+++ clang/test/CodeGenCXX/builtin-source-location.cpp
@@ -13,12 +13,12 @@
 namespace std {
 class source_location {
 public:
-  static constexpr source_location current(const void *__p = __builtin_source_location()) noexcept {
+  static consteval source_location current(const void *__p = __builtin_source_location()) noexcept {
 source_location __loc;
 __loc.__m_impl = static_cast(__p);
 return __loc;
   }
-  static source_location bad_current(const void *__p = __builtin_source_location()) noexcept {
+  static constexpr source_location bad_current(const void *__p = __builtin_source_location()) noexcept {
 return current(__p);
   }
   constexpr source_location() = default;
Index: clang/lib/Serialization/ASTWrit

[PATCH] D129488: [Sema] Fix evaluation context of immediate functions in default arguments

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: aaron.ballman.
ilya-biryukov added a subscriber: aaron.ballman.
ilya-biryukov added a comment.

This seems to mostly work and ready for another round of review, but I still 
need to update the codegen test, it seems it has caught an error with 
default-initilization of globals.
For reference, I have also tried forcing evaluation of relevant calls during 
semantic analysis, but did not finish it. See my attempt here if interested: 
https://reviews.llvm.org/D132941.

Current design of Clang does not seem to be friendly to this kind of change as 
this involves having different expressions inside `CXXDefaultArgExpr` even if 
the underlying parameter is the same.

@aaron.ballman could you take a look at this and share your thoughts too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[clang] fafa48e - [AIX][clang][driver] Check the command string to the linker for exportlist opts

[via cfe-commits]

Author: zhijian
Date: 2022-08-30T10:38:38-04:00
New Revision: fafa48e7b51899f0fda80b0962679d57a1f58169

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

LOG: [AIX][clang][driver] Check the command string to the linker for exportlist 
opts
Summary:
Some of code in the patch are contributed by David Tenty.

1.  We currently only check driver Wl options and don't check for the plain -b, 
-Xlinker or other options which get passed through to the linker when we decide 
whether to run llvm-nm --export-symbols, so we may run it in situations where 
we wouldn't if the user had used the equivalent -Wl, prefixed options. If we 
run the export list utility when the user has specified an export list, we 
could export more symbols than they intended.
2.  Add a new functionality to allow redirecting the stdin, stdout, stderr of 
individual Jobs, if redirects are set for the Job use them, otherwise fall back 
to the global Compilation redirects if any.

Reviewers: David Tenty, Fangrui Song, Steven Wan
Differential Revision: https://reviews.llvm.org/D119147

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Driver/Job.h
clang/lib/Driver/Job.cpp
clang/lib/Driver/ToolChains/AIX.cpp
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 54060deca5c30..132e282797053 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,6 +159,9 @@ Windows Support
 
 AIX Support
 ---
+* When using `-shared`, the clang driver now invokes llvm-nm to create an
+  export list if the user doesn't specify one via linker flag or pass an
+  alternative export control option.
 
 C Language Changes in Clang
 ---

diff  --git a/clang/include/clang/Driver/Job.h 
b/clang/include/clang/Driver/Job.h
index ae9337f3c2d0a..96cdd6fd957aa 100644
--- a/clang/include/clang/Driver/Job.h
+++ b/clang/include/clang/Driver/Job.h
@@ -141,6 +141,9 @@ class Command {
   /// See Command::setEnvironment
   std::vector Environment;
 
+  /// Optional redirection for stdin, stdout, stderr.
+  std::vector> RedirectFiles;
+
   /// Information on executable run provided by OS.
   mutable Optional ProcStat;
 
@@ -204,6 +207,8 @@ class Command {
   /// from the parent process will be used.
   virtual void setEnvironment(llvm::ArrayRef NewEnvironment);
 
+  void setRedirectFiles(const std::vector> &Redirects);
+
   void replaceArguments(llvm::opt::ArgStringList List) {
 Arguments = std::move(List);
   }

diff  --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp
index f63763effaffe..c34ae13a5fc52 100644
--- a/clang/lib/Driver/Job.cpp
+++ b/clang/lib/Driver/Job.cpp
@@ -301,6 +301,11 @@ void Command::setEnvironment(llvm::ArrayRef 
NewEnvironment) {
   Environment.push_back(nullptr);
 }
 
+void Command::setRedirectFiles(
+const std::vector> &Redirects) {
+  RedirectFiles = Redirects;
+}
+
 void Command::PrintFileNames() const {
   if (PrintInputFilenames) {
 for (const auto &Arg : InputInfoList)
@@ -352,6 +357,22 @@ int Command::Execute(ArrayRef> 
Redirects,
   }
 
   auto Args = llvm::toStringRefArray(Argv.data());
+
+  // Use Job-specific redirect files if they are present.
+  if (!RedirectFiles.empty()) {
+std::vector> RedirectFilesOptional;
+for (const auto &Ele : RedirectFiles)
+  if (Ele)
+RedirectFilesOptional.push_back(Optional(*Ele));
+  else
+RedirectFilesOptional.push_back(None);
+
+return llvm::sys::ExecuteAndWait(Executable, Args, Env,
+ makeArrayRef(RedirectFilesOptional),
+ /*secondsToWait=*/0, /*memoryLimit=*/0,
+ ErrMsg, ExecutionFailed, &ProcStat);
+  }
+
   return llvm::sys::ExecuteAndWait(Executable, Args, Env, Redirects,
/*secondsToWait*/ 0, /*memoryLimit*/ 0,
ErrMsg, ExecutionFailed, &ProcStat);

diff  --git a/clang/lib/Driver/ToolChains/AIX.cpp 
b/clang/lib/Driver/ToolChains/AIX.cpp
index 64be5fe23558f..653fbeaffbd4e 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -74,6 +74,29 @@ void aix::Assembler::ConstructJob(Compilation &C, const 
JobAction &JA,
  Exec, CmdArgs, Inputs, Output));
 }
 
+// Determine whether there are any linker options that supply an export list
+// (or equivalent information about what to export) being sent to the linker.
+static bool hasExportListLinkerOpts(const ArgStringList &CmdArgs) {
+  for (size_t i = 0, Size = CmdArgs.size(); i < Size; ++i) {
+llvm::StringRef ArgString(CmdArgs[i]);
+
+if (ArgString.startswith("-bE:") 

[PATCH] D119147: [AIX][clang][driver] Check the command string to the linker for exportlist opts

[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 rGfafa48e7b518: [AIX][clang][driver] Check the command string 
to the linker for exportlist opts (authored by zhijian 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119147

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -596,17 +596,29 @@
 // RUN:   | FileCheck --check-prefix=CHECK-LD-LIBSTDCXX %s
 // CHECK-LD-LIBSTDCXX: LLVM ERROR: linking libstdc++ unimplemented on AIX
 
-// Check powerpc64-ibm-aix7.1.0.0, 32-bit. -shared.
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared.
 // RUN: %clangxx -x c++ %s 2>&1 -### \
-// RUN:-resource-dir=%S/Inputs/resource_dir \
-// RUN:-shared \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared (with exp option strings in other opt).
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
 // RUN:--target=powerpc-ibm-aix7.1.0.0 \
 // RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:--unwindlib=libunwind \
+// RUN:-Wl,-Z/expall/expfull/a-bE:/a-bexport:/ \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED %s
+
 // CHECK-LD32-SHARED: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
 // CHECK-LD32-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD32-SHARED: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-SHARED: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED: "--export-symbols"
+// CHECK-LD32-SHARED: "-X" "32"
 // CHECK-LD32-SHARED: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-SHARED: "-bM:SRE"
 // CHECK-LD32-SHARED: "-bnoentry"
@@ -623,10 +635,53 @@
 // CHECK-LD32-SHARED: "-lm"
 // CHECK-LD32-SHARED: "-lc"
 
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared with export list.
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-Wl,-bE:input.exp \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED-EXPORTS %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared with export list (no -Wl, variant).
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-bE:input.exp \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED-EXPORTS %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared with export list (-Xlinker variant).
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-Xlinker -bE:input.exp \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED-EXPORTS %s
+
+// CHECK-LD32-SHARED-EXPORTS: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-SHARED-EXPORTS: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LD32-SHARED-EXPORTS: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "32"
+// CHECK-LD32-SHARED-EXPORTS: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-SHARED-EXPORTS: "-bM:SRE"
+// CHECK-LD32-SHARED-EXPORTS: "-bnoentry"
+// CHECK-LD32-SHARED-EXPORTS: "-b32"
+// CHECK-LD32-SHARED-EXPORTS: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-LD32-SHARED-EXPORTS: "-b{{(" ")?}}E:input.exp"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-bE:{{[^"]+}}"
+// CHECK-LD32-SHARED-EXPORTS: "-lc++"
+// CHECK-LD32-SHARED-EXPORTS: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-SHARED-EXPORTS: "-lm"
+// CHECK-LD32-SHARED-EXPORTS: "-lc"
+
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. -shared.
 // RUN: %clangxx -x c++ %s 2>&1 -### \
-// RUN:-resource-dir=%S/Inputs/resource_dir \
-// RUN:-shared \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
 // RUN:--target=powerpc64-ibm-aix7.1.0.0 \
 // RUN:--sysro

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I'm confused about this new strategy of special-casing 
`source_location::current()`. Isn't it wrong to eagerly evaluate _other_ calls 
in default args, as well? ISTM that source_location is simply _exposing_ the 
bug in where we evaluate these expressions, but that it's actually a more 
general problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[PATCH] D132944: [clang] cleanup -fstrict-flex-arrays implementation

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: jyknight.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a follow up to https://reviews.llvm.org/D126864, addressing some 
remaining
comments. No functional change intended.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132944

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/object-size-flex-array.c

Index: clang/test/CodeGen/object-size-flex-array.c
===
--- clang/test/CodeGen/object-size-flex-array.c
+++ clang/test/CodeGen/object-size-flex-array.c
@@ -24,7 +24,7 @@
   double c[2];
 } foo2_t;
 
-// CHECK-LABEL: @bar
+// CHECK-LABEL: @bar(
 unsigned bar(foo_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 %
@@ -32,7 +32,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @bar0
+// CHECK-LABEL: @bar0(
 unsigned bar0(foo0_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 %
@@ -40,7 +40,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @bar1
+// CHECK-LABEL: @bar1(
 unsigned bar1(foo1_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 %
@@ -48,7 +48,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @bar2
+// CHECK-LABEL: @bar2(
 unsigned bar2(foo2_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 16
@@ -73,7 +73,7 @@
   float f;
 } foofoo2_t;
 
-// CHECK-LABEL: @babar0
+// CHECK-LABEL: @babar0(
 unsigned babar0(foofoo0_t *f) {
   // CHECK-STRICT-0: ret i32 0
   // CHECK-STRICT-1: ret i32 0
@@ -81,7 +81,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @babar1
+// CHECK-LABEL: @babar1(
 unsigned babar1(foofoo1_t *f) {
   // CHECK-STRICT-0: ret i32 8
   // CHECK-STRICT-1: ret i32 8
@@ -89,7 +89,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @babar2
+// CHECK-LABEL: @babar2(
 unsigned babar2(foofoo2_t *f) {
   // CHECK-STRICT-0: ret i32 16
   // CHECK-STRICT-1: ret i32 16
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15849,16 +15849,10 @@
   if (!ND)
 return false;
 
-  if (StrictFlexArraysLevel >= 2 && Size != 0)
-return false;
-
-  if (StrictFlexArraysLevel == 1 && Size.uge(2))
-return false;
-
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
   // arrays to be treated as flexible-array-members, we still emit diagnostics
   // as if they are not. Pending further discussion...
-  if (StrictFlexArraysLevel == 0 && Size != 1)
+  if (StrictFlexArraysLevel >= 2 || Size.uge(2))
 return false;
 
   const FieldDecl *FD = dyn_cast(ND);
@@ -16026,8 +16020,8 @@
 if (BaseType->isIncompleteType())
   return;
 
-// FIXME: this check should belong to the IsTailPaddedMemberArray call
-// below.
+// FIXME: this check should be used to set IsUnboundedArray from the
+// beginning.
 llvm::APInt size = ArrayTy->getSize();
 if (!size.isStrictlyPositive())
   return;
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -906,10 +906,8 @@
 if (const auto *FD = dyn_cast(ME->getMemberDecl())) {
   // FIXME: Sema doesn't treat a T[1] union member as a flexible array
   // member, only a T[0] or T[] member gets that treatment.
-  // Under StrictFlexArraysLevel, obey c99+ that disallows FAM in union, see
-  // C11 6.7.2.1 §18
   if (FD->getParent()->isUnion())
-return StrictFlexArraysLevel < 2;
+return true;
   RecordDecl::field_iterator FI(
   DeclContext::decl_iterator(const_cast(FD)));
   return ++FI == FD->getParent()->field_end();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1147,7 +1147,7 @@
 def fapple_kext : Flag<["-"], "fapple-kext">, Group, Flags<[CC1Option]>,
   HelpText<"Use Apple's kernel extensions ABI">,
   MarshallingInfoFlag>;
-def fstrict_flex_arrays_EQ : Joined<["-"], "fstrict-flex-arrays=">,Group,
+def fstrict_flex_arrays_EQ : Joined<["-"], "fstrict-flex-arrays=">, Group,
   MetaVarName<"">, Values<"0,1,2">,
   LangOpts<"StrictFlexArrays">,
   Flags<[CC1Option]>,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan updated this revision to Diff 456674.
egorzhdan added a comment.

- Add tests for conversion of doc comments to HTML/XML
- Fix a bug in `StringRef` -> `CXString` conversion logic for an empty

string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

Files:
  clang/lib/AST/CommentLexer.cpp
  clang/test/Index/comment-to-html-xml-conversion.cpp
  clang/test/Sema/warn-documentation.cpp
  clang/tools/libclang/CXString.cpp

Index: clang/tools/libclang/CXString.cpp
===
--- clang/tools/libclang/CXString.cpp
+++ clang/tools/libclang/CXString.cpp
@@ -78,13 +78,19 @@
 }
 
 CXString createRef(StringRef String) {
+  // If the string is empty, it might point to a position in another string
+  // while having zero length. Make sure we don't create a reference to the
+  // larger string.
+  if (String.empty())
+return createEmpty();
+
   // If the string is not nul-terminated, we have to make a copy.
 
   // FIXME: This is doing a one past end read, and should be removed! For memory
   // we don't manage, the API string can become unterminated at any time outside
   // our control.
 
-  if (!String.empty() && String.data()[String.size()] != 0)
+  if (String.data()[String.size()] != 0)
 return createDup(String);
 
   CXString Result;
Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -62,6 +62,21 @@
 /// 
 int test_html11(int);
 
+/// Aaa bbb
+int test_html12(int);
+
+/// Aaa bbb
+int test_html13(int);
+
+/// Aaa bbb
+int test_html14(int);
+
+/// Aaa bbb
+int test_html15(int);
+
+/// Aaa bbb
+int test_html16(int);
+
 /// Meow
 int test_html_nesting1(int);
 
Index: clang/test/Index/comment-to-html-xml-conversion.cpp
===
--- clang/test/Index/comment-to-html-xml-conversion.cpp
+++ clang/test/Index/comment-to-html-xml-conversion.cpp
@@ -744,6 +744,65 @@
 // CHECK-NEXT: (CXComment_Text Text=[ ] IsWhitespace)
 // CHECK-NEXT: (CXComment_InlineCommand CommandName=[anchor] RenderAnchor Arg[0]=A)))]
 
+/// Aaa bbb
+void comment_to_html_conversion_38();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_38:{{.*}} FullCommentAsHTML=[ Aaa bbb] FullCommentAsXML=[comment_to_html_conversion_38c:@F@comment_to_html_conversion_38#void comment_to_html_conversion_38() Aaa bbb]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa bbb])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
+/// Aaa ccc
+void comment_to_html_conversion_39();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_39:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_39c:@F@comment_to_html_conversion_39#void comment_to_html_conversion_39() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
+/// Aaa ccc
+void comment_to_html_conversion_40();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_40:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_40c:@F@comment_to_html_conversion_40#void comment_to_html_conversion_40() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=)
+
+/// Aaa ccc
+void comment_to_html_conversion_41();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_41:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_41c:@F@comment_to_html_conversion_41#void comment_to_html_conversion_41() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=path)
+
+/// Aaa ccc
+void comment_to_html_conversion_42();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_42:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_42c:@F@comment_to_html_conversion_42#void comment_to_html_conversion_42() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/warn-documentation.cpp:78
+/// Aaa bbb
+int test_html_img4(int);
+

gribozavr2 wrote:
> Could you also add these tests to 
> clang/test/Index/comment-to-html-xml-conversion.cpp to show that the whole 
> tag is captured correctly? (for example, the ">" is correctly captured as a 
> part of the tag, not as plain text)
Thanks, added a few tests there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

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

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9436
+CGM.getModule(), Type, true,
+llvm::GlobalValue::LinkageTypes::LinkOnceODRLinkage,
+llvm::ConstantInt::get(Type, Value), Name, nullptr,

jhuber6 wrote:
> yaxunl wrote:
> > yaxunl wrote:
> > > jhuber6 wrote:
> > > > yaxunl wrote:
> > > > > This does not support per-TU control variables. Probably should use 
> > > > > internal linkage.
> > > > The AMDGPU device libraries use `linkone_odr` so I figured it was the 
> > > > most appropriate here. It should mean that we can have multiple 
> > > > identical definitions and they don't clash. There's also no requirement 
> > > > for these to be emitted as symbols AFAIK.
> > > > The AMDGPU device libraries use `linkone_odr` so I figured it was the 
> > > > most appropriate here. It should mean that we can have multiple 
> > > > identical definitions and they don't clash. There's also no requirement 
> > > > for these to be emitted as symbols AFAIK.
> > > 
> > > clang uses  -mlink-builtin-bitcode to link these device libraries for HIP 
> > > and OpenCL. Then the linkage of these variables becomes internal linkage. 
> > > That's why it works for per-TU control.
> > > > The AMDGPU device libraries use `linkone_odr` so I figured it was the 
> > > > most appropriate here. It should mean that we can have multiple 
> > > > identical definitions and they don't clash. There's also no requirement 
> > > > for these to be emitted as symbols AFAIK.
> > > 
> > > clang uses  -mlink-builtin-bitcode to link these device libraries for HIP 
> > > and OpenCL. Then the linkage of these variables becomes internal linkage. 
> > > That's why it works for per-TU control.
> > 
> > You may let HIP and OpenCL use internal linkage and C/C++/OpenMP use 
> > linkonce_odr since only HIP and OpenCL toolchain use -mlink-builtin-bitcode 
> > to link these device libraries 
> I see, `linkonce_odr` implies that these should all have the same value which 
> isn't necessarily true after linking. I'll change it to use private linkage.
> 
> OpenMP right now links everything late which means that we don't allow these 
> to be defined differently per-TU. This may be incorrect given this new method 
> as each TU will have different things set. I can change OpenMP to use the 
> `mlink` method after this patch which may be more strictly correct.
> I see, `linkonce_odr` implies that these should all have the same value which 
> isn't necessarily true after linking. I'll change it to use private linkage.
> 
> OpenMP right now links everything late which means that we don't allow these 
> to be defined differently per-TU. This may be incorrect given this new method 
> as each TU will have different things set. I can change OpenMP to use the 
> `mlink` method after this patch which may be more strictly correct.

On second thoughts, the idea for letting clang to emit these control variables 
might not work for HIP and OpenCL. The reason is that to support per-TU control 
variables, these variables need to be internal or private linkage, however, 
that means they cannot be used by other device library functions which are 
expecting non-internal linkage for them. Those device library functions will 
end up using control variables from device library bitcode any way.

For OpenMP, it may be necessary to emit them as linkonce_odr, otherwise device 
library functions may not find them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130096

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

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

Fixes: https://github.com/llvm/llvm-project/issues/56183
Fixes: https://github.com/llvm/llvm-project/issues/51695
Fixes: https://github.com/llvm/llvm-project/issues/50455


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132945

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -939,3 +939,63 @@
 static_assert(max(1,2)==2);
 static_assert(mid(1,2,3)==2);
 } // namespace GH51182
+
+// https://github.com/llvm/llvm-project/issues/56183
+namespace GH56183 {
+consteval auto Foo(auto c) { return c; }
+consteval auto Bar(auto f) { return f(); }
+void test() {
+  constexpr auto x = Foo(Bar([] { return 'a'; }));
+  static_assert(x == 'a');
+}
+}  // namespace GH56183
+
+// https://github.com/llvm/llvm-project/issues/51695
+namespace GH51695 {
+// Original 
+template 
+struct type_t {};
+
+template 
+struct list_t {};
+
+template 
+consteval auto pop_front(list_t) -> auto {
+  return list_t{};
+}
+
+template 
+consteval auto apply(list_t, F fn) -> auto {
+  return fn(type_t{}...);
+}
+
+void test1() {
+  constexpr auto x = apply(pop_front(list_t{}),
+[](type_t...) { return 42; });
+  static_assert(x == 42);
+}
+// Reduced 
+consteval bool zero() { return false; }
+
+template 
+consteval bool foo(bool, F f) {
+  return f();
+}
+
+void test2() {
+  constexpr auto x = foo(zero(), []() { return true; });
+  static_assert(x);
+}
+}  // namespace GH51695
+
+// https://github.com/llvm/llvm-project/issues/50455
+namespace GH50455 {
+void f() {
+  []() consteval { int i{}; }();
+  []() consteval { int i{}; ++i; }();
+}
+void g() {
+  (void)[](int i) consteval { return i; }(0);
+  (void)[](int i) consteval { return i; }(0);
+}
+}  // namespace GH50455
\ No newline at end of file
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17598,6 +17598,7 @@
   DRSet.erase(E);
   return E;
 }
+ExprResult TransformLambdaExpr(LambdaExpr *E) { return E; }
 bool AlwaysRebuild() { return false; }
 bool ReplacingOriginal() { return true; }
 bool AllowSkippingCXXConstructExpr() {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -191,8 +191,12 @@
 
 - Correctly set expression evaluation context as 'immediate function context' 
in
   consteval functions.
-  This fixes `GH51182 `
+  This fixes `GH51182 `_.
 
+- Skip re-building lambda expressions when they appear as parameters to an 
immediate invocation.
+  This fixes `GH56183 `_,
+  `GH51695 `_,
+  `GH50455 `_.
 
 C++2b Feature Support
 ^


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -939,3 +939,63 @@
 static_assert(max(1,2)==2);
 static_assert(mid(1,2,3)==2);
 } // namespace GH51182
+
+// https://github.com/llvm/llvm-project/issues/56183
+namespace GH56183 {
+consteval auto Foo(auto c) { return c; }
+consteval auto Bar(auto f) { return f(); }
+void test() {
+  constexpr auto x = Foo(Bar([] { return 'a'; }));
+  static_assert(x == 'a');
+}
+}  // namespace GH56183
+
+// https://github.com/llvm/llvm-project/issues/51695
+namespace GH51695 {
+// Original 
+template 
+struct type_t {};
+
+template 
+struct list_t {};
+
+template 
+consteval auto pop_front(list_t) -> auto {
+  return list_t{};
+}
+
+template 
+consteval auto apply(list_t, F fn) -> auto {
+  return fn(type_t{}...);
+}
+
+void test1() {
+  constexpr auto x = apply(pop_front(list_t{}),
+[](type_t...) { return 42; });
+  static_assert(x == 42);
+}
+// Reduced 
+consteval bool zero() { return false; }
+
+template 
+consteval bool foo(bool, F f) {
+  return f();
+}
+
+void test2() {
+  constexpr auto x = foo(zero(), []() { return true; });
+  static_assert(x);
+}
+}  // namespace GH51695
+
+// https://github.com/llvm/llvm-project/issues/50455
+namespace GH50455 {
+void f() {
+  []() co

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9436
+CGM.getModule(), Type, true,
+llvm::GlobalValue::LinkageTypes::LinkOnceODRLinkage,
+llvm::ConstantInt::get(Type, Value), Name, nullptr,

yaxunl wrote:
> jhuber6 wrote:
> > yaxunl wrote:
> > > yaxunl wrote:
> > > > jhuber6 wrote:
> > > > > yaxunl wrote:
> > > > > > This does not support per-TU control variables. Probably should use 
> > > > > > internal linkage.
> > > > > The AMDGPU device libraries use `linkone_odr` so I figured it was the 
> > > > > most appropriate here. It should mean that we can have multiple 
> > > > > identical definitions and they don't clash. There's also no 
> > > > > requirement for these to be emitted as symbols AFAIK.
> > > > > The AMDGPU device libraries use `linkone_odr` so I figured it was the 
> > > > > most appropriate here. It should mean that we can have multiple 
> > > > > identical definitions and they don't clash. There's also no 
> > > > > requirement for these to be emitted as symbols AFAIK.
> > > > 
> > > > clang uses  -mlink-builtin-bitcode to link these device libraries for 
> > > > HIP and OpenCL. Then the linkage of these variables becomes internal 
> > > > linkage. That's why it works for per-TU control.
> > > > > The AMDGPU device libraries use `linkone_odr` so I figured it was the 
> > > > > most appropriate here. It should mean that we can have multiple 
> > > > > identical definitions and they don't clash. There's also no 
> > > > > requirement for these to be emitted as symbols AFAIK.
> > > > 
> > > > clang uses  -mlink-builtin-bitcode to link these device libraries for 
> > > > HIP and OpenCL. Then the linkage of these variables becomes internal 
> > > > linkage. That's why it works for per-TU control.
> > > 
> > > You may let HIP and OpenCL use internal linkage and C/C++/OpenMP use 
> > > linkonce_odr since only HIP and OpenCL toolchain use 
> > > -mlink-builtin-bitcode to link these device libraries 
> > I see, `linkonce_odr` implies that these should all have the same value 
> > which isn't necessarily true after linking. I'll change it to use private 
> > linkage.
> > 
> > OpenMP right now links everything late which means that we don't allow 
> > these to be defined differently per-TU. This may be incorrect given this 
> > new method as each TU will have different things set. I can change OpenMP 
> > to use the `mlink` method after this patch which may be more strictly 
> > correct.
> > I see, `linkonce_odr` implies that these should all have the same value 
> > which isn't necessarily true after linking. I'll change it to use private 
> > linkage.
> > 
> > OpenMP right now links everything late which means that we don't allow 
> > these to be defined differently per-TU. This may be incorrect given this 
> > new method as each TU will have different things set. I can change OpenMP 
> > to use the `mlink` method after this patch which may be more strictly 
> > correct.
> 
> On second thoughts, the idea for letting clang to emit these control 
> variables might not work for HIP and OpenCL. The reason is that to support 
> per-TU control variables, these variables need to be internal or private 
> linkage, however, that means they cannot be used by other device library 
> functions which are expecting non-internal linkage for them. Those device 
> library functions will end up using control variables from device library 
> bitcode any way.
> 
> For OpenMP, it may be necessary to emit them as linkonce_odr, otherwise 
> device library functions may not find them.
> On second thoughts, the idea for letting clang to emit these control 
> variables might not work for HIP and OpenCL. The reason is that to support 
> per-TU control variables, these variables need to be internal or private 
> linkage, however, that means they cannot be used by other device library 
> functions which are expecting non-internal linkage for them. Those device 
> library functions will end up using control variables from device library 
> bitcode any way.

Right now we include each file per-TU using `-mlink-builtin-bitcode` which 
converts `linkonce_odr` to `private` linkage. Shouldn't this be equivalent? It 
may be possible to make some test showing a user of these constants to verify 
they get picked up correctly. If you're worried about these getting removed we 
may be able to stash them in `compiler.used`, that shouldn't impede the 
necessary constant propagation.

Side note, OpenCL seems to optimize these out without `-disable-llvm-optzns` 
while HIP will not. Does OpenCL use some mandatory passes to ensure that these 
control variables get handled? This method of using control constants in 
general is somewhat problematic as it hides invalid code behind some mandatory 
CP and DCE passes. For OpenMP right now we just generate one version for each 
architecture, which is wasteful but somewhat easier to work with.
 


Repository:
  rG LLVM

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 456682.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
  clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
  clang/test/SemaTemplate/pack-deduction.cpp

Index: clang/test/SemaTemplate/pack-deduction.cpp
===
--- clang/test/SemaTemplate/pack-deduction.cpp
+++ clang/test/SemaTemplate/pack-deduction.cpp
@@ -134,14 +134,14 @@
   template struct tuple {};
   template struct A {
 template static pair, tuple> f(pair ...p);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 // expected-note@-2 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (at least 3 vs. 2) from outer parameter packs}}
 
 template static pair, tuple> g(pair ...p, ...);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 
 template static tuple h(tuple..., pair>);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 1) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 1)}}
   };
 
   pair, tuple> k1 = A().f(pair(), pair());
Index: clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
===
--- clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -97,7 +97,7 @@
   template  struct Constant {};
 
   template  struct Sum {
-template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter pack 'Js' that has a different length (1 vs. 2) from outer parameter packs}}
+template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter packs 'Is' and 'Js' that have different lengths (1 vs. 2)}}
   };
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -473,7 +473,7 @@
 namespace pr56094 {
 template  struct D {
   template  using B = int(int (*...p)(T, U));
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'T' and 'U' that have different lengths (1 vs. 2)}}
   template  D(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
@@ -484,7 +484,7 @@
 template  struct G {};
 template  struct E {
   template  using B = G...>;
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'I' and 'U' that have different lengths (1 vs. 2)}}
   template  E(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,23 @@
   return true;
 }
 
+bool
+VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) {
+  Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+  return true;
+}
+
+bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
+  Unexpanded.push_back({T, SourceLocation()});
+  return true;
+}
+
+bool
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}
+
 /// Record occurrences of function and non-type template
 /// parameter packs in an expression.
 bool VisitDeclRefExpr(DeclRefExpr *E) {
@@ -306,7 +323,8 @@
   auto *TTPD = dyn_cast(LocalPack);
   return TTPD && TTPD->getTy

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:860
+} else if (const auto *ND = Unexpanded[I].first.get();
+   isa(ND)) {
+  // Function parameter pack or init-capture pack.

mizvekov wrote:
> erichkeane wrote:
> > This pattern with the init + condition is a little awkward here... any 
> > reason to not just use the cast around the 'ND" and just use the VD in the 
> > use below? or is there a good reason to split it up like this?
> > 
> > Same with the above case.
> No very strong reason than that just from my different perception this looked 
> fine :)
> 
> Minor advantage that we don't make the variable a VarDecl pointer if we don't 
> need to access it as such.
> 
> But no strong preference here, I can have another look tomorrow.
I played a little bit with this change.

I think one thing that makes that pattern of adding separate ND and VD a bit 
confusing is that at a glance it almost looks like these are different cases in 
the PointerUnion variant. You have to do a double take to see that, since the 
nested cast is a bit subtle. This can become even subtler as we add more cases 
in the next patch.

Or we could stop using PointerUnion on the next patch, since it's so 
unergonomic with so many variants.

Anyway, I did some other refactorings and I think in general this will be much 
clearer to read on my next push.

With this refactoring, on this part here that problem goes away since we make 
this section produce a NamedDecl.

On the second part, below, I tidied up the code so much that I think that 
nested else isn't almost invisible anymore, since the other blocks become about 
the same size.

Otherwise, let me know what you think.

I also added to this first part here a FIXME comment describing a pre-existing 
problem where if we get a canonical TTP, the diagnostics fail to point to the 
relevant parameter since we won't have it's identifier.

Will try to add a repro and fix for that on the next patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

More tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132945

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -939,3 +939,68 @@
 static_assert(max(1,2)==2);
 static_assert(mid(1,2,3)==2);
 } // namespace GH51182
+
+// https://github.com/llvm/llvm-project/issues/56183
+namespace GH56183 {
+consteval auto Foo(auto c) { return c; }
+consteval auto Bar(auto f) { return f(); }
+void test() {
+  constexpr auto x = Foo(Bar([] { return 'a'; }));
+  static_assert(x == 'a');
+}
+}  // namespace GH56183
+
+// https://github.com/llvm/llvm-project/issues/51695
+namespace GH51695 {
+// Original 
+template 
+struct type_t {};
+
+template 
+struct list_t {};
+
+template 
+consteval auto pop_front(list_t) -> auto {
+  return list_t{};
+}
+
+template 
+consteval auto apply(list_t, F fn) -> auto {
+  return fn(type_t{}...);
+}
+
+void test1() {
+  constexpr auto x = apply(pop_front(list_t{}),
+[](type_t...) { return 42; });
+  static_assert(x == 42);
+}
+// Reduced 
+consteval bool zero() { return false; }
+
+template 
+consteval bool foo(bool, F f) {
+  return f();
+}
+
+template 
+consteval auto bar(F f) { return f;}
+
+void test2() {
+  constexpr auto x = foo(zero(), []() { return true; });
+  constexpr auto y = bar(bar(bar(bar([]() { return true; }();
+  static_assert(x && y);
+}
+
+}  // namespace GH51695
+
+// https://github.com/llvm/llvm-project/issues/50455
+namespace GH50455 {
+void f() {
+  []() consteval { int i{}; }();
+  []() consteval { int i{}; ++i; }();
+}
+void g() {
+  (void)[](int i) consteval { return i; }(0);
+  (void)[](int i) consteval { return i; }(0);
+}
+}  // namespace GH50455
\ No newline at end of file
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17598,6 +17598,7 @@
   DRSet.erase(E);
   return E;
 }
+ExprResult TransformLambdaExpr(LambdaExpr *E) { return E; }
 bool AlwaysRebuild() { return false; }
 bool ReplacingOriginal() { return true; }
 bool AllowSkippingCXXConstructExpr() {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -191,8 +191,12 @@
 
 - Correctly set expression evaluation context as 'immediate function context' in
   consteval functions.
-  This fixes `GH51182 `
+  This fixes `GH51182 `_.
 
+- Skip re-building lambda expressions when they appear as parameters to an immediate invocation.
+  This fixes `GH56183 `_,
+  `GH51695 `_,
+  `GH50455 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

new line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132945

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -939,3 +939,68 @@
 static_assert(max(1,2)==2);
 static_assert(mid(1,2,3)==2);
 } // namespace GH51182
+
+// https://github.com/llvm/llvm-project/issues/56183
+namespace GH56183 {
+consteval auto Foo(auto c) { return c; }
+consteval auto Bar(auto f) { return f(); }
+void test() {
+  constexpr auto x = Foo(Bar([] { return 'a'; }));
+  static_assert(x == 'a');
+}
+}  // namespace GH56183
+
+// https://github.com/llvm/llvm-project/issues/51695
+namespace GH51695 {
+// Original 
+template 
+struct type_t {};
+
+template 
+struct list_t {};
+
+template 
+consteval auto pop_front(list_t) -> auto {
+  return list_t{};
+}
+
+template 
+consteval auto apply(list_t, F fn) -> auto {
+  return fn(type_t{}...);
+}
+
+void test1() {
+  constexpr auto x = apply(pop_front(list_t{}),
+[](type_t...) { return 42; });
+  static_assert(x == 42);
+}
+// Reduced 
+consteval bool zero() { return false; }
+
+template 
+consteval bool foo(bool, F f) {
+  return f();
+}
+
+template 
+consteval auto bar(F f) { return f;}
+
+void test2() {
+  constexpr auto x = foo(zero(), []() { return true; });
+  constexpr auto y = bar(bar(bar(bar([]() { return true; }();
+  static_assert(x && y);
+}
+
+}  // namespace GH51695
+
+// https://github.com/llvm/llvm-project/issues/50455
+namespace GH50455 {
+void f() {
+  []() consteval { int i{}; }();
+  []() consteval { int i{}; ++i; }();
+}
+void g() {
+  (void)[](int i) consteval { return i; }(0);
+  (void)[](int i) consteval { return i; }(0);
+}
+}  // namespace GH50455
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17598,6 +17598,7 @@
   DRSet.erase(E);
   return E;
 }
+ExprResult TransformLambdaExpr(LambdaExpr *E) { return E; }
 bool AlwaysRebuild() { return false; }
 bool ReplacingOriginal() { return true; }
 bool AllowSkippingCXXConstructExpr() {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -191,8 +191,12 @@
 
 - Correctly set expression evaluation context as 'immediate function context' in
   consteval functions.
-  This fixes `GH51182 `
+  This fixes `GH51182 `_.
 
+- Skip re-building lambda expressions when they appear as parameters to an immediate invocation.
+  This fixes `GH56183 `_,
+  `GH51695 `_,
+  `GH50455 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Erich Keane via Phabricator via cfe-commits]

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

Happy enough, LGTM.




Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:860
+} else if (const auto *ND = Unexpanded[I].first.get();
+   isa(ND)) {
+  // Function parameter pack or init-capture pack.

mizvekov wrote:
> mizvekov wrote:
> > erichkeane wrote:
> > > This pattern with the init + condition is a little awkward here... any 
> > > reason to not just use the cast around the 'ND" and just use the VD in 
> > > the use below? or is there a good reason to split it up like this?
> > > 
> > > Same with the above case.
> > No very strong reason than that just from my different perception this 
> > looked fine :)
> > 
> > Minor advantage that we don't make the variable a VarDecl pointer if we 
> > don't need to access it as such.
> > 
> > But no strong preference here, I can have another look tomorrow.
> I played a little bit with this change.
> 
> I think one thing that makes that pattern of adding separate ND and VD a bit 
> confusing is that at a glance it almost looks like these are different cases 
> in the PointerUnion variant. You have to do a double take to see that, since 
> the nested cast is a bit subtle. This can become even subtler as we add more 
> cases in the next patch.
> 
> Or we could stop using PointerUnion on the next patch, since it's so 
> unergonomic with so many variants.
> 
> Anyway, I did some other refactorings and I think in general this will be 
> much clearer to read on my next push.
> 
> With this refactoring, on this part here that problem goes away since we make 
> this section produce a NamedDecl.
> 
> On the second part, below, I tidied up the code so much that I think that 
> nested else isn't almost invisible anymore, since the other blocks become 
> about the same size.
> 
> Otherwise, let me know what you think.
> 
> I also added to this first part here a FIXME comment describing a 
> pre-existing problem where if we get a canonical TTP, the diagnostics fail to 
> point to the relevant parameter since we won't have it's identifier.
> 
> Will try to add a repro and fix for that on the next patch.
Thanks for spending time on this... the nested conditionals on the var-decl was 
hiding 90%+ of what was going on in the branch, and at least this top one is 
BETTER enough than before.  I too hate the pointer union (note BTW, that _4_ is 
the maximum number of elements thanks to 32 bit platforms, that'll burn you :)).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D129488#3758449 , @jyknight wrote:

> I'm confused about this new strategy of special-casing 
> `source_location::current()`. Isn't it wrong to eagerly evaluate _other_ 
> calls in default args, as well? ISTM that source_location is simply 
> _exposing_ the bug in where we evaluate these expressions, but that it's 
> actually a more general problem?

+1 -- special casing things by name is almost always a red flag in the frontend 
(we do it from time to time when trying to add compatibility hacks for broken 
3rd party headers though, which is reasonable). I don't think there's supposed 
to be anything special about `std::source_location::current()` evaluation. 
Default arguments are always evaluated at the site of the call, not the callee, 
so eager evaluation seems like the root cause of the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/tools/libclang/CXString.cpp:85
+  if (String.empty())
+return createEmpty();
+

Please split this change into a separate patch and add a unit test.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

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

[PATCH] D132950: Remove unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests.

[Ying Yi via Phabricator via cfe-commits]

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

Some PS4/PS45 tests include the line `REQUIRES: x86-registered-target` meaning 
they depend on having the X86 backend included in the build. This should not be 
necessary since the driver understands all triples regardless of which backends 
are included.

The change removes all unnecessary `REQUIRES: x86-registered-target` from 
ps4/ps5 driver tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132950

Files:
  clang/test/Driver/ps4-pic.c
  clang/test/Driver/ps4-ps5-header-search.c
  clang/test/Driver/ps4-ps5-linker-non-win.c
  clang/test/Driver/ps4-ps5-linker-win.c
  clang/test/Driver/ps4-ps5-relax-relocations.c
  clang/test/Driver/ps4-ps5-runtime-flags.c
  clang/test/Driver/ps4-sdk-root.c
  clang/test/Driver/ps4ps5base.c
  clang/test/Driver/ps5-pic.c
  clang/test/Driver/ps5-sdk-root.c

Index: clang/test/Driver/ps5-sdk-root.c
===
--- clang/test/Driver/ps5-sdk-root.c
+++ clang/test/Driver/ps5-sdk-root.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-
 /// (Essentially identical to ps4-sdk-root.c except for the target.)
 
 /// Check that PS5 clang doesn't report a warning message when locating
Index: clang/test/Driver/ps5-pic.c
===
--- clang/test/Driver/ps5-pic.c
+++ clang/test/Driver/ps5-pic.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-
 // Test the driver's control over the PIC behavior for PS5 compiler.
 // These consist of tests of the relocation model flags and the
 // pic level flags passed to CC1.
Index: clang/test/Driver/ps4ps5base.c
===
--- clang/test/Driver/ps4ps5base.c
+++ clang/test/Driver/ps4ps5base.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-
 // Test that the driver always emits -fno-use-init-array on the PS4/PS5 targets
 // since their ABI does not support the .init_array section.
 
Index: clang/test/Driver/ps4-sdk-root.c
===
--- clang/test/Driver/ps4-sdk-root.c
+++ clang/test/Driver/ps4-sdk-root.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-
 // Check that PS4 clang doesn't report a warning message when locating
 // system header files (either by looking at the value of SCE_ORBIS_SDK_DIR
 // or relative to the location of the compiler driver), if "-nostdinc",
Index: clang/test/Driver/ps4-ps5-runtime-flags.c
===
--- clang/test/Driver/ps4-ps5-runtime-flags.c
+++ clang/test/Driver/ps4-ps5-runtime-flags.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-//
 /// Test the profile runtime library to be linked for PS4/PS5 compiler.
 /// Check runtime flag --dependent-lib which does not append the default library search path.
 //
Index: clang/test/Driver/ps4-ps5-relax-relocations.c
===
--- clang/test/Driver/ps4-ps5-relax-relocations.c
+++ clang/test/Driver/ps4-ps5-relax-relocations.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-
 // RUN: %clang -### -target x86_64-scei-ps4 %s -o - 2>&1 | \
 // RUN:   FileCheck %s
 // RUN: %clang -### -target x86_64-scei-ps4 -Wa,-mrelax-relocations=yes %s -o - 2>&1 | \
Index: clang/test/Driver/ps4-ps5-linker-win.c
===
--- clang/test/Driver/ps4-ps5-linker-win.c
+++ clang/test/Driver/ps4-ps5-linker-win.c
@@ -1,7 +1,7 @@
 // This test checks that orbis-ld is used for PS4 linker all the time, and
 // prospero-lld is used for PS5 linker. Specifying -fuse-ld causes an error.
 
-// REQUIRES: system-windows, x86-registered-target
+// REQUIRES: system-windows
 
 // RUN: mkdir -p %t
 // RUN: touch %t/orbis-ld.exe
Index: clang/test/Driver/ps4-ps5-linker-non-win.c
===
--- clang/test/Driver/ps4-ps5-linker-non-win.c
+++ clang/test/Driver/ps4-ps5-linker-non-win.c
@@ -1,6 +1,5 @@
 /// Checks proper linker prefixing for PS4 and PS5.
 // UNSUPPORTED: system-windows
-// REQUIRES: x86-registered-target
 
 // RUN: mkdir -p %t
 // RUN: rm -f %t/orbis-ld
Index: clang/test/Driver/ps4-ps5-header-search.c
===
--- clang/test/Driver/ps4-ps5-header-search.c
+++ clang/test/Driver/ps4-ps5-header-search.c
@@ -1,5 +1,3 @@
-// REQUIRES: x86-registered-target
-
 /// PS4 and PS5 use the same SDK layout, so use the same tree for both.
 // RUN: env SCE_ORBIS_SDK_DIR=%S/Inputs/scei-ps4_tree %clang -target x86_64-scei-ps4 -E -v %s 2>&1 | FileCheck %s --check-prefix=ENVPS4
 // RUN: env SCE_PROSPERO_SDK_DIR=%S/Inputs/sc

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[YingChi Long via Phabricator via cfe-commits]

inclyc created this revision.
inclyc added a reviewer: aaron.ballman.
Herald added a project: All.
inclyc added reviewers: rsmith, clang-language-wg.
inclyc published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes: https://github.com/llvm/llvm-project/issues/57098


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132952

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/warn-vla.c


Index: clang/test/Sema/warn-vla.c
===
--- clang/test/Sema/warn-vla.c
+++ clang/test/Sema/warn-vla.c
@@ -5,8 +5,18 @@
   int v[n]; // expected-warning {{variable length array}}
 }
 
-void test2(int n, int v[n]) { // expected-warning {{variable length array}}
+void test2(int n, int v[n]) { // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
 }
 
-void test3(int n, int v[n]); // expected-warning {{variable length array}}
+void test3(int n, int v[n]); // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
 
+void test4(int n, int v[][*]); // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2367,9 +2367,11 @@
 unsigned VLADiag;
 bool VLAIsError;
 bool IsVLA = false;
+bool SuppressNotICEVLA = false;
 
-VLADiagnoser(unsigned VLADiag, bool VLAIsError)
-: VLADiag(VLADiag), VLAIsError(VLAIsError) {}
+VLADiagnoser(unsigned VLADiag, bool VLAIsError, bool SuppressNotICEVLA)
+: VLADiag(VLADiag), VLAIsError(VLAIsError),
+  SuppressNotICEVLA(SuppressNotICEVLA) {}
 
 Sema::SemaDiagnosticBuilder diagnoseNotICEType(Sema &S, SourceLocation Loc,
QualType T) override {
@@ -2379,14 +2381,18 @@
 Sema::SemaDiagnosticBuilder diagnoseNotICE(Sema &S,
SourceLocation Loc) override {
   IsVLA = !VLAIsError;
-  return S.Diag(Loc, VLADiag);
+  if (!SuppressNotICEVLA)
+return S.Diag(Loc, VLADiag);
+  return Sema::SemaDiagnosticBuilder(S);
 }
 
 Sema::SemaDiagnosticBuilder diagnoseFold(Sema &S,
  SourceLocation Loc) override {
   return S.Diag(Loc, diag::ext_vla_folded_to_constant);
 }
-  } Diagnoser(VLADiag, VLAIsError);
+  } Diagnoser(VLADiag, VLAIsError,
+  S.getCurScope()->isFunctionPrototypeScope() &&
+  VLADiag == diag::warn_vla_used);
 
   ExprResult R =
   S.VerifyIntegerConstantExpression(ArraySize, &SizeVal, Diagnoser);
@@ -2528,7 +2534,9 @@
   llvm::APSInt ConstVal(Context.getTypeSize(Context.getSizeType()));
   if (!ArraySize) {
 if (ASM == ArrayType::Star) {
-  Diag(Loc, VLADiag);
+  if (!(getCurScope()->isFunctionPrototypeScope() &&
+VLADiag == diag::warn_vla_used))
+Diag(Loc, VLADiag);
   if (VLAIsError)
 return QualType();
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1756,6 +1756,8 @@
   K_Deferred
 };
 
+// Special builder emitting no diagnostics
+SemaDiagnosticBuilder(Sema &S) : S(S) {}
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   FunctionDecl *Fn, Sema &S);
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &&D);


Index: clang/test/Sema/warn-vla.c
===
--- clang/test/Sema/warn-vla.c
+++ clang/test/Sema/warn-vla.c
@@ -5,8 +5,18 @@
   int v[n]; // expected-warning {{variable length array}}
 }
 
-void test2(int n, int v[n]) { // expected-warning {{variable length array}}
+void test2(int n, int v[n]) { // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
 }
 
-void test3(int n, int v[n]); // expected-warning {{variable length array}}
+void test3(int n, int v[n]); // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
 
+void test4(int n, int v[][*]); // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2367,9 +2367,11 @@
 unsigned VLADiag;
 bool VLAIsError;
 bool IsVLA = false;
+bool SuppressNotICEVLA = false;
 
-V

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:191
+  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
+ Analysis,  InitEnv, {}};
+  if (AI.SetupTest) {

formatting seems funny here?



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:194
+auto Error = AI.SetupTest(AO);
+if (Error) {
+  return Error;

no braces for a single-statement `if`. could also put the binding in the if 
condition: `if (auto Error = `...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

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

[PATCH] D132918: [clang] Fix a crash in constant evaluation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4797
 }
+// Can't access properties of an incomplete type.
+if (!RD->hasDefinition()) {

It seems to me that we shouldn't GET to this function with an incomplete type.  
I suspect whoever calls this is doing so incorrectly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132918

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

[PATCH] D132918: [clang] Fix a crash in constant evaluation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Also, without a reduced example lit-test, we shouldn't be making changes.  Use 
creduce or something, but PLEASE come back with a lit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132918

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

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Just a first glance at the patch, will try to do a more comprehensive review 
later.




Comment at: clang/lib/Sema/SemaConcept.cpp:823
+  //
+  // Using the pattern is suffice because the partial ordering rules guarantee
+  // the template paramaters are equivalent.





Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1838
 
-QualType Replacement = Arg.getAsType();
 

I think this change could be masking a bug.

The arguments to an instantiation should always be canonical.

We could implement one day instantiation with non-canonical args, but this 
would not be the only change required to get this to work.

And regardless it should be done in separate patch with proper test cases :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128750

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

[PATCH] D132950: Remove unnecessary `REQUIRES: x86-registered-target` from ps4/ps5 driver tests.

[Paul Robinson via Phabricator via cfe-commits]

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

LGTM.  Checking the tests over by eye, it looks like all run the clang driver 
with `-###` or `-E` so only the driver itself is invoked, and no backend 
dependency exists.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132950

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:1760
+// Special builder emitting no diagnostics
+SemaDiagnosticBuilder(Sema &S) : S(S) {}
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,

I haven't looked particularly closely, but this doesn't seem right to me.  
There is already the 'K_Nop' diagnostic Kind (see the line below), that we 
should look at the uses of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:652
ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));

Given that its now a map, should we use `UnorderedElementsAre`? Or, does 
`StringMap` guarantee ordering?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:1760
+// Special builder emitting no diagnostics
+SemaDiagnosticBuilder(Sema &S) : S(S) {}
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,

erichkeane wrote:
> I haven't looked particularly closely, but this doesn't seem right to me.  
> There is already the 'K_Nop' diagnostic Kind (see the line below), that we 
> should look at the uses of.
I've tried using the following constructor, I have to to pass in a series of 
redundant parameters (such as `DiagID` and `Loc`), which makes it strange to 
construct a Builder emitting no error message at all?

e.g.

```
Sema::SemaDiagnosticBuilder(Sema::SemaDiagnosticBuilder::Kind::K_Nop, Loc, 0,
  nullptr, S);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! Can you be sure to add more information to the 
description so that reviewers more quickly understand why the changes are being 
made?

In terms of the changes themselves, I don't think they're heading in the right 
direction just yet. The changes are because we want to implement 
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2992.pdf, which clarifies 
what constitutes a VLA, which is a problem with our type system or how we use 
it. We have the `VariableArrayType` to represent a VLA type and I think we're 
using that type when we should be using a variably modified type (see 
`Type::isVariablyModifiedType()`).




Comment at: clang/include/clang/Sema/Sema.h:1760
+// Special builder emitting no diagnostics
+SemaDiagnosticBuilder(Sema &S) : S(S) {}
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,

erichkeane wrote:
> I haven't looked particularly closely, but this doesn't seem right to me.  
> There is already the 'K_Nop' diagnostic Kind (see the line below), that we 
> should look at the uses of.
+1, I think it's dangerous to add this interface (especially because it's not 
an explicit ctor).



Comment at: clang/test/Sema/warn-vla.c:8-12
+void test2(int n, int v[n]) { // c99 no-warning
+#if __STDC_VERSION__ < 199901L
+// expected-warning@-2{{variable length arrays are a C99 feature}}
+#endif
 }

The diagnostic there is rather unfortunate because we're not using a 
variable-length array in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I was also under impression that this should fall out of the C++ semantics, but 
after playing around with it I am not so sure anymore.

> Isn't it wrong to eagerly evaluate _other_ calls in default args, as well?
> ISTM that source_location is simply _exposing_ the bug in where we evaluate 
> these expressions, but that it's actually a more general problem?

Yes, it will evaluate calls to other `consteval` functions and and I don't 
think there is a way to notice this if there are no errors in the `consteval` 
functions. `constexpr` and `consteval` functions should produce the same value 
when called with the same argument IIUC.
`source_location::current()` is special in that regard and it breaks that 
assumption. E.g. the fact that `CXXDefaultArgExpr` references the same 
expression from `ParmVarDecl`.
We could delay evaluation of other `consteval` functions until the default 
argument is actually used relatively cheaply for other functions, but 
`source_location::current()` is the only one that requires a complete revamp of 
how `CXXDefaultArgExpr` works (see D132941  
for an attempt, it's not pretty, unless there is a simple way I missed).

I am also torn on how to interpret the standard. It might be that evaluating 
immediate function in default arguments in actually required.
`[dcl.fct.default]p5` mentions we need to do full semantic checking of the 
initializer for default arguments:

  The default argument has the same semantic constraints as the initializer in 
a declaration of a variable of the
  parameter type, using the copy-initialization semantics (9.4). The names in 
the default argument are bound,
  and the semantic constraints are checked, at the point where the default 
argument appears.

Additionally, the standard requires all immediate invocations to be evaluated 
(unless they are in immediate function context, which is not the case if 
default arguments are not for consteval function).
Hence, we should also evaluate the immediate invocations as part of the 
semantic checking we do for initializer of the default argument.

All major compilers certainly evaluate the `consteval` functions in the default 
arguments even without calls to the function that has those parameters:
https://gcc.godbolt.org/z/qTzrf6Msx

It might be an implementation quirk and a clarification would be nice. I am 
leaning towards thinking this behavior is actually standard-compliant.
This does not contradict the fact that default arguments must be evaluated in 
the context of the caller.
One can't see the difference for `consteval` functions as they must be replaced 
by constants.

Both GCC and MSVC seem to special-case `std::source_location::current()` from 
what I can tell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 456699.
wyt marked an inline comment as done.
wyt added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -94,6 +94,11 @@
   /// takes as argument the AST generated from the code being analyzed and the
   /// initial state from which the analysis starts with.
   std::function MakeAnalysis;
+  /// Optional. If provided, this function is executed immediately before
+  /// running the dataflow analysis to allow for additional setup. All fields in
+  /// the `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
   std::function
-getAnnotationLinesAndContent(const AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(const AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt());
-  if (It == StmtToAnnotations.end())
-return;
-  auto *Lattice =
-  llvm::any_cast(&State.Lattice.Value);
-  Results.emplace_back(It->second, StateT{*Lattice, State.Env});
-});
-  }
-
-  return Results;
-}
+buildLineToAnnotationMapping(SourceManager &SM,
+ llvm::Annotations AnnotatedCode);
 
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
 /// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`.
@@ -174,9 +135,9 @@
 ///
 ///   `VerifyResults` must be provided.
 template 
-llvm::Error checkDataflow(
-AnalysisInputs AI,
-std::function VerifyResults) {
+llvm::Error
+checkDataflow(AnalysisInputs AI,
+  std::function VerifyResults) {
   // Build AST context from code.
   llvm::Annotations AnnotatedCode(AI.Code);
   auto Unit = tooling::buildASTFromCodeWithArgs(
@@ -210,7 +171,7 @@
 return MaybeCFCtx.takeError();
   auto &CFCtx = *MaybeCFCtx;
 
-  // Initialize states and run dataflow analysis.
+  // Initialize states for running dataflow analysis.
   DataflowAnalysisContext DACtx(std::make_unique());
   Environment InitEnv(DACtx, *Target);
   auto Analysis = AI.MakeAnalysis(Context, InitEnv);
@@ -225,19 +186,26 @@
 };
   }
 
-  // If successful, the run returns a mapping from block IDs to the
-  // post-analysis states for the CFG blocks that have been evaluated.
+  // Additional test setup.
+  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
+ Analysis,  InitEnv, {}};
+  if (AI.SetupTest) {
+if (auto Error = AI.SetupTest(AO))
+  return Error;
+  }
+
+  // If successful, the dataflow analysis returns a mapping from block IDs to
+  // the post-analysis states for the CFG blocks that have been evaluated.
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv,
PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
-  auto &BlockStates = *MaybeBlockStates;
+  AO.BlockStates = *MaybeBlockStates;
 
   // Verify dataflow analysis outputs.
-  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
- Analysis, 

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

[weiyi via Phabricator via cfe-commits]

wyt added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:191
+  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
+ Analysis,  InitEnv, {}};
+  if (AI.SetupTest) {

ymandel wrote:
> formatting seems funny here?
this is returned from clang-format, seems to align the arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

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

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

[weiyi via Phabricator via cfe-commits]

wyt added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:652
ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));

ymandel wrote:
> Given that its now a map, should we use `UnorderedElementsAre`? Or, does 
> `StringMap` guarantee ordering?
`StringMap` is not yet exposed to the tests in this patch.

Some ordering is currently required to correctly retrieve the states in the 
tests. Hence, the conversion from `StringMap` to `vector>` 
(in deprecated `checkDataflow`) orders the elements alphabetically on the 
annotations.

Future refactoring will expose `StringMap` in the tests, where we will then not 
need to maintain an ordering since we can retrieve elements by keys, we can 
then use `UnorderedElementsAre`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

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

[PATCH] D132918: [clang] Fix a crash in constant evaluation

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4797
 }
+// Can't access properties of an incomplete type.
+if (!RD->hasDefinition()) {

erichkeane wrote:
> It seems to me that we shouldn't GET to this function with an incomplete 
> type.  I suspect whoever calls this is doing so incorrectly.
Also note we only check in `ExprConstant.cpp` for `hasDefinition()` in one 
other place in `findCompleteObject` and that is around extern see: 
https://github.com/llvm/llvm-project/commit/c0d04a2567c22631595bed8092bc042bb91ea4ee#diff-255a21a02a8966766225831836d482547787baf9a770fbf67178ebb7d7347e27


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132918

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

[PATCH] D132874: [clang] Don't emit debug vtable information for consteval functions

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1761
 
-  if (Method->isVirtual()) {
+  if (Method->isVirtual() && !Method->isConsteval()) {
 if (Method->isPure())

luken-google wrote:
> shafik wrote:
> > It is not clear to me if this is the core issue or not. Can you explain 
> > this a little better.
> Thanks for the feedback!
> 
> Sure, here's what I'm thinking. This code calls 
> ItaniumVTableContext::getMethodVTableIndex(GlobalDecl) on line 1771 below. 
> That method asserts in VTableBuilder.cpp:2281 because it tries to look up a 
> vtable entry for the consteval method and fails to do so. Any consteval 
> methods are prevented from gaining vtable entries by the logic in 
> VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD):
> 
> ```
> bool VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD) {
>   return MD->isVirtual() && !MD->isConsteval();
> }
> ```
> 
> This call is peppered throughout the VTableBuilder code, but the relevant 
> call for our purposes is in ItaniumVTableBuilder::AddMethods() in 
> VTableBuilder.cpp:1492, which causes any consteval method to be skipped when 
> the code is iterating through all virtual member functions and adding them to 
> the vtable.
> 
> My fix mirrors the logic in VTableContextBase::hasVtableSlot here, because 
> the code assumes that if the method is virtual it has a vtable index, which 
> consteval functions don't. Writing this, I've convinced myself it's better to 
> just call that method directly, so I've refactored the code to avoid the 
> duplicate logic.
Thank you, that makes sense. Using the function call is much better, it avoids 
logic duplication and assures that if the logic is updated this section will 
stay in sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132874

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

[PATCH] D132352: Introduce noread_thread_id to address the thread identification problem in coroutines

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D132352#3757433 , @ChuanqiXu wrote:

> In D132352#3757415 , @rjmccall 
> wrote:
>
>> Stackful coroutine bodies should be straightforward to support on top of the 
>> other work you've been doing, if anyone's actually interested in pursuing 
>> them.  As far as the optimizer needs to know, a stackful coroutine function 
>> is just like a presplit stackless coroutine except that calls and returns 
>> work normally and it's never split.  Because it's never split, the backends 
>> would need to understand that they can't arbitrarily reorder TLS 
>> materializations and so on in those functions, which would probably be the 
>> most complicated piece of work there.  Otherwise, I think we'd just need to 
>> mark stackful coroutine bodies with some new attribute and then change 
>> `cannotReadDifferentThreadIDIfMoved` to check for that, the same way it 
>> checks for presplit stackless coroutines.
>
> As far as I understand, we can't mark stackful coroutine bodies with special 
> attributes. It is slightly different from stackless coroutine. A stackless 
> coroutine is a suspendable function. So we can mark the function. But the 
> stackful coroutine is a thread in the user space actually. (Or we can think 
> stackful coroutine as a stack instead of a function)  In another word, if a 
> function A in a stackful coroutine calls another function B, then B lives in 
> the stackful coroutine too. The stackful coroutine switches by user(library) 
> implemented methods and they are not standardized so we can't even do hacks 
> for them.

Well, it's complicated.  Stackful coroutines don't generally involve creating a 
true thread, just a stack, and then yes, you swap the stack on the current 
thread in some system-specific way.  What I'm pointing out is that stackful 
coroutines can therefore observe changes in their thread ID across suspension 
points, which is the exact same problem as stackless coroutines have.  So we 
actually *would* need them to be identified specially, even if they're 
otherwise straightforward to compile, just so that we understand those special 
semantics and don't e.g. miscompile TLV accesses by hoisting them over a 
suspension.  (That this is still necessary might perhaps undermine some of the 
arguments for pursuing stackful coroutines in the first place; nonetheless, 
it's true.)

> I don't pursue stackful coroutine too. I raise the example to show the idea 
> of `noread_thread_id` may have some slight advantage.

Sure, we don't need to talk about this any further, since nobody's interested 
in pursuing it right now.  I'm just trying to underline the connections to what 
we've already done.


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

https://reviews.llvm.org/D132352

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

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

I just noticed that there probably needs to be one change. The doc sometimes 
describes `-fprebuilt-module-interface`, which is not a valid option. I think 
the occurrences were meant to be `-fpbrebuilt-module-path`.


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

https://reviews.llvm.org/D131388

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

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Another test under llvm-project/clang/test/CodeGen/ is needed which tests IR 
generated from C/C++ contains expected info.
Random example clang/test/CodeGen/attr-noundef.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130888

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

[clang] 3a0309c - [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2022-08-30T18:58:38+02:00
New Revision: 3a0309c53674be56b5cfce038d78a0c2c6e2a98c

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

LOG: [clang] Improve diagnostics for expansion length mismatch

When checking parameter packs for expansion, instead of basing the diagnostic 
for
length mismatch for outer parameters only on the known number of expansions,
we should also analyze SubstTemplateTypeParmPackType and 
SubstNonTypeTemplateParmPackExpr
for unexpanded packs, so we can emit a diagnostic pointing to a concrete
outer parameter.

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/SemaInternal.h
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
clang/test/SemaTemplate/pack-deduction.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 132e282797053..824ab42706334 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -118,6 +118,8 @@ Improvements to Clang's diagnostics
 - Correctly diagnose a future keyword if it exist as a keyword in the higher
   language version and specifies in which version it will be a keyword. This
   supports both c and c++ language.
+- When diagnosing multi-level pack expansions of mismatched lengths, Clang will
+  now, in most cases, be able to point to the relevant outer parameter.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0ee21cf64aa34..bd81d3a7e5ee5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -238,8 +238,11 @@ namespace threadSafety {
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair,
-  SourceLocation> UnexpandedParameterPack;
+using UnexpandedParameterPack = std::pair<
+llvm::PointerUnion<
+const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
+const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
+SourceLocation>;
 
 /// Describes whether we've seen any nullability information for the given
 /// file.

diff  --git a/clang/include/clang/Sema/SemaInternal.h 
b/clang/include/clang/Sema/SemaInternal.h
index 842eec099540c..4eca50919dc7e 100644
--- a/clang/include/clang/Sema/SemaInternal.h
+++ b/clang/include/clang/Sema/SemaInternal.h
@@ -62,7 +62,7 @@ inline InheritableAttr *getDLLAttr(Decl *D) {
 }
 
 /// Retrieve the depth and index of a template parameter.
-inline std::pair getDepthAndIndex(NamedDecl *ND) {
+inline std::pair getDepthAndIndex(const NamedDecl *ND) {
   if (const auto *TTP = dyn_cast(ND))
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
@@ -79,7 +79,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
   if (const auto *TTP = UPP.first.dyn_cast())
 return std::make_pair(TTP->getDepth(), TTP->getIndex());
 
-  return getDepthAndIndex(UPP.first.get());
+  return getDepthAndIndex(UPP.first.get());
 }
 
 class TypoCorrectionConsumer : public VisibleDeclConsumer {

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 84c4a191ec327..e8837d88f97dd 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -738,8 +738,11 @@ class PackDeductionScope {
   SmallVector Unexpanded;
   S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
   for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
-unsigned Depth, Index;
-std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
+UnexpandedParameterPack U = Unexpanded[I];
+if (U.first.is() ||
+U.first.is())
+  continue;
+auto [Depth, Index] = getDepthAndIndex(U);
 if (Depth == Info.getDeducedDepth())
   AddPack(Index);
   }

diff  --git a/clang/lib/Sema/SemaTemplateVariadic.cpp 
b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 285bf67398f16..69e0c70bbec53 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,23 @@ namespace {
   return true;
 }
 
+bool
+VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) 
{
+  Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+  return true;
+}
+
+bool VisitSubstTemplateTypeParmPackType(SubstTemp

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a0309c53674: [clang] Improve diagnostics for expansion 
length mismatch (authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaInternal.h
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
  clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
  clang/test/SemaTemplate/pack-deduction.cpp

Index: clang/test/SemaTemplate/pack-deduction.cpp
===
--- clang/test/SemaTemplate/pack-deduction.cpp
+++ clang/test/SemaTemplate/pack-deduction.cpp
@@ -134,14 +134,14 @@
   template struct tuple {};
   template struct A {
 template static pair, tuple> f(pair ...p);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 // expected-note@-2 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (at least 3 vs. 2) from outer parameter packs}}
 
 template static pair, tuple> g(pair ...p, ...);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 3) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 3)}}
 
 template static tuple h(tuple..., pair>);
-// expected-note@-1 {{[with U = ]: pack expansion contains parameter pack 'U' that has a different length (2 vs. 1) from outer parameter packs}}
+// expected-note@-1 {{[with U = ]: pack expansion contains parameter packs 'T' and 'U' that have different lengths (2 vs. 1)}}
   };
 
   pair, tuple> k1 = A().f(pair(), pair());
Index: clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
===
--- clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
+++ clang/test/SemaTemplate/cxx1z-fold-expressions.cpp
@@ -97,7 +97,7 @@
   template  struct Constant {};
 
   template  struct Sum {
-template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter pack 'Js' that has a different length (1 vs. 2) from outer parameter packs}}
+template  using type = Constant<((Is + Js) + ... + 0)>; // expected-error {{pack expansion contains parameter packs 'Is' and 'Js' that have different lengths (1 vs. 2)}}
   };
 
   Sum<1>::type<1, 2> x; // expected-note {{instantiation of}}
Index: clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
+++ clang/test/CXX/temp/temp.decls/temp.variadic/p5.cpp
@@ -473,7 +473,7 @@
 namespace pr56094 {
 template  struct D {
   template  using B = int(int (*...p)(T, U));
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'T' and 'U' that have different lengths (1 vs. 2)}}
   template  D(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
@@ -484,7 +484,7 @@
 template  struct G {};
 template  struct E {
   template  using B = G...>;
-  // expected-error@-1 {{pack expansion contains parameter pack 'U' that has a different length (1 vs. 2) from outer parameter packs}}
+  // expected-error@-1 {{pack expansion contains parameter packs 'I' and 'U' that have different lengths (1 vs. 2)}}
   template  E(B *);
   // expected-note@-1 {{in instantiation of template type alias 'B' requested here}}
 };
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -88,6 +88,23 @@
   return true;
 }
 
+bool
+VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) {
+  Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
+  return true;
+}
+
+bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
+  Unexpanded.push_back({T, SourceLocation()});
+  return true;
+}
+
+bool
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}
+
 /// Record occurrences of function and non-type template
 /// parameter packs in an expression.

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

[Matheus Izvekov via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:860
+} else if (const auto *ND = Unexpanded[I].first.get();
+   isa(ND)) {
+  // Function parameter pack or init-capture pack.

erichkeane wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > erichkeane wrote:
> > > > This pattern with the init + condition is a little awkward here... any 
> > > > reason to not just use the cast around the 'ND" and just use the VD in 
> > > > the use below? or is there a good reason to split it up like this?
> > > > 
> > > > Same with the above case.
> > > No very strong reason than that just from my different perception this 
> > > looked fine :)
> > > 
> > > Minor advantage that we don't make the variable a VarDecl pointer if we 
> > > don't need to access it as such.
> > > 
> > > But no strong preference here, I can have another look tomorrow.
> > I played a little bit with this change.
> > 
> > I think one thing that makes that pattern of adding separate ND and VD a 
> > bit confusing is that at a glance it almost looks like these are different 
> > cases in the PointerUnion variant. You have to do a double take to see 
> > that, since the nested cast is a bit subtle. This can become even subtler 
> > as we add more cases in the next patch.
> > 
> > Or we could stop using PointerUnion on the next patch, since it's so 
> > unergonomic with so many variants.
> > 
> > Anyway, I did some other refactorings and I think in general this will be 
> > much clearer to read on my next push.
> > 
> > With this refactoring, on this part here that problem goes away since we 
> > make this section produce a NamedDecl.
> > 
> > On the second part, below, I tidied up the code so much that I think that 
> > nested else isn't almost invisible anymore, since the other blocks become 
> > about the same size.
> > 
> > Otherwise, let me know what you think.
> > 
> > I also added to this first part here a FIXME comment describing a 
> > pre-existing problem where if we get a canonical TTP, the diagnostics fail 
> > to point to the relevant parameter since we won't have it's identifier.
> > 
> > Will try to add a repro and fix for that on the next patch.
> Thanks for spending time on this... the nested conditionals on the var-decl 
> was hiding 90%+ of what was going on in the branch, and at least this top one 
> is BETTER enough than before.  I too hate the pointer union (note BTW, that 
> _4_ is the maximum number of elements thanks to 32 bit platforms, that'll 
> burn you :)).
> 
> 
Thanks, true about the maximum size :)

Some of the cases could be unified as we have two types and two expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan via Phabricator via cfe-commits]

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



Comment at: clang/tools/libclang/CXString.cpp:85
+  if (String.empty())
+return createEmpty();
+

gribozavr2 wrote:
> Please split this change into a separate patch and add a unit test.
> 
Do you know a good way to add a unit-test for this? This function only gets 
called from within libclang, it isn't exposed to clients of libclang, including 
libclangTest. So I think a unit test would probably need to invoke the same 
CXComment API that `c-index-test` invokes while running 
`comment-to-html-xml-conversion.cpp`. Would that be worth it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

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

[PATCH] D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> So I think the most valuable optimizations are low-level optimizations to 
> `ImmutableMap`. There were a few suggestions on the mailing list to use 
> something more modern than the AVL trees under the hood but I don't think 
> authors found much success with those.

Back then I was experimenting 

 by replacing some of our `ImmutableMap` instances with an alternative 
implementation. I've started with `Environment::ExprBindings` because profiling 
showed that as the most frequently used. I've chosen `immer::map` as an 
alternative immutable map implementation. It seemed to be very promising 
because of

- the applied Relaxed Radix Balanced Tree implementation 

- efficient batch manipulations 
 .

Results: I could make the LIT tests pass, except two checks. The performance 
was not promising though. The run-time was similar that we had with the AVL 
trees, however, the memory consumption was 5-10x higher! Consequently, I've 
abandoned the prototype. But, I think an **//efficient batch manipulation 
implementation with the existing AVL based//** `ImmutableMap` might be worth to 
experiment with further.

> - From high-level perspective almost half the time is spent in mark-and-sweep 
> garbage collection in `ExprEngine::removeDead()`. Which can't really be cut 
> as running `removeDead()` less often only increases the analysis time as it 
> makes the maps larger and therefore slower to access. And it's also hard to 
> optimize because the procedure is very complicated and fragile and very few 
> people actually understand how it works.

Yes, this is also aligned with my measurements. There are analyzer options to 
manipulate when the `removeDead` is called, before every Stmt, or before every 
basic block, or never. Actually, the last option is meaningless, because 
renders both the engine and many checkers faulty. The `before every basic 
block` option might still be worth to experiment with, but I could not find 
reasonable arguments to change to that yet (I cannot recall, but some lit tests 
were failing miserably with that option too).


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

https://reviews.llvm.org/D131707

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

[PATCH] D132421: [HLSL] Support PCH for cc1 mode

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Sema/HLSLExternalSemaSource.h:58
+/// them before we initialize the ExternalSemaSource base class.
+struct ChainedHLSLExternalSemaSourceMembers {
+  ChainedHLSLExternalSemaSourceMembers(ExternalSemaSource *ExtSema)

IIUC, this code just exists to make sure that the `ASTReader` deserializes 
before the external sema source starts adding things. Is that correct?

If so, you don't need to do this, instead you can just add this code to 
`InitializeSema()` to force the `ASTReader` to de-serialize the decls:

```
// If the translation unit has external storage force external decls to load.
if (AST.getTranslationUnitDecl()->hasExternalLexicalStorage())
(void)AST.getTranslationUnitDecl()->decls_begin();
```



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:297
+  }
+
   IdentifierInfo &HLSL = AST.Idents.get("hlsl", tok::TokenKind::identifier);

I think the core of what you'e doing here is not far off, but I think this 
pattern doesn't scale or cover all the use cases that matter.

The cases that come to my mind are:
1) PCH has a forward declaration, source doesn't use the type so we shouldn't 
complete the type.
2) PCH has a forward declaration, source uses the type, so we need to generate 
a complete decl.
3) PCH has a complete declaration, so we should re-use the declaration and not 
need to do anything.

We also need to keep in mind that we're going to keep adding more and more 
types, so this pattern where each new type needs to add code to lookup is going 
to be a challenge. It is also going to be a challenge to manage the complexity 
of PCH defines one thing but not another.

One thing you should look at is how clang handles the following C++ code:

```
template
class Foo;

template
class Foo {
  T Val;
};
```

This results in two decls. One for the forward decl and the second for the 
definition. The definition lists the forward declaration as the previous decl. 
We should do that here.

When we generate the HLSL namespace, we can do a lookup and if we find a 
previous decl, set the previous decl.

When we generate any of the builtin types, we can lookup the previous decl, and 
annotate as the previous decl. We'll can also handle the case where a decl from 
the PCH is complete. If the PCH provides a complete decl, we can skip 
generating any decls for it, and the lookups should just work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132421

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

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked 12 inline comments as done.
tejohnson added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1252
+AllocInfo->Info.getMinLifetime());
+  SmallVector StackIds;
+  std::set StackHashSet;

snehasish wrote:
> I think if you use an llvm::SetVector here instead then you don't need the 
> StackHashSet std::set below. CallstackTrie::addCallstack already accepts an 
> ArrayRef so it won't need to change if we use a SetVector.
Noted, but moot as this has been removed (see below)



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1253
+  SmallVector StackIds;
+  std::set StackHashSet;
+  for (auto StackFrame : AllocInfo->CallStack) {

snehasish wrote:
> nit: It doesn't look like we #include  in this file so we are probably 
> relying on having it transitively being included from somewhere.
Although this std::set has been removed, I use it elsewhere so added the 
include.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1256
+auto StackId = computeStackId(StackFrame);
+// Remove recursion cycles.
+// TODO: Consider handling this during profile generation.

I have removed this handling. It is not correct in the case of mutual recursion 
involving more than one function where it will result in non-sensical stack 
traces. I am deferring handling recursion until later during the LTO phase.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1276
+  if (Error E = MemProfResult.takeError()) {
+handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
+  auto Err = IPE.get();

snehasish wrote:
> Consider defining the lambda outside above the condition to reduce 
> indentation. IMO it will be  a little easier to follow if it wasn't inlined 
> into the if statement itself.
I could do this, but as is it mirrors the structure of the similar handling in 
readCounters, which has some advantages. wdyt?



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1313
+  std::map *, unsigned>>>
+  LocHashToCallSites;
+  const auto MemProfRec = std::move(MemProfResult.get());

snehasish wrote:
> LocHashToCallSiteFrame to indicate the value in the map corresponds to an 
> individual frame?
The key is the location hash (stack id) of a single frame, but the value is a 
set of all the CallSites from the profile that reference it.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1330
+  // Once we find this function, we can stop recording.
+  if (StackFrame.Function == FuncGUID)
+break;

snehasish wrote:
> Should we assert that it was actually found?
Added assert after the loop.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1360
+  // If leaf was found in a map, iterators pointing to its location in both
+  // of the maps (it may only exist in one).
+  std::map>::iterator

snehasish wrote:
> Can you add an assert for this?
In this case "may only" meant "might only", not "may only at most". So I can't 
assert on anything. This can happen for example if we have a location that 
corresponds to both an allocation call and another callsite (I've seen this 
periodically, and can reproduce e.g. with a macro). We would need to use 
discriminators more widely to better distinguish them in that case (with the 
handling here we will only match to the allocation call for now - edit: a 
slight change noted further below ensures this is the case). Will change 
/may/might/ and add a note.



Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1414
+
+  // First add !memprof metadata from allocation info, if we found the
+  // instruction's leaf location in that map, and if the rest of the

snehasish wrote:
> "First add !memprof metadata  ..." -- the ordering of the if-else condition 
> isn't necessary though since only one of the iters can be non-null? We could 
> rewrite the else condition first to reduce the complexity here a bit. Eg --
> 
> ```
> if (CallSitesIter != LocHashToCallSites.end()) {
>   ...
>   continue
> }
> 
> // Flip the conditions here
> if (!isNewLikeFn() || AllocInfoIter == LocHashToAllocInfo.end()) {
>continue
> }
> 
> CallStackTrie AllocTrie;
> ...
> ```
As noted earlier, it might be in more than one map. But I realized we could 
sometimes add the callsite metadata, instead of the memprof metadata, to a 
non-new allocation call (e.g. malloc) when there is a matching location in both 
maps given the structuring of the handling below. I've changed it so we handle 
all instructions with matching allocation profile data in the below if 
statement, and skip adding any metadat

[PATCH] D128142: [MemProf] Memprof profile matching and annotation

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 456710.
tejohnson marked 5 inline comments as done.
tejohnson added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128142

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/Inputs/memprof.exe
  clang/test/CodeGen/Inputs/memprof.memprofraw
  clang/test/CodeGen/memprof.cpp
  llvm/include/llvm/Analysis/MemoryBuiltins.h
  llvm/include/llvm/ProfileData/InstrProfReader.h
  llvm/lib/Analysis/MemoryBuiltins.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/memprof.exe
  llvm/test/Transforms/PGOProfile/Inputs/memprof.memprofraw
  llvm/test/Transforms/PGOProfile/Inputs/memprof_pgo.profraw
  llvm/test/Transforms/PGOProfile/memprof.ll
  llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll

Index: llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
@@ -0,0 +1,25 @@
+;; Tests that we get a missing memprof error for a function not in profile when
+;; using -pgo-warn-missing-function.
+
+;; TODO: Use text profile inputs once that is available for memprof.
+
+;; The raw profiles have been generated from the source used for the memprof.ll
+;; test (see comments at the top of that file).
+
+; RUN: llvm-profdata merge %S/Inputs/memprof.memprofraw --profiled-binary %S/Inputs/memprof.exe -o %t.memprofdata
+
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s
+
+; CHECK: memprof record not found for function hash {{.*}} _Z16funcnotinprofilev
+
+; ModuleID = 'memprofmissingfunc.cc'
+source_filename = "memprofmissingfunc.cc"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define dso_local void @_Z16funcnotinprofilev() {
+entry:
+  ret void
+}
+
Index: llvm/test/Transforms/PGOProfile/memprof.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/memprof.ll
@@ -0,0 +1,483 @@
+;; Tests memprof profile matching (with and without instrumentation profiles).
+
+;; TODO: Use text profile inputs once that is available for memprof.
+
+;; The input IR and raw profiles have been generated from the following source:
+;;
+;; #include 
+;; #include 
+;; #include 
+;; char *foo() {
+;;   return new char[10];
+;; }
+;; char *foo2() {
+;;   return foo();
+;; }
+;; char *bar() {
+;;   return foo2();
+;; }
+;; char *baz() {
+;;   return foo2();
+;; }
+;; char *recurse(unsigned n) {
+;;   if (!n)
+;; return foo();
+;;   return recurse(n-1);
+;; }
+;; int main(int argc, char **argv) {
+;;   // Test allocations with different combinations of stack contexts and
+;;   // coldness (based on lifetime, since they are all accessed a single time
+;;   // per byte via the memset).
+;;   char *a = new char[10];
+;;   char *b = new char[10];
+;;   char *c = foo();
+;;   char *d = foo();
+;;   char *e = bar();
+;;   char *f = baz();
+;;   memset(a, 0, 10);
+;;   memset(b, 0, 10);
+;;   memset(c, 0, 10);
+;;   memset(d, 0, 10);
+;;   memset(e, 0, 10);
+;;   memset(f, 0, 10);
+;;   // a and c have short lifetimes
+;;   delete[] a;
+;;   delete[] c;
+;;   // b, d, e, and f have long lifetimes and will be detected as cold by default.
+;;   sleep(200);
+;;   delete[] b;
+;;   delete[] d;
+;;   delete[] e;
+;;   delete[] f;
+;;   // Loop ensures the two calls to recurse have stack contexts that only differ
+;;   // in one level of recursion. We should get two stack contexts reflecting the
+;;   // different levels of recursion and different allocation behavior (since the
+;;   // first has a very long lifetime and the second has a short lifetime).
+;;   for (unsigned i = 0; i < 2; i++) {
+;; char *g = recurse(i + 3);
+;; memset(g, 0, 10);
+;; if (!i)
+;;   sleep(200);
+;; delete[] g;
+;;   }
+;;   return 0;
+;; }
+;;
+;; The following commands were used to compile the source to instrumented
+;; executables and collect raw binary format profiles:
+;;
+;; # Collect memory profile:
+;; $ clang++ -fuse-ld=lld -no-pie -Wl,--no-rosegment -gmlt \
+;; 	-fdebug-info-for-profiling -mno-omit-leaf-frame-pointer \
+;;	-fno-omit-frame-pointer -fno-optimize-sibling-calls -m64 -Wl,-build-id \
+;; 	memprof.cc -o memprof.exe -fmemory-profile
+;; $ env MEMPROF_OPTIONS=log_path=stdout ./memprof.exe > memprof.memprofraw
+;;
+;; # Collect IR PGO profile:
+;; $ clang++ -fuse-ld=lld -no-pie -Wl,--no-rosegment -gmlt \
+;; 	-fdebug-info-for-profiling -mno-omit-leaf-frame-pointer \
+;;	-fno-omit-frame-pointer -fno-optimize-sibling-calls -m64 -Wl,-build-id \
+;; 	memprof.cc -o pgo.exe -fprofile-generate=.
+;; 

[clang] 739a747 - [Docs] [HLSL] Documenting HLSL Entry Functions

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-08-30T12:18:44-05:00
New Revision: 739a747b2368652155ac78f0ac341a6bfe640c60

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

LOG: [Docs] [HLSL] Documenting HLSL Entry Functions

This document describes the basic usage and implementation details for
HLSL entry functions in Clang.

Reviewed By: python3kgae

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

Added: 
clang/docs/HLSL/EntryFunctions.rst

Modified: 
clang/docs/HLSL/HLSLDocs.rst

Removed: 




diff  --git a/clang/docs/HLSL/EntryFunctions.rst 
b/clang/docs/HLSL/EntryFunctions.rst
new file mode 100644
index ..0e547aab420c
--- /dev/null
+++ b/clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,65 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL, entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler 
assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and 
all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In Clang, the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specified name. This allows code generation for entry functions to always key
+off the presence of the ``HLSLShader`` attribute, regardless of what shader
+profile you are compiling.
+
+In code generation, two functions are generated. One is the user defined
+function, which is code generated as a mangled C++ function with internal
+linkage following normal function code generation.
+
+The actual exported entry function which can be called by the GPU driver is a
+``void(void)`` function that isn't name mangled. In code generation we generate
+the unmangled entry function, instantiations of the parameters with their
+semantic values populated, and a call to the user-defined function. After the
+call instruction the return value (if any) is saved using a target-appropriate
+intrinsic for storing outputs (for DirectX, the ``llvm.dx.store.output``).
+
+.. note::
+
+   HLSL support in Clang is currently focused on compute shaders, which do not
+   support output semantics. Support for output semantics will not be
+   implemented until other shader profiles are supported.
+
+Below is example IR that represents the planned implementation, subject to
+change as the ``llvm.dx.store.output`` and ``llvm.dx.load.input`` intrinsics 
are
+not yet implemented.
+
+.. code-block:: none
+
+   ; Function Attrs: norecurse
+   define void @main() #1 {
+  entry:
+  %0 = call i32 @llvm.dx.load.input.i32(...)
+  %1 = call i32 @"?main@@YAXII@Z"(i32 %0)
+  call @llvm.dx.store.output.i32(%1, ...)
+  ret void
+   }
+

diff  --git a/clang/docs/HLSL/HLSLDocs.rst b/clang/docs/HLSL/HLSLDocs.rst
index 0b66e517a73a..2cfe631da939 100644
--- a/clang/docs/HLSL/HLSLDocs.rst
+++ b/clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@ HLSL Design and Implementation
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions



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

[PATCH] D132672: [Docs] [HLSL] Documenting HLSL Entry Functions

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG739a747b2368: [Docs] [HLSL] Documenting HLSL Entry Functions 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132672

Files:
  clang/docs/HLSL/EntryFunctions.rst
  clang/docs/HLSL/HLSLDocs.rst


Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- /dev/null
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,65 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL, entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler 
assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and 
all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In Clang, the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specified name. This allows code generation for entry functions to always key
+off the presence of the ``HLSLShader`` attribute, regardless of what shader
+profile you are compiling.
+
+In code generation, two functions are generated. One is the user defined
+function, which is code generated as a mangled C++ function with internal
+linkage following normal function code generation.
+
+The actual exported entry function which can be called by the GPU driver is a
+``void(void)`` function that isn't name mangled. In code generation we generate
+the unmangled entry function, instantiations of the parameters with their
+semantic values populated, and a call to the user-defined function. After the
+call instruction the return value (if any) is saved using a target-appropriate
+intrinsic for storing outputs (for DirectX, the ``llvm.dx.store.output``).
+
+.. note::
+
+   HLSL support in Clang is currently focused on compute shaders, which do not
+   support output semantics. Support for output semantics will not be
+   implemented until other shader profiles are supported.
+
+Below is example IR that represents the planned implementation, subject to
+change as the ``llvm.dx.store.output`` and ``llvm.dx.load.input`` intrinsics 
are
+not yet implemented.
+
+.. code-block:: none
+
+   ; Function Attrs: norecurse
+   define void @main() #1 {
+  entry:
+  %0 = call i32 @llvm.dx.load.input.i32(...)
+  %1 = call i32 @"?main@@YAXII@Z"(i32 %0)
+  call @llvm.dx.store.output.i32(%1, ...)
+  ret void
+   }
+


Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- /dev/null
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,65 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL, entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In Clang, the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specifie

[clang] de8f372 - [Docs] Fixing incorrect document title

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-08-30T12:19:05-05:00
New Revision: de8f372bfec7070a364fac57fb5f201fc2d8cb22

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

LOG: [Docs] Fixing incorrect document title

Doh! This clearly slipped my review. Thanks DuckDuckGo for showing me
the error of my ways :).

Added: 


Modified: 
clang/docs/HLSL/ResourceTypes.rst

Removed: 




diff  --git a/clang/docs/HLSL/ResourceTypes.rst 
b/clang/docs/HLSL/ResourceTypes.rst
index c537e824758b..aad7b3314f08 100644
--- a/clang/docs/HLSL/ResourceTypes.rst
+++ b/clang/docs/HLSL/ResourceTypes.rst
@@ -1,6 +1,6 @@
-
-HLSL Support
-
+===
+HLSL Resource Types
+===
 
 .. contents::
:local:



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

[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs updated this revision to Diff 456715.
isuckatcs marked an inline comment as done.
isuckatcs added a comment.

Removed the unnecessary extra RUN command.


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

https://reviews.llvm.org/D132654

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't 
warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't 
warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -75,13 +75,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D129488#3758798 , @ilya-biryukov 
wrote:

> I was also under impression that this should fall out of the C++ semantics, 
> but after playing around with it I am not so sure anymore.

I'm also starting to question that. :-)

>> Isn't it wrong to eagerly evaluate _other_ calls in default args, as well?
>> ISTM that source_location is simply _exposing_ the bug in where we evaluate 
>> these expressions, but that it's actually a more general problem?
>
> Yes, it will evaluate calls to other `consteval` functions and and I don't 
> think there is a way to notice this if there are no errors in the `consteval` 
> functions. `constexpr` and `consteval` functions should produce the same 
> value when called with the same argument IIUC.
> `source_location::current()` is special in that regard and it breaks that 
> assumption. E.g. the fact that `CXXDefaultArgExpr` references the same 
> expression from `ParmVarDecl`.
> We could delay evaluation of other `consteval` functions until the default 
> argument is actually used relatively cheaply for other functions, but 
> `source_location::current()` is the only one that requires a complete revamp 
> of how `CXXDefaultArgExpr` works (see D132941 
>  for an attempt, it's not pretty, unless 
> there is a simple way I missed).
>
> I am also torn on how to interpret the standard. It might be that evaluating 
> immediate function in default arguments in actually required.
> `[dcl.fct.default]p5` mentions we need to do full semantic checking of the 
> initializer for default arguments:
>
>   The default argument has the same semantic constraints as the initializer 
> in a declaration of a variable of the
>   parameter type, using the copy-initialization semantics (9.4). The names in 
> the default argument are bound,
>   and the semantic constraints are checked, at the point where the default 
> argument appears.
>
> Additionally, the standard requires all immediate invocations to be evaluated 
> (unless they are in immediate function context, which is not the case if 
> default arguments are not for consteval function).
> Hence, we should also evaluate the immediate invocations as part of the 
> semantic checking we do for initializer of the default argument.
>
> All major compilers certainly evaluate the `consteval` functions in the 
> default arguments even without calls to the function that has those 
> parameters:
> https://gcc.godbolt.org/z/qTzrf6Msx
>
> It might be an implementation quirk and a clarification would be nice. I am 
> leaning towards thinking this behavior is actually standard-compliant.
> This does not contradict the fact that default arguments must be evaluated in 
> the context of the caller.
> One can't see the difference for `consteval` functions as they must be 
> replaced by constants.
>
> Both GCC and MSVC seem to special-case `std::source_location::current()` from 
> what I can tell.

https://eel.is/c++draft/expr.call#7.sentence-3 is what causes the argument to 
be evaluated at the call expression rather than at the declaration while 
https://eel.is/c++draft/dcl.fct.default#5 makes it clear that the semantic 
constraints are checked at the declaration location. I couldn't find explicit 
wording which says this, but I believe that semantic checking in this case also 
involves determining whether the immediate function is a valid or not 
(https://eel.is/c++draft/expr.const#14), which should explain the behavior of 
your example in https://gcc.godbolt.org/z/qTzrf6Msx.

However, https://eel.is/c++draft/dcl.constexpr#5 says that the behavior has to 
be the same whether the constexpr function is runtime evaluated or constant 
evaluated, and a function declared `consteval` is a constexpr function 
(https://eel.is/c++draft/dcl.constexpr#2.sentence-1). That sure does make 
std::source_location::current() a rather special function -- if the call were 
to be evaluated at runtime (which it can't because it's an immediate function), 
it would give different results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

I just discovered this and was about to post on Discourse about it. Glad you're 
already on it :)

I don't think this is quite correct though? It'll turn off unwind tables for 
AArch64 entirely, whereas we want to keep sync unwind tables. If we want to 
minimize changes to the existing logic, maybe we could add 
`IsSyncUnwindTablesDefault` instead and have `SyncUnwindTables` default to that 
and `AsyncUnwindTables` default to `!` that?




Comment at: clang/include/clang/Driver/ToolChain.h:501
+  /// IsAsyncUnwindTablesDefault - Does this tool chain use
+  /// -fasync-unwind-tables by default.
+  virtual bool

I believe the option is spelled `-fasynchronous-unwind-tables`.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2898
+bool MachO::IsAsyncUnwindTablesDefault(const ArgList &Args) const {
+  // AArch64 has compact unwind format, making the size overhead from
+  // asynchronous unwind particularly bad, so disable by default.

This comment is a bit confusing. I thought Darwin supported compact unwind for 
all architectures. Is it that compact unwind can't handle async unwind on arm64 
but can on other architectures?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131153

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

[PATCH] D132911: [clang-format] Fix annotating when deleting array of pointers

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:2381-2385
 if (PrevToken->Tok.isLiteral() ||
 PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
tok::kw_false, tok::r_brace)) {
   return TT_BinaryOperator;
 }

jackhong12 wrote:
> The `*` token in `delete *x;` will be annotated as UnaryOperator 
> (dereferencing a pointer). 
> 
> In the case `delete[] *x;`, there is a `]` before `*`. So, it will be 
> annotated as BinaryOperator by this rule.
> 
> I think both `*` here should be UnaryOperator. Therefore, I add a new rule to 
> match the `delete[]` pattern.
 `it will be annotated as BinaryOperator by this rule.` of course my bad



Comment at: clang/unittests/Format/FormatTest.cpp:10541
+  verifyFormat("delete[] **ptr;", Style);
+  verifyFormat("delete[] *(ptr);", Style);
+

could you cover the other PointerAlignment case for completeness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132911

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

[PATCH] D131268: [HLSL] Generate buffer subscript operators

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 456731.
beanz added a comment.

Updating based on PR feedback and rebasing.

- Rebased on main today
- Made const subscript return type const &


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131268

Files:
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/HLSL/RWBuffer-AST.hlsl
  clang/test/CodeGenHLSL/buffer-array-operator.hlsl

Index: clang/test/CodeGenHLSL/buffer-array-operator.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/buffer-array-operator.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+const RWBuffer In;
+RWBuffer Out;
+
+void fn(int Idx) {
+  Out[Idx] = In[Idx];
+}
+
+// This test is intended to verify reasonable code generation of the subscript
+// operator. In this test case we should be generating both the const and
+// non-const operators so we verify both cases.
+
+// Non-const comes first.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QBAAAMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
+
+// Const comes next, and returns the pointer instead of the value.
+// CHECK: ptr @"??A?$RWBuffer@M@hlsl@@QMI@Z"
+// CHECK: %this1 = load ptr, ptr %this.addr, align 4
+// CHECK-NEXT: %h = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %this1, i32 0, i32 0
+// CHECK-NEXT: %0 = load ptr, ptr %h, align 4
+// CHECK-NEXT: %1 = load i32, ptr %Idx.addr, align 4
+// CHECK-NEXT: %arrayidx = getelementptr inbounds float, ptr %0, i32 %1
+// CHECK-NEXT: ret ptr %arrayidx
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -39,11 +39,30 @@
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'element_type *'
+
+// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &const (unsigned int) const'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
+// CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &(unsigned int)'
+// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  Idx 'unsigned int'
+// CHECK-NEXT: CompoundStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ReturnStmt 0x{{[0-9A-Fa-f]+}} <>
+// CHECK-NEXT: ArraySubscriptExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type' lvalue
+// CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' lvalue ->h 0x{{[0-9A-Fa-f]+}}
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'RWBuffer *' implicit this
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+
 // CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <>  class RWBuffer definition
 
 // CHECK: TemplateArgument type 'float'
 // CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
 // CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
 // CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
-// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'void *'
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit referenced h 'float *'
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2174,7 +2174,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
 return QualType();
   }
@@ -2244,7 +2244,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isValid()) {
 Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
 return QualType();
   }
@@ -3008,7 +3008,7 @@
 return QualType();
   }
 
-  if (getLangOpts().HLSL) {
+  if (getLangOpts().HLSL && Loc.isV

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> We have the VariableArrayType to represent a VLA type and I think we're using 
> that type when we should be using a variably modified type

The clang AST for function signatures encodes two types: the type as written, 
and the type after promotion.  Semantic analysis always uses the type after 
promotion, but the type as written is useful if you're trying to do source 
rewriting, or something like that.

We could theoretically mess with the AST representation somehow, but I don't 
see any compelling reason to.  We probably just want to emit the warn_vla_used 
diagnostic somewhere else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132952

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

[PATCH] D132913: [HLSL] Preserve vec3 for HLSL.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3524
+  // Preserve vec3 for HLSL.
+  CmdArgs.push_back("-fpreserve-vec3-type");
 }

Preserving vec3 is required for HLSL correctness, this shouldn't be tied to the 
DXC driver mode. Instead, the option should be implied by the HLSL language 
mode. You should be able to add the following to the TableGen definition for 
the `preserve-vec3-type` flag:

```
ImpliedByAnyOf<[hlsl.KeyPath]>;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132913

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

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
dgoldman added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

- Render protocols as interfaces to differentiate them from classes since a 
protocol and class can have the same name

- Properly call `includeSymbolFromIndex` even with a cached speculative fuzzy 
find request

- Don't use the index to provide completions for category names, symbols there 
don't make sense


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132962

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3215,7 +3215,10 @@
  /*Opts=*/{}, "Foo.m");
 
   auto C = Results.Completions;
-  EXPECT_THAT(C, UnorderedElementsAre(named("Food"), named("Fooey")));
+  EXPECT_THAT(C,
+  UnorderedElementsAre(
+  AllOf(named("Food"), kind(CompletionItemKind::Interface)),
+  AllOf(named("Fooey"), kind(CompletionItemKind::Interface;
 }
 
 TEST(CompletionTest, CursorInSnippets) {
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -94,10 +94,14 @@
   case SK::Struct:
 return CompletionItemKind::Struct;
   case SK::Class:
-  case SK::Protocol:
   case SK::Extension:
   case SK::Union:
 return CompletionItemKind::Class;
+  case SK::Protocol:
+// Use interface instead of class for differentiation of classes and
+// protocols with the same name (e.g. @interface NSObject vs. @protocol
+// NSObject).
+return CompletionItemKind::Interface;
   case SK::TypeAlias:
 // We use the same kind as the VSCode C++ extension.
 // FIXME: pick a better option when we have one.
@@ -712,13 +716,13 @@
   case CodeCompletionContext::CCC_Type:
   case CodeCompletionContext::CCC_ParenthesizedExpression:
   case CodeCompletionContext::CCC_ObjCInterfaceName:
-  case CodeCompletionContext::CCC_ObjCCategoryName:
   case CodeCompletionContext::CCC_Symbol:
   case CodeCompletionContext::CCC_SymbolOrNewName:
 return true;
   case CodeCompletionContext::CCC_OtherWithMacros:
   case CodeCompletionContext::CCC_DotMemberAccess:
   case CodeCompletionContext::CCC_ArrowMemberAccess:
+  case CodeCompletionContext::CCC_ObjCCategoryName:
   case CodeCompletionContext::CCC_ObjCPropertyAccess:
   case CodeCompletionContext::CCC_MacroName:
   case CodeCompletionContext::CCC_MacroNameUse:
@@ -1343,6 +1347,22 @@
   llvm_unreachable("invalid NestedNameSpecifier kind");
 }
 
+// Should we include a symbol from the index given the completion kind?
+// FIXME: Ideally we can filter in the fuzzy find request itself.
+bool includeSymbolFromIndex(CodeCompletionContext::Kind Kind,
+const Symbol &Sym) {
+  // Objective-C protocols are only useful in ObjC protocol completions,
+  // in other places they're confusing, especially when they share the same
+  // identifier with a class.
+  if (Sym.SymInfo.Kind == index::SymbolKind::Protocol &&
+  Sym.SymInfo.Lang == index::SymbolLanguage::ObjC)
+return Kind == CodeCompletionContext::CCC_ObjCProtocolName;
+  else if (Kind == CodeCompletionContext::CCC_ObjCProtocolName)
+// Don't show anything else in ObjC protocol completions.
+return false;
+  return true;
+}
+
 std::future startAsyncFuzzyFind(const SymbolIndex &Index,
 const FuzzyFindRequest &Req) {
   return runAsync([&Index, Req]() {
@@ -1675,14 +1695,6 @@
 return Output;
   }
 
-  bool includeSymbolFromIndex(const Symbol &Sym) {
-if (CCContextKind == CodeCompletionContext::CCC_ObjCProtocolName) {
-  return Sym.SymInfo.Lang == index::SymbolLanguage::ObjC &&
-  Sym.SymInfo.Kind == index::SymbolKind::Protocol;
-}
-return true;
-  }
-
   SymbolSlab queryIndex() {
 trace::Span Tracer("Query index");
 SPAN_ATTACH(Tracer, "limit", int64_t(Opts.Limit));
@@ -1716,10 +1728,8 @@
 
 // Run the query against the index.
 SymbolSlab::Builder ResultsBuilder;
-if (Opts.Index->fuzzyFind(Req, [&](const Symbol &Sym) {
-  if (includeSymbolFromIndex(Sym))
-ResultsBuilder.insert(Sym);
-}))
+if (Opts.Index->fuzzyFind(
+Req, [&](const Symbol &Sym) { ResultsBuilder.insert(Sym); }))
   Incomplete = true;
 return std::move(ResultsBuilder).build();
   }
@@ -1783,6 +1793,8 @@
 for (const auto &IndexResult : IndexResults) {
   if (Us