[clang] [Clang] allow `` `@$ `` in raw string delimiters in C++26 (PR #93216)

2024-05-23 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM but I would like Tom or Aaron to also take a look

https://github.com/llvm/llvm-project/pull/93216
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)

2024-05-22 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Can you also confirm this fixes: 
https://github.com/llvm/llvm-project/issues/70191 

https://github.com/llvm/llvm-project/pull/93079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Fix crash when diagnosing candidates with parameter packs (PR #93079)

2024-05-22 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

Thank you for the fix. Can you please a little more details to your summary so 
that folks reading git log have more context.

This also needs a release note.

Please also add that this also fixes: 
https://github.com/llvm/llvm-project/issues/76354

https://github.com/llvm/llvm-project/pull/93079
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add tests for CWG issues regarding completeness of types (PR #92113)

2024-05-21 Thread Shafik Yaghmour via cfe-commits


@@ -451,6 +463,25 @@ static_assert(!__is_nothrow_constructible(D4, int), "");
 #endif
 } // namespace cwg1350
 
+namespace cwg1352 { // cwg1352: 3.0
+struct A {
+#if __cplusplus >= 201103L
+  int a = sizeof(A);

shafik wrote:

It also mentions in the body of member function

https://github.com/llvm/llvm-project/pull/92113
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add tests for CWG issues regarding completeness of types (PR #92113)

2024-05-21 Thread Shafik Yaghmour via cfe-commits


@@ -86,6 +86,23 @@ struct A {
 };
 }
 
+namespace cwg1458 { // cwg1458: 3.1
+#if __cplusplus >= 201103L
+struct A;
+
+void f() {
+  constexpr A* a = nullptr;
+  constexpr int p = &*a;
+  // expected-error@-1 {{cannot initialize a variable of type 'const int' with 
an rvalue of type 'A *'}}
+  constexpr A *p2 = &*a;

shafik wrote:

So the DR says it is "unspecified" so are we documenting here that this will 
always be the behavior? Maybe worth a comment?

https://github.com/llvm/llvm-project/pull/92113
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add tests for CWG issues regarding completeness of types (PR #92113)

2024-05-21 Thread Shafik Yaghmour via cfe-commits


@@ -451,6 +463,25 @@ static_assert(!__is_nothrow_constructible(D4, int), "");
 #endif
 } // namespace cwg1350
 
+namespace cwg1352 { // cwg1352: 3.0
+struct A {
+#if __cplusplus >= 201103L
+  int a = sizeof(A);

shafik wrote:

I think it might be worth it to see that this fails for a `static member` since 
the DR specifically says `non-static` so we should cover both.

https://github.com/llvm/llvm-project/pull/92113
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add tests for CWG issues regarding completeness of types (PR #92113)

2024-05-21 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/92113
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add tests for CWG issues regarding completeness of types (PR #92113)

2024-05-21 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM w/ a few nitpicks

https://github.com/llvm/llvm-project/pull/92113
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C] Disallow declarations where a statement is required (PR #92908)

2024-05-21 Thread Shafik Yaghmour via cfe-commits


@@ -467,15 +467,18 @@ class Parser : public CodeCompletionHandler {
 
   /// Flags describing a context in which we're parsing a statement.
   enum class ParsedStmtContext {
+/// This context permits declarations in language modes where declarations
+/// are not statements.
+AllowDeclarationsInC = 0x1,

shafik wrote:

This feels a little mysterious, I only see that we check it but it is not 
obvious how it gets set. 

https://github.com/llvm/llvm-project/pull/92908
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Fix an out-of-bounds crash when diagnosing bad conversion for a function with a parameter pack. (PR #92721)

2024-05-20 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Can you add some details to the summary. What was the original code doing wrong 
and the proposed new approach.

https://github.com/llvm/llvm-project/pull/92721
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Fix an out-of-bounds crash when diagnosing bad conversion for a function with a parameter pack. (PR #92721)

2024-05-20 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/92721
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Fix an out-of-bounds crash when diagnosing bad conversion for a function with a parameter pack. (PR #92721)

2024-05-20 Thread Shafik Yaghmour via cfe-commits


@@ -11298,8 +11298,14 @@ static void DiagnoseBadConversion(Sema , 
OverloadCandidate *Cand,
   Expr *FromExpr = Conv.Bad.FromExpr;
   QualType FromTy = Conv.Bad.getFromType();
   QualType ToTy = Conv.Bad.getToType();
-  SourceRange ToParamRange =
-  !isObjectArgument ? Fn->getParamDecl(I)->getSourceRange() : 
SourceRange();
+  SourceRange ToParamRange;
+  if (!isObjectArgument) {
+if (I < Fn->getNumParams())
+  ToParamRange = Fn->getParamDecl(I)->getSourceRange();
+else
+  // parameter pack case.

shafik wrote:

While this comment does the right thing™️ wrt to the `else` I would just use 
braces b/c it is not obvious and would probably give a lot of folks pause.

https://github.com/llvm/llvm-project/pull/92721
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] Disable compiling and testing Flang on Clang changes (PR #92740)

2024-05-20 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

This makes sense given the pain we are seeing here.

https://github.com/llvm/llvm-project/pull/92740
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn consecutive builtin comparisons in an expression (PR #92200)

2024-05-17 Thread Shafik Yaghmour via cfe-commits


@@ -36,7 +36,7 @@ namespace InExpr {
 
 // These are valid expressions.
 foo(0);
+foo(0); // expected-warning {{comparisons like 'X<=Y<=Z' don't have 
their mathematical meaning}}
 foo(false);

shafik wrote:

It is a shame we don't catch this one but neither does gcc

https://github.com/llvm/llvm-project/pull/92200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Implement CWG2428 "Deprecating a concept" (PR #92295)

2024-05-16 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.


https://github.com/llvm/llvm-project/pull/92295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Implement CWG2428 "Deprecating a concept" (PR #92295)

2024-05-16 Thread Shafik Yaghmour via cfe-commits


@@ -167,9 +167,11 @@ Parser::DeclGroupPtrTy 
Parser::ParseTemplateDeclarationOrSpecialization(
   LastParamListWasEmpty);
 
   // Parse the actual template declaration.
-  if (Tok.is(tok::kw_concept))
-return Actions.ConvertDeclToDeclGroup(
-ParseConceptDefinition(TemplateInfo, DeclEnd));
+  if (Tok.is(tok::kw_concept)) {
+Decl *ConceptDecl = ParseConceptDefinition(TemplateInfo, DeclEnd);
+ParsingTemplateParams.complete(ConceptDecl);

shafik wrote:

Curious, why do we need this?

https://github.com/llvm/llvm-project/pull/92295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] ASTContext::getUnconstrainedType propagates dependence (PR #92425)

2024-05-16 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template

shafik wrote:

Nitpick wrap this in `namespace GH77377` since this is a regression test from a 
bug report

https://github.com/llvm/llvm-project/pull/92425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] ASTContext::getUnconstrainedType propagates dependence (PR #92425)

2024-05-16 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/92425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] ASTContext::getUnconstrainedType propagates dependence (PR #92425)

2024-05-16 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/92425
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)

2024-05-14 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

Thank you for the additional test coverage, LGTM

https://github.com/llvm/llvm-project/pull/90500
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix malformed AST for anonymous class access in template. (PR #90842)

2024-05-02 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Note there is a `BuildAnonymousStructUnionMemberReference`, I am not sure it 
solves your problem.

https://github.com/llvm/llvm-project/pull/90842
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix malformed AST for anonymous class access in template. (PR #90842)

2024-05-02 Thread Shafik Yaghmour via cfe-commits


@@ -2876,10 +2876,21 @@ class TreeTransform {
 return ExprError();
   Base = BaseResult.get();
 
+  // We want to use `BuildMemberReferenceExpr()` so we can use its logic
+  // that materializes `Base` into a temporary if it's a prvalue.
+  // To do so, we need to create a `LookupResult` for `Member`, even though
+  // it's an unnamed field (that we could never actually have looked up).
+  // This small hack seems preferable to duplicating the logic for
+  // materializing the temporary.
+  LookupResult R(getSema(), MemberNameInfo, Sema::LookupMemberName);
+  R.addDecl(Member);
+  R.resolveKind();
+
   CXXScopeSpec EmptySS;
-  return getSema().BuildFieldReferenceExpr(
-  Base, isArrow, OpLoc, EmptySS, cast(Member),
-  DeclAccessPair::make(FoundDecl, FoundDecl->getAccess()), 
MemberNameInfo);
+  return getSema().BuildMemberReferenceExpr(
+  Base, Base->getType(), OpLoc, isArrow, EmptySS, TemplateKWLoc,
+  FirstQualifierInScope, R, ExplicitTemplateArgs,
+  /*S*/ nullptr);

shafik wrote:

```suggestion
  /*S=*/nullptr);
```
We use 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)
 format for these.

https://github.com/llvm/llvm-project/pull/90842
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix malformed AST for anonymous class access in template. (PR #90842)

2024-05-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Thank you for this fix.

You should reference the issue that this fixes in your summary.

This also need a release note.

https://github.com/llvm/llvm-project/pull/90842
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix malformed AST for anonymous class access in template. (PR #90842)

2024-05-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/90842
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] CTAD: fix the aggregate deduction guide for alias templates. (PR #90894)

2024-05-02 Thread Shafik Yaghmour via cfe-commits


@@ -2803,7 +2803,207 @@ getRHSTemplateDeclAndArgs(Sema , 
TypeAliasTemplateDecl *AliasTemplate) {
   return {Template, AliasRhsTemplateArgs};
 }
 
-// Build deduction guides for a type alias template.
+// Build deduction guides for a type alias template from the given underlying
+// deduction guide F.
+FunctionTemplateDecl *
+BuildDeductionGuideForTypeAlias(Sema ,
+TypeAliasTemplateDecl *AliasTemplate,
+FunctionTemplateDecl *F, SourceLocation Loc) {
+  LocalInstantiationScope Scope(SemaRef);
+  Sema::InstantiatingTemplate BuildingDeductionGuides(
+  SemaRef, AliasTemplate->getLocation(), F,
+  Sema::InstantiatingTemplate::BuildingDeductionGuidesTag{});
+  if (BuildingDeductionGuides.isInvalid())
+return nullptr;
+
+  auto  = SemaRef.Context;
+  auto [Template, AliasRhsTemplateArgs] =
+  getRHSTemplateDeclAndArgs(SemaRef, AliasTemplate);
+
+  auto RType = F->getTemplatedDecl()->getReturnType();
+  // The (trailing) return type of the deduction guide.
+  const TemplateSpecializationType *FReturnType =
+  RType->getAs();
+  if (const auto *InjectedCNT = RType->getAs())
+// implicitly-generated deduction guide.
+FReturnType = InjectedCNT->getInjectedTST();
+  else if (const auto *ET = RType->getAs())
+// explicit deduction guide.
+FReturnType = ET->getNamedType()->getAs();
+  assert(FReturnType && "expected to see a return type");
+  // Deduce template arguments of the deduction guide f from the RHS of
+  // the alias.
+  //
+  // C++ [over.match.class.deduct]p3: ...For each function or function
+  // template f in the guides of the template named by the
+  // simple-template-id of the defining-type-id, the template arguments
+  // of the return type of f are deduced from the defining-type-id of A
+  // according to the process in [temp.deduct.type] with the exception
+  // that deduction does not fail if not all template arguments are
+  // deduced.
+  //
+  //
+  //  template
+  //  f(X, Y) -> f;
+  //
+  //  template
+  //  using alias = f;
+  //
+  // The RHS of alias is f, we deduced the template arguments of
+  // the return type of the deduction guide from it: Y->int, X->U
+  sema::TemplateDeductionInfo TDeduceInfo(Loc);
+  // Must initialize n elements, this is required by DeduceTemplateArguments.
+  SmallVector DeduceResults(
+  F->getTemplateParameters()->size());
+
+  // FIXME: DeduceTemplateArguments stops immediately at the first
+  // non-deducible template argument. However, this doesn't seem to casue
+  // issues for practice cases, we probably need to extend it to continue
+  // performing deduction for rest of arguments to align with the C++
+  // standard.
+  SemaRef.DeduceTemplateArguments(
+  F->getTemplateParameters(), FReturnType->template_arguments(),
+  AliasRhsTemplateArgs, TDeduceInfo, DeduceResults,
+  /*NumberOfArgumentsMustMatch=*/false);
+
+  SmallVector DeducedArgs;
+  SmallVector NonDeducedTemplateParamsInFIndex;
+  // !!NOTE: DeduceResults respects the sequence of template parameters of
+  // the deduction guide f.
+  for (unsigned Index = 0; Index < DeduceResults.size(); ++Index) {
+if (const auto  = DeduceResults[Index]; !D.isNull()) // Deduced
+  DeducedArgs.push_back(D);
+else
+  NonDeducedTemplateParamsInFIndex.push_back(Index);
+  }
+  auto DeducedAliasTemplateParams =
+  TemplateParamsReferencedInTemplateArgumentList(
+  AliasTemplate->getTemplateParameters()->asArray(), DeducedArgs);
+  // All template arguments null by default.
+  SmallVector TemplateArgsForBuildingFPrime(
+  F->getTemplateParameters()->size());
+
+  // Create a template parameter list for the synthesized deduction guide f'.
+  //
+  // C++ [over.match.class.deduct]p3.2:
+  //   If f is a function template, f' is a function template whose template
+  //   parameter list consists of all the template parameters of A
+  //   (including their default template arguments) that appear in the above
+  //   deductions or (recursively) in their default template arguments
+  SmallVector FPrimeTemplateParams;
+  // Store template arguments that refer to the newly-created template
+  // parameters, used for building `TemplateArgsForBuildingFPrime`.
+  SmallVector TransformedDeducedAliasArgs(
+  AliasTemplate->getTemplateParameters()->size());
+
+  for (unsigned AliasTemplateParamIdx : DeducedAliasTemplateParams) {
+auto *TP =
+
AliasTemplate->getTemplateParameters()->getParam(AliasTemplateParamIdx);
+// Rebuild any internal references to earlier parameters and reindex as
+// we go.
+MultiLevelTemplateArgumentList Args;
+Args.setKind(TemplateSubstitutionKind::Rewrite);
+Args.addOuterTemplateArguments(TransformedDeducedAliasArgs);
+NamedDecl *NewParam = transformTemplateParameter(
+SemaRef, AliasTemplate->getDeclContext(), TP, Args,
+/*NewIndex*/ FPrimeTemplateParams.size());


[clang] [clang] CTAD: fix the aggregate deduction guide for alias templates. (PR #90894)

2024-05-02 Thread Shafik Yaghmour via cfe-commits


@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -verify -ast-dump -ast-dump-decl-types 
-ast-dump-filter "deduction guide" %s | FileCheck %s --strict-whitespace
+// RUN: %clang_cc1 -std=c++2a -verify -ast-dump -ast-dump-decl-types 
-Wno-c++11-narrowing -ast-dump-filter "deduction guide" %s | FileCheck %s 
--strict-whitespace

shafik wrote:

Is there a way to verify this fix w/o using a narrowing conversion?

https://github.com/llvm/llvm-project/pull/90894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)

2024-04-30 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> @cor3ntin @shafik Hi, I want to take charge of this issue and submit a PR for 
> the fix.

I would open a new PR and reference this one in the summary for completeness. 
It looks like this one is not going to picked up by the author and so if you 
can take it over and finish it that would be very much appreciated. 

There are a lot of issues that crash similarly and so we likely have plenty of 
tests to try against a fix :-)

https://github.com/llvm/llvm-project/pull/78598
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Implement CWG2851: floating-point conversions in converted constant expressions (PR #90387)

2024-04-30 Thread Shafik Yaghmour via cfe-commits


@@ -67,6 +68,69 @@ void B::g() requires true;
 
 } // namespace cwg2847
 
+namespace cwg2851 { // cwg2851: 19
+
+#if __cplusplus >= 202002L
+template struct Val { static constexpr T value = v; };
+
+
+// Floating-point promotions
+
+static_assert(Val::value == 0.0L);
+static_assert(Val::value == 0.0L);
+static_assert(Val::value == 0.0);
+static_assert(Val::value == -0.0L);
+
+static_assert(!__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val(__builtin_nanf(""))>));
+static_assert(__is_same(Val, Val(__builtin_nansf(""))>));
+static_assert(__is_same(Val, Val(__builtin_nanf("0x1"))>));
+static_assert(__is_same(Val, Val(__builtin_nansf("0x1"))>));
+
+
+// Floating-point conversions where the source value can be represented 
exactly in the destination type
+
+static_assert(Val::value == 0.0L);
+static_assert(__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+static_assert(!__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val));
+Val _1;
+// since-cxx20-error-re@-1 {{non-type template argument evaluates to {{.+}} 
which cannot be exactly represented in type 'float'}}
+Val(__FLT_DENORM_MIN__) / 2.0L> _2;
+// since-cxx20-error-re@-1 {{non-type template argument evaluates to {{.+}} 
which cannot be exactly represented in type 'float'}}
+Val _3;
+// since-cxx20-error-re@-1 {{non-type template argument evaluates to {{.+}} 
which cannot be exactly represented in type 'float'}}
+
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val(__builtin_nanl(""))>));
+static_assert(__is_same(Val, Val(__builtin_nansl(""))>));
+#if __SIZEOF_LONG_DOUBLE__ > 8
+// since-cxx20-error@-2 {{non-type template argument evaluates to nan which 
cannot be exactly represented in type 'float'}}
+#endif
+// Payload is shifted right so these payloads will be preserved
+static_assert(__is_same(Val, Val(__builtin_nan("0xFF"))>));
+static_assert(__is_same(Val, Val(__builtin_nans("0xFF"))>));
+static_assert(__is_same(Val, Val(__builtin_nanl("0x1"))>));

shafik wrote:

I discussed this with @jensmaurer offline and he felt similarly as well. So 
let's go with that for now but I will make sure we discuss that next we discuss 
cwg2864.

https://github.com/llvm/llvm-project/pull/90387
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Implement CWG2851: floating-point conversions in converted constant expressions (PR #90387)

2024-04-29 Thread Shafik Yaghmour via cfe-commits


@@ -67,6 +68,69 @@ void B::g() requires true;
 
 } // namespace cwg2847
 
+namespace cwg2851 { // cwg2851: 19
+
+#if __cplusplus >= 202002L
+template struct Val { static constexpr T value = v; };
+
+
+// Floating-point promotions
+
+static_assert(Val::value == 0.0L);
+static_assert(Val::value == 0.0L);
+static_assert(Val::value == 0.0);
+static_assert(Val::value == -0.0L);
+
+static_assert(!__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val(__builtin_nanf(""))>));
+static_assert(__is_same(Val, Val(__builtin_nansf(""))>));
+static_assert(__is_same(Val, Val(__builtin_nanf("0x1"))>));
+static_assert(__is_same(Val, Val(__builtin_nansf("0x1"))>));
+
+
+// Floating-point conversions where the source value can be represented 
exactly in the destination type
+
+static_assert(Val::value == 0.0L);
+static_assert(__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+static_assert(!__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val));
+Val _1;
+// since-cxx20-error-re@-1 {{non-type template argument evaluates to {{.+}} 
which cannot be exactly represented in type 'float'}}
+Val(__FLT_DENORM_MIN__) / 2.0L> _2;
+// since-cxx20-error-re@-1 {{non-type template argument evaluates to {{.+}} 
which cannot be exactly represented in type 'float'}}
+Val _3;
+// since-cxx20-error-re@-1 {{non-type template argument evaluates to {{.+}} 
which cannot be exactly represented in type 'float'}}
+
+static_assert(__is_same(Val, Val));
+
+static_assert(__is_same(Val, Val(__builtin_nanl(""))>));
+static_assert(__is_same(Val, Val(__builtin_nansl(""))>));
+#if __SIZEOF_LONG_DOUBLE__ > 8
+// since-cxx20-error@-2 {{non-type template argument evaluates to nan which 
cannot be exactly represented in type 'float'}}
+#endif
+// Payload is shifted right so these payloads will be preserved
+static_assert(__is_same(Val, Val(__builtin_nan("0xFF"))>));
+static_assert(__is_same(Val, Val(__builtin_nans("0xFF"))>));
+static_assert(__is_same(Val, Val(__builtin_nanl("0x1"))>));

shafik wrote:

Why does `nanl` fail but `nans` does not? I am not sure this is consistent with 
[CWG2864](https://cplusplus.github.io/CWG/issues/2864.html), which is not final 
yet either.

https://github.com/llvm/llvm-project/pull/90387
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [SemaCXX] Implement CWG2351 `void{}` (PR #78060)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

@MitalAshok ping 

https://github.com/llvm/llvm-project/pull/78060
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Prevent null pointer dereference in Sema::​CodeCompleteQualifiedId() (PR #90490)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Looks good, I will let Tom make the final accept.

https://github.com/llvm/llvm-project/pull/90490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Prevent null pointer dereference in Sema::​CodeCompleteQualifiedId() (PR #90490)

2024-04-29 Thread Shafik Yaghmour via cfe-commits


@@ -6714,14 +6714,16 @@ void Sema::CodeCompleteQualifiedId(Scope *S, 
CXXScopeSpec ,
 
   // If the scope is a concept-constrained type parameter, infer nested
   // members based on the constraints.
-  if (const auto *TTPT =
-  dyn_cast_or_null(NNS->getAsType())) {
-for (const auto  : ConceptInfo(*TTPT, S).members()) {
-  if (R.Operator != ConceptInfo::Member::Colons)
-continue;
-  Results.AddResult(CodeCompletionResult(
-  R.render(*this, CodeCompleter->getAllocator(),
-   CodeCompleter->getCodeCompletionTUInfo(;
+  if (NNS) {

shafik wrote:

This looks good, We can even seen above `NNS` is checked before access. This 
looks like an unfortunate oversight.

https://github.com/llvm/llvm-project/pull/90490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Prevent null pointer dereference in Sema::​CodeCompleteQualifiedId() (PR #90490)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/90490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

So this seems to apply to member pointer types, pointer types, arrays and 
function types but I don't see coverage for the full set of paths in the tests. 
Can we please add more testing.

Erich also expressed some concern about breaking existing code due to eager 
instantiation. Can we make sure try to cover any change in behavior in the 
tests so reviewers can get a better idea of impact.

https://github.com/llvm/llvm-project/pull/90500
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][NFC] Reformat suspicious condition (PR #89923)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

The `amdgpu-toolchain.c` test failure looks unrelated. I think we need another 
empty commit to kick off the build again unfortunately.

https://github.com/llvm/llvm-project/pull/89923
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [NFC] Fix hasQualifier comment (PR #90485)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

LGTM, thank you for the documentation fix.

https://github.com/llvm/llvm-project/pull/90485
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread Shafik Yaghmour via cfe-commits


@@ -908,6 +908,74 @@ void CodeGenFunction::EmitIfStmt(const IfStmt ) {
 incrementProfileCounter();
 }
 
+bool CodeGenFunction::checkIfLoopMustProgress(const Expr 
*ControllingExpression,
+  bool IsTrivialCXXLoop) {
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Always)
+return true;
+  if (CGM.getCodeGenOpts().getFiniteLoops() ==
+  CodeGenOptions::FiniteLoopsKind::Never)
+return false;
+
+  // Now apply rules for plain C (see  6.8.5.6 in C11).
+  // Loops with constant conditions do not have to make progress in any C
+  // version.
+  // As an extension, we consisider loops whose constant expression
+  // can be constant-folded.
+  Expr::EvalResult Result;
+  bool CondIsConstInt =
+  !ControllingExpression ||
+  (ControllingExpression->EvaluateAsInt(Result, getContext()) &&
+   Result.Val.isInt());
+
+  bool CondIsTrue = CondIsConstInt &&
+(!ControllingExpression || Result.Val.getInt().getBoolValue());
+
+  if (getLangOpts().C99 && CondIsConstInt)
+return false;
+
+  // Loops with non-constant conditions must make progress in C11 and later.
+  if (getLangOpts().C11)
+return true;
+
+  // [C++26][intro.progress] (DR)
+  // The implementation may assume that any thread will eventually do one of 
the
+  // following:
+  // [...]
+  // - continue execution of a trivial infinite loop ([stmt.iter.general]).
+  if (getLangOpts().CPlusPlus11) {
+if (IsTrivialCXXLoop && CondIsTrue) {
+  CurFn->removeFnAttr(llvm::Attribute::MustProgress);
+  return false;
+}
+return true;
+  }
+
+  return false;
+}
+
+// [C++26][stmt.iter.general] (DR)
+// A trivially empty iteration statement is an iteration statement matching one
+// of the following forms:
+//  - while ( expression ) ;
+//  - while ( expression ) { }
+//  - do ; while ( expression ) ;
+//  - do { } while ( expression ) ;
+//  - for ( init-statement expression(opt); ) ;
+//  - for ( init-statement expression(opt); ) { }
+template  static bool hasEmptyLoopBody(const LoopStmt ) {
+  if constexpr (std::is_same_v) {
+if (S.getInc())
+  return false;
+  }
+  const Stmt *Body = S.getBody();
+  if (!Body || isa(Body))
+return true;
+  if (const CompoundStmt *Compound = dyn_cast(Body))

shafik wrote:

It does not look like the test cover the `CompoundStmt` case.

https://github.com/llvm/llvm-project/pull/90066
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (PR #90066)

2024-04-29 Thread Shafik Yaghmour via cfe-commits


@@ -1465,6 +1465,7 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, 
llvm::Function *Fn,
 
   // Ensure that the function adheres to the forward progress guarantee, which
   // is required by certain optimizations.
+  // The attribute will be removed if the body contains a trivial empty loop.

shafik wrote:

```suggestion
  // In C++11 and forward, the attribute will be removed if the body contains a 
trivial empty loop.
```

https://github.com/llvm/llvm-project/pull/90066
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] MangledSymbol: remove pointless copy of vector (PR #90012)

2024-04-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> Looking at the logs, and the error seems to be unrelated to the changes made 
> https://buildkite.com/llvm-project/clang-ci/builds/16430#018f132d-506e-440c-b18b-fed98237def9/54-5446

You can try using `--allow-empty` to do an empty commit to kick off the build 
again.

https://github.com/llvm/llvm-project/pull/90012
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][NFC] Reformat suspicious condition (PR #89923)

2024-04-24 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

This makes sense, I added Nico since they added the change that brought in that 
line.

https://github.com/llvm/llvm-project/pull/89923
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] MangledSymbol: remove pointless copy of vector (PR #90012)

2024-04-24 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/90012
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Reject VLAs in `__is_layout_compatible()` (PR #87737)

2024-04-05 Thread Shafik Yaghmour via cfe-commits


@@ -1741,8 +1741,10 @@ void is_layout_compatible(int n)
   static_assert(!__is_layout_compatible(unsigned char, signed char));
   static_assert(__is_layout_compatible(int[], int[]));
   static_assert(__is_layout_compatible(int[2], int[2]));
-  static_assert(!__is_layout_compatible(int[n], int[2])); // FIXME: VLAs 
should be rejected
-  static_assert(!__is_layout_compatible(int[n], int[n])); // FIXME: VLAs 
should be rejected
+  static_assert(!__is_layout_compatible(int[n], int[2]));
+  // expected-error@-1 {{variable length arrays are not supported for 
'__is_layout_compatible'}}
+  static_assert(!__is_layout_compatible(int[n], int[n]));

shafik wrote:

I think focusing on VLAs here is fine, that was the original scope and I don't 
see an urgent need to expand it here.

https://github.com/llvm/llvm-project/pull/87737
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix the lambda call expression inside of a type alias declaration (PR #82310)

2024-04-04 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

ping @cor3ntin 

https://github.com/llvm/llvm-project/pull/82310
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][ExprConst] allow single element access of vector object to be constant expression (PR #72607)

2024-04-04 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

This mostly makes sense to me, @AaronBallman does this look good to you?

https://github.com/llvm/llvm-project/pull/72607
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Disable missing definition warning on pure virtual functions (PR #74510)

2024-04-04 Thread Shafik Yaghmour via cfe-commits


@@ -18931,7 +18931,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, 
FunctionDecl *Func,
   //   constant evaluated
   bool NeededForConstantEvaluation =
   isPotentiallyConstantEvaluatedContext(*this) &&
-  isImplicitlyDefinableConstexprFunction(Func);
+  isImplicitlyDefinableConstexprFunction(Func) && !Func->isPure();

shafik wrote:

I am not sure why moving it down silences the diagnostic, I would think doing 
`(OdrUse == OdrUseContext::Used || NeededForConstantEvaluation || 
!Func->isPure())` should work. 

Can you explain in more detail?

https://github.com/llvm/llvm-project/pull/74510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Disable missing definition warning on pure virtual functions (PR #74510)

2024-04-04 Thread Shafik Yaghmour via cfe-commits


@@ -18931,7 +18931,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, 
FunctionDecl *Func,
   //   constant evaluated
   bool NeededForConstantEvaluation =
   isPotentiallyConstantEvaluatedContext(*this) &&
-  isImplicitlyDefinableConstexprFunction(Func);
+  isImplicitlyDefinableConstexprFunction(Func) && !Func->isPure();

shafik wrote:

Thank to @erichkeane I realized I am wrong, we can actually define a pure 
virtual function. So we do want to retain the diagnostic here.

https://github.com/llvm/llvm-project/pull/74510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Disable missing definition warning on pure virtual functions (PR #74510)

2024-04-04 Thread Shafik Yaghmour via cfe-commits


@@ -18931,7 +18931,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, 
FunctionDecl *Func,
   //   constant evaluated
   bool NeededForConstantEvaluation =
   isPotentiallyConstantEvaluatedContext(*this) &&
-  isImplicitlyDefinableConstexprFunction(Func);
+  isImplicitlyDefinableConstexprFunction(Func) && !Func->isPure();

shafik wrote:

I am not sure the diagnostic makes sense in that case since it can never 
actually be called. 

https://github.com/llvm/llvm-project/pull/74510
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang]Treat arguments to builtin type traits as template type arguments (PR #87132)

2024-04-01 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM but I want @fahadnayyar to verify this addresses his concerns.

https://github.com/llvm/llvm-project/pull/87132
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Skip checking anonymous enum in using enum declaration (PR #87144)

2024-04-01 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM after addressing Aaron's comments.

Can you elaborate more on the details of the bug in the summary. This goes into 
the git log and we want folks to be able to understand the problem well from 
the summary w/o having to do additional checks.

Thank you

https://github.com/llvm/llvm-project/pull/87144
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] set declaration invalid earlier to prevent crash in calculating record layout (PR #87173)

2024-04-01 Thread Shafik Yaghmour via cfe-commits


@@ -3899,6 +3899,9 @@ static QualType 
GetDeclSpecTypeForDeclarator(TypeProcessingState ,
   SemaRef.Diag(OwnedTagDecl->getLocation(), DiagID)
   << SemaRef.Context.getTypeDeclType(OwnedTagDecl);
   D.setInvalidType(true);
+  OwnedTagDecl->setCompleteDefinition(false);
+  OwnedTagDecl->setInvalidDecl();
+  OwnedTagDecl->setCompleteDefinition();

shafik wrote:

`GetDeclSpecTypeForDeclarator` is called from two places 
`GetTypeForDeclaratorCast(...)` and where that is called `D.isInvalidType()` is 
checked. It is also called from `GetTypeForDeclarator(...)` and the checking of 
after that call is a bit more varied. I am wondering if we are being sloppy in 
one of those calls. 

https://github.com/llvm/llvm-project/pull/87173
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] set declaration invalid earlier to prevent crash in calculating record layout (PR #87173)

2024-04-01 Thread Shafik Yaghmour via cfe-commits


@@ -3899,6 +3899,9 @@ static QualType 
GetDeclSpecTypeForDeclarator(TypeProcessingState ,
   SemaRef.Diag(OwnedTagDecl->getLocation(), DiagID)
   << SemaRef.Context.getTypeDeclType(OwnedTagDecl);
   D.setInvalidType(true);
+  OwnedTagDecl->setCompleteDefinition(false);
+  OwnedTagDecl->setInvalidDecl();
+  OwnedTagDecl->setCompleteDefinition();

shafik wrote:

I am little uncomfortable with this change here, I see that you are calling 
`setCompleteDefinition(false)` in order to get around the `assert` that the 
decl is not complete in `Decl::setInvalidDecl(...)`. If this precondition is 
not true we need to understand why it break down or if this implies we should 
be handling this differently.

https://github.com/llvm/llvm-project/pull/87173
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add test for CWG1606 (PR #87274)

2024-04-01 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

It looks like we do have a test and it looks like the restriction was lifted in 
C++20.

https://github.com/llvm/llvm-project/pull/87274
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add test for CWG1606 (PR #87274)

2024-04-01 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

Thank you for the test! LGTM.

Do we also have a test that `sizeof([=]{ return i + j;})` should fail as well? 
Tangentially related to this DR but if we don't we should cover that in our 
tests someplace.

https://github.com/llvm/llvm-project/pull/87274
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Bailout when the substitution of template parameter mapping is invalid. (PR #86869)

2024-03-27 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

LGTM, interesting it looks like we don't do this check in `fromContraintExpr` 
either.

https://github.com/llvm/llvm-project/pull/86869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Invalidate the alias template decl if it has multiple written (PR #85413)

2024-03-27 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/85413
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Fix an out-of-bound crash when checking template partial specializations. (PR #86794)

2024-03-27 Thread Shafik Yaghmour via cfe-commits


@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -ferror-limit 0 -verify %s

shafik wrote:

Why the `ferror-limit 0`?

https://github.com/llvm/llvm-project/pull/86794
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] CTAD: Track template template type parameters that referenced in (PR #85405)

2024-03-27 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM, I will let @ilya-biryukov approve

https://github.com/llvm/llvm-project/pull/85405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Ignore assumptions with side effects at compile time (PR #85534)

2024-03-16 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

This is not really an NFC change so you should have waited for an approval. 
This LGTM

https://github.com/llvm/llvm-project/pull/85534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Sema] Allow -Wno-main to suppress the arg wrong error (PR #85494)

2024-03-16 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

I believe this should have a release note.

https://github.com/llvm/llvm-project/pull/85494
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] nolock/noalloc attributes (PR #84983)

2024-03-13 Thread Shafik Yaghmour via cfe-commits


@@ -4181,6 +4185,127 @@ class FunctionNoProtoType : public FunctionType, public 
llvm::FoldingSetNode {
   }
 };
 
+class FunctionEffect;
+class FunctionEffectSet;
+
+// It is the user's responsibility to keep this in set form: elements are
+// ordered and unique.
+// We could hide the mutating methods which are capable of breaking the
+// invariant, but they're needed and safe when used with STL set algorithms.
+class MutableFunctionEffectSet : public SmallVector 
{
+public:
+  using SmallVector::insert;
+  using SmallVector::SmallVector;
+
+  /// Maintains order/uniquenesss.
+  void insert(const FunctionEffect *effect);
+
+  MutableFunctionEffectSet |=(FunctionEffectSet rhs);
+};
+
+class FunctionEffectSet {
+public:
+  // These sets will tend to be very small (1 element), so represent them as
+  // sorted vectors, which are compatible with the STL set algorithms. Using an
+  // array or vector also means the elements are contiguous, keeping iterators
+  // simple.
+
+private:
+  // 'Uniqued' refers to the set itself being uniqued. Storage is allocated
+  // separately. Use ArrayRef for its iterators. Subclass so as to be able to
+  // compare (it seems ArrayRef would silently convert itself to a vector for
+  // comparison?!).
+  class UniquedAndSortedFX : public llvm::ArrayRef {
+  public:
+using Base = llvm::ArrayRef;
+
+UniquedAndSortedFX(Base Array) : Base(Array) {}
+UniquedAndSortedFX(const FunctionEffect **Ptr, size_t Len)
+: Base(Ptr, Len) {}
+
+bool operator<(const UniquedAndSortedFX ) const;
+  };
+
+  // Could have used a TinyPtrVector if it were unique-able.
+  // Empty set has a null Impl.
+  llvm::PointerUnion Impl;
+
+  explicit FunctionEffectSet(const FunctionEffect *Single) : Impl(Single) {}
+  explicit FunctionEffectSet(const UniquedAndSortedFX *Multi) : Impl(Multi) {}
+
+public:
+  using Differences =
+  SmallVector>;

shafik wrote:

This is consistent with 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)
 which we use. I don't think it catches this case but consistency is nice.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] nolock/noalloc attributes (PR #84983)

2024-03-13 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Quick drive by comment

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] nolock/noalloc attributes (PR #84983)

2024-03-13 Thread Shafik Yaghmour via cfe-commits


@@ -4181,6 +4185,127 @@ class FunctionNoProtoType : public FunctionType, public 
llvm::FoldingSetNode {
   }
 };
 
+class FunctionEffect;
+class FunctionEffectSet;
+
+// It is the user's responsibility to keep this in set form: elements are
+// ordered and unique.
+// We could hide the mutating methods which are capable of breaking the
+// invariant, but they're needed and safe when used with STL set algorithms.
+class MutableFunctionEffectSet : public SmallVector 
{
+public:
+  using SmallVector::insert;
+  using SmallVector::SmallVector;
+
+  /// Maintains order/uniquenesss.
+  void insert(const FunctionEffect *effect);
+
+  MutableFunctionEffectSet |=(FunctionEffectSet rhs);
+};
+
+class FunctionEffectSet {
+public:
+  // These sets will tend to be very small (1 element), so represent them as
+  // sorted vectors, which are compatible with the STL set algorithms. Using an
+  // array or vector also means the elements are contiguous, keeping iterators
+  // simple.
+
+private:
+  // 'Uniqued' refers to the set itself being uniqued. Storage is allocated
+  // separately. Use ArrayRef for its iterators. Subclass so as to be able to
+  // compare (it seems ArrayRef would silently convert itself to a vector for
+  // comparison?!).
+  class UniquedAndSortedFX : public llvm::ArrayRef {
+  public:
+using Base = llvm::ArrayRef;
+
+UniquedAndSortedFX(Base Array) : Base(Array) {}
+UniquedAndSortedFX(const FunctionEffect **Ptr, size_t Len)
+: Base(Ptr, Len) {}
+
+bool operator<(const UniquedAndSortedFX ) const;
+  };
+
+  // Could have used a TinyPtrVector if it were unique-able.
+  // Empty set has a null Impl.
+  llvm::PointerUnion Impl;
+
+  explicit FunctionEffectSet(const FunctionEffect *Single) : Impl(Single) {}
+  explicit FunctionEffectSet(const UniquedAndSortedFX *Multi) : Impl(Multi) {}
+
+public:
+  using Differences =
+  SmallVector>;

shafik wrote:

```suggestion
  SmallVector>;
```

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] nolock/noalloc attributes (PR #84983)

2024-03-13 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Diagnose misuse of the cleanup attribute (PR #80040)

2024-03-12 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

It looks like it passed on your last commit but you have a conflict now which 
you need to resolve.

Can you merge or do you need help with that?

https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix type of enumerators in incomplete enumerations (PR #84068)

2024-03-11 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> @shafik We dont have a dedicated cpp test for this. I can add one if you 
> want, but clang/test/Sema/warn-compare-enum-types-mismatch.c runs clang both 
> on C and C++ mode, so I didnt think it necessary.

I think we just a bug that demonstrates this issue: 
https://github.com/llvm/llvm-project/issues/84712

So if this fixes that as well then we should add tests for those cases too.

https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Refine unused-member-function diagnostic message for constructors (PR #84515)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Looks like the build failed b/c you did not run `git clang-format`

https://github.com/llvm/llvm-project/pull/84515
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

2024-03-08 Thread Shafik Yaghmour via cfe-commits


@@ -596,8 +596,21 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
 
   // Add implicit object parameter.
   if (auto *MD = dyn_cast(FD)) {
-if (MD->isImplicitObjectMemberFunction() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = ActOnCXXThis(Loc);
+if (MD->isImplicitObjectMemberFunction()) {
+  ExprResult ThisExpr{};
+
+  if (isLambdaCallOperator(MD) && !MD->isStatic()) {
+Qualifiers ThisQuals = MD->getMethodQualifiers();
+CXXRecordDecl *Record = MD->getParent();
+
+Sema::CXXThisScopeRAII ThisScope(*this, Record, ThisQuals,
+ Record != nullptr);

shafik wrote:

```suggestion
 /*Enabled=*/Record != nullptr);
```

https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

nitpick

https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20][Coroutines] lambda-coroutine with promise_type ctor. (PR #84519)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/84519
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Update missing varargs arg extension warnings (PR #84520)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/84520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Update missing varargs arg extension warnings (PR #84520)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Just a nit here.

https://github.com/llvm/llvm-project/pull/84520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] Update missing varargs arg extension warnings (PR #84520)

2024-03-08 Thread Shafik Yaghmour via cfe-commits


@@ -993,11 +993,18 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token 
,
   // If the macro contains the comma pasting extension, the diagnostic
   // is suppressed; we know we'll get another diagnostic later.
   if (!MI->hasCommaPasting()) {
-// C++20 allows this construct, but standards before C++20 and all C
-// standards do not allow the construct (we allow it as an extension).
-Diag(Tok, getLangOpts().CPlusPlus20
-  ? diag::warn_cxx17_compat_missing_varargs_arg
-  : diag::ext_missing_varargs_arg);
+// C++20 and C23 allow this construct, but standards before that
+// do not (we allow it as an extension).

shafik wrote:

CC @AaronBallman should we add a standard quote like we normally do for C++? It 
looks like it should be 6.10.5 p12. It took me a but to remember in C they are 
referred to as *variable arguments* and not variadic like in C++. 

https://github.com/llvm/llvm-project/pull/84520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix type of enumerators in incomplete enumerations (PR #84068)

2024-03-08 Thread Shafik Yaghmour via cfe-commits


@@ -6,7 +6,9 @@ typedef enum EnumA {
 } EnumA;
 
 enum EnumB {
-  B
+  B,

shafik wrote:

I think what I was asking, was do we have an equivalent C++ test that verifies 
in a `.cpp` file that we also do not obtain a diagnostic for this.

https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix type of enumerators in incomplete enumerations (PR #84068)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> So I believe:
> 
> ```
> typedef enum EnumA {
>   A
> } EnumA;
> 
> enum EnumB {
>   B,
>   B1 = 1,
>   B2 = A == B1
> };
> ```
> 
> is not an enum compare warning in C++ because `B1` doesn't have an 
> enumeration type due to the enumeration not being fully-defined, and is not 
> an enum compare warning in C because `A` has type `int` (due to p15) and `B1` 
> has type `int` (due to p12).

Right, so do we have a test for C++ that verifies that.

https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Error on explicit specialization of lambda call operator (PR #84343)

2024-03-08 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Can we please get more descriptive summaries in the future. We should at a 
minimum state 1) what the underlying problem is 2) what the approach for fixing 
the problem is.

In this case the title describes the problem but it would be nice to have a 
summary of the fix as well. So folks reading the git log do not have to do an 
extra step to understand what the fix was at a high level.

https://github.com/llvm/llvm-project/pull/84343
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix type of enumerators in incomplete enumerations (PR #84068)

2024-03-07 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Should we also have a C++ test for this fix?

https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Factor out OpenACC part of `Sema` (PR #84184)

2024-03-07 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Factor out OpenACC part of `Sema` (PR #84184)

2024-03-07 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

I think this makes sense.

https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Factor out OpenACC part of `Sema` (PR #84184)

2024-03-07 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file declares semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_SEMA_SEMAOPENACC_H
+#define LLVM_CLANG_SEMA_SEMAOPENACC_H
+
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/OpenACCKinds.h"

shafik wrote:

I would have thought we could remove this include from `Sema.h` above.

https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [Sema] No longer diagnose type definitions in `offsetof` in C23 mode (PR #84169)

2024-03-07 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Should we verify that we diagnose the case where the definition includes a 
comma?

https://github.com/llvm/llvm-project/pull/84169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

2024-03-06 Thread Shafik Yaghmour via cfe-commits


@@ -1434,13 +1434,18 @@ ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
 return Diag(Loc, diag::err_invalid_this_use) << 0;
   }
 
-  return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
+  return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false,
+  SkipLambdaCaptureCheck);
 }
 
-Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
- bool IsImplicit) {
+Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool 
IsImplicit,
+ bool SkipLambdaCaptureCheck) {
   auto *This = CXXThisExpr::Create(Context, Loc, Type, IsImplicit);
-  MarkThisReferenced(This);
+
+  if (!SkipLambdaCaptureCheck) {

shafik wrote:

Can we provide a standard reference for this code that justifies why we skip 
this check here?

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [C++20][Coroutines] Lambda-coroutine with operator new in promise_type (PR #84193)

2024-03-06 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -I%S/Inputs -std=c++20 %s
+
+// expected-no-diagnostics
+
+#include "std-coroutine.h"
+
+using size_t = decltype(sizeof(0));
+
+struct Generator {
+  struct promise_type {
+int _val{};
+
+Generator get_return_object() noexcept
+{
+  return {};
+}
+
+std::suspend_never initial_suspend() noexcept
+{
+  return {};
+}
+
+std::suspend_always final_suspend() noexcept
+{
+  return {};
+}
+
+void return_void() noexcept {}
+void unhandled_exception() noexcept {}
+
+template
+static void*
+operator new(size_t size,
+ This&,
+ TheRest&&...) noexcept
+{
+return nullptr;
+}
+
+static void operator delete(void*, size_t)
+{
+}
+  };
+};
+
+int main()
+{
+  auto lamb = []() -> Generator {

shafik wrote:

What should happen if the lambda has a capture? Is it allowed?

https://github.com/llvm/llvm-project/pull/84193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add some CodeGen tests for CWG 4xx issues (PR #83715)

2024-03-06 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Late review but LGTM

https://github.com/llvm/llvm-project/pull/83715
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Sequence C++20 Parenthesized List Init (PR #83476)

2024-03-04 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.


https://github.com/llvm/llvm-project/pull/83476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Fix null-deref thanks to an attribute on a global declarator chunk (PR #83611)

2024-03-04 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.


https://github.com/llvm/llvm-project/pull/83611
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread Shafik Yaghmour via cfe-commits


@@ -5042,7 +5042,7 @@ void RecordDecl::completeDefinition() {
 
   // Layouts are dumped when computed, so if we are dumping for all complete
   // types, we need to force usage to get types that wouldn't be used 
elsewhere.
-  if (Ctx.getLangOpts().DumpRecordLayoutsComplete)
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete && !isDependentType())

shafik wrote:

Can you update the comment above to reflect this change.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

I think this makes sense but I would like another set of eyes.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][AST] fix dereference on class/struct layouts check. (PR #83686)

2024-03-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/83686
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][AST] fix dereference on class/struct layouts check. (PR #83686)

2024-03-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/83686
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][AST] fix dereference on class/struct layouts check. (PR #83686)

2024-03-02 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik requested changes to this pull request.

We need a minimal reproducer here. Looking at the bug report it is not clear to 
me if this is the correct fix or not. After we have a reproducer we would need 
a test added to the PR and a release note.

https://github.com/llvm/llvm-project/pull/83686
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Fix null-deref thanks to an attribute on a global declarator chunk (PR #83611)

2024-03-01 Thread Shafik Yaghmour via cfe-commits


@@ -100,6 +100,12 @@ void AttributePool::takePool(AttributePool ) {
   pool.Attrs.clear();
 }
 
+void AttributePool::takeFrom(ParsedAttributesView , AttributePool ) {
+  assert( != this && "AttributePool can't take attributes from itself");
+  llvm::for_each(List.AttrList, [](ParsedAttr *A) { Pool.remove(A); });

shafik wrote:

Should we remove from the pool after we added them to the `Attrs`?

https://github.com/llvm/llvm-project/pull/83611
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Fix null-deref thanks to an attribute on a global declarator chunk (PR #83611)

2024-03-01 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Note, I opened an issue for this here: 
https://github.com/llvm/llvm-project/issues/83385

https://github.com/llvm/llvm-project/pull/83611
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Clang][Sema] Fix incorrect rejection default construction of union with nontrivial member (PR #82407)

2024-02-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik updated 
https://github.com/llvm/llvm-project/pull/82407

>From 5fcaeaddccc0f7e370bf7bebce113d8d52e1b1bd Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Tue, 20 Feb 2024 11:22:39 -0800
Subject: [PATCH] [Clang][Sema] Fix incorrect rejection default construction of
 union with nontrivial member

In 765d8a192180f8f33618087b15c022fe758044af we impelemented a fix for incorrect 
deletion of
default constructors in unions. This fix missed a case and so this PR will
extend the fix to cover the additional case.

Fixes: https://github.com/llvm/llvm-project/issues/81774
---
 clang/docs/ReleaseNotes.rst|  3 +++
 clang/lib/Sema/SemaDeclCXX.cpp | 18 +++---
 .../test/CodeGen/union-non-trivial-member.cpp  | 17 +
 clang/test/SemaCXX/cxx0x-nontrivial-union.cpp  | 11 +++
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5bca2c965c866b..452382eb6c4a1e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -292,6 +292,9 @@ Bug Fixes to C++ Support
   was only accepted at namespace scope but not at local function scope.
 - Clang no longer tries to call consteval constructors at runtime when they 
appear in a member initializer.
   (`#782154 `_`)
+- Fix for clang incorrectly rejecting the default construction of a union with
+  nontrivial member when another member has an initializer.
+  (`#81774 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 79263bc3ff671d..25a4b4381ca25e 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -9442,9 +9442,21 @@ bool 
SpecialMemberDeletionInfo::shouldDeleteForSubobjectCall(
 
   int DiagKind = -1;
 
-  if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted)
-DiagKind = !Decl ? 0 : 1;
-  else if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::Ambiguous)
+  if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted) {
+if (CSM == Sema::CXXDefaultConstructor && Field &&
+Field->getParent()->isUnion()) {
+  // [class.default.ctor]p2:
+  //   A defaulted default constructor for class X is defined as deleted if
+  //   - X is a union that has a variant member with a non-trivial default
+  // constructor and no variant member of X has a default member
+  // initializer
+  const auto *RD = cast(Field->getParent());
+  if (!RD->hasInClassInitializer())
+DiagKind = !Decl ? 0 : 1;
+} else {
+  DiagKind = !Decl ? 0 : 1;
+}
+  } else if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::Ambiguous)
 DiagKind = 2;
   else if (!isAccessible(Subobj, Decl))
 DiagKind = 3;
diff --git a/clang/test/CodeGen/union-non-trivial-member.cpp 
b/clang/test/CodeGen/union-non-trivial-member.cpp
index fdc9fd16911e14..8b055a9970fc75 100644
--- a/clang/test/CodeGen/union-non-trivial-member.cpp
+++ b/clang/test/CodeGen/union-non-trivial-member.cpp
@@ -15,14 +15,25 @@ union UnionNonTrivial {
 non_trivial_constructor b{};
 };
 
+struct Handle {
+Handle(int) {}
+};
+
+union UnionNonTrivialEqualInit {
+int NoState = 0;
+Handle CustomState;
+};
+
 void f() {
 UnionInt u1;
 UnionNonTrivial u2;
+UnionNonTrivialEqualInit u3;
 }
 
 // CHECK:  define dso_local void @_Z1fv()
 // CHECK:call void @_ZN8UnionIntC1Ev
 // CHECK-NEXT:   call void @_ZN15UnionNonTrivialC1Ev
+// CHECK-NEXT:   call void @_ZN24UnionNonTrivialEqualInitC1Ev
 
 // CHECK:  define {{.*}}void @_ZN8UnionIntC1Ev
 // CHECK:call void @_ZN8UnionIntC2Ev
@@ -30,8 +41,14 @@ void f() {
 // CHECK:  define {{.*}}void @_ZN15UnionNonTrivialC1Ev
 // CHECK:call void @_ZN15UnionNonTrivialC2Ev
 
+// CHECK:  define {{.*}}void @_ZN24UnionNonTrivialEqualInitC1Ev
+// CHECK:call void @_ZN24UnionNonTrivialEqualInitC2Ev
+
 // CHECK:  define {{.*}}void @_ZN8UnionIntC2Ev
 // CHECK:store i32 1000
 
 // CHECK:  define {{.*}}void @_ZN15UnionNonTrivialC2Ev
 // CHECK:call void @_ZN23non_trivial_constructorC1Ev
+
+// CHECK:  define {{.*}}void @_ZN24UnionNonTrivialEqualInitC2Ev
+// CHECK:store i32 0
diff --git a/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp 
b/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
index c7cdf76d850dbe..833642b3d739ab 100644
--- a/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
+++ b/clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
@@ -188,3 +188,14 @@ static_assert(U2().b.x == 100, "");
 static_assert(U3().b.x == 100, "");
 
 } // namespace GH48416
+
+namespace GH81774 {
+struct Handle {
+Handle(int) {}
+};
+// Should be well-formed because NoState has a brace-or-equal-initializer.
+union a {
+int NoState = 0;
+ 

[clang] [clang] Sequence C++20 Parenthesized List Init (PR #83476)

2024-02-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik approved this pull request.


https://github.com/llvm/llvm-project/pull/83476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Sequence C++20 Parenthesized List Init (PR #83476)

2024-02-29 Thread Shafik Yaghmour via cfe-commits


@@ -17615,31 +17615,28 @@ class SequenceChecker : public 
ConstEvaluatedExprVisitor {
   return VisitExpr(CCE);
 
 // In C++11, list initializations are sequenced.
-SmallVector Elts;
-SequenceTree::Seq Parent = Region;
-for (CXXConstructExpr::const_arg_iterator I = CCE->arg_begin(),
-  E = CCE->arg_end();
- I != E; ++I) {
-  Region = Tree.allocate(Parent);
-  Elts.push_back(Region);
-  Visit(*I);
-}
-
-// Forget that the initializers are sequenced.
-Region = Parent;
-for (unsigned I = 0; I < Elts.size(); ++I)
-  Tree.merge(Elts[I]);
+SequenceExpressionsInOrder({CCE->getArgs(), CCE->getNumArgs()});

shafik wrote:

I took me a bit of checking to convince myself this was doing the right thing. 
It might be nice to refactor `CXXConstructExpr` to have an a member that does 
the same as `ILE->inits()` and returns an `ArrayRef`. It looks like do similar 
things to create an `ArrayRef` in other places as well but probably should be a 
second PR.

Maybe change this to `llvm::ArrayRef(CCE->getArgs(), CCE->getNumArgs())` to 
make it more explicit for now.

https://github.com/llvm/llvm-project/pull/83476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Sequence C++20 Parenthesized List Init (PR #83476)

2024-02-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/83476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Sequence C++20 Parenthesized List Init (PR #83476)

2024-02-29 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM after addressing comment.

https://github.com/llvm/llvm-project/pull/83476
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang] Add `clang::behaves_like_std(...)` attribute (PR #76596)

2024-02-29 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Adding @AaronBallman and @erichkeane for a wider audience 

https://github.com/llvm/llvm-project/pull/76596
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][Sema] Warn on self move for inlined static cast (PR #76646)

2024-02-29 Thread Shafik Yaghmour via cfe-commits


@@ -35,11 +41,14 @@ class field_test {
   int x;
   field_test(field_test&& other) {
 x = std::move(x);  // expected-warning{{explicitly moving}}
+x = static_cast(x);  // expected-warning{{explicitly moving}}

shafik wrote:

Can you add some tests where we should not get diagnostics for example like the 
one right below but w/ `static_cast`

https://github.com/llvm/llvm-project/pull/76646
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   3   4   5   6   >