[clang] [clang] Add tests for some CWG issues from 2024-05-31 telecon (PR #94167)
https://github.com/shafik commented: LGTM, curious why you skipped some and not others from that telecom. Likely folks won't be able to check out the DRs until after St Louis. https://github.com/llvm/llvm-project/pull/94167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add tests for Core issues about friend templates (PR #94288)
https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/94288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add tests for Core issues about friend templates (PR #94288)
https://github.com/shafik approved this pull request. https://github.com/llvm/llvm-project/pull/94288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add tests for Core issues about friend templates (PR #94288)
@@ -121,6 +145,21 @@ derived d2(42, 9); #endif } +namespace cwg1945 { // cwg1945: no +template struct A { + class B { +class C {}; + }; +}; +class X { + static int x; + // FIXME: this is ill-formed, because A::B::C does not end with a simple-template-id + template + friend class A::B::C; + // expected-warning@-1 {{dependent nested name specifier 'A::B::' for friend class declaration is not supported; turning off access control for 'X'}} +}; +} // namespace cwg1918 shafik wrote: ```suggestion } // namespace cwg1945 ``` https://github.com/llvm/llvm-project/pull/94288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add tests for Core issues about friend templates (PR #94288)
@@ -373,6 +373,98 @@ namespace cwg1837 { // cwg1837: 3.3 #endif } +namespace cwg1862 { // cwg1862: no +template +struct A { + struct B { +void e(); + }; + + void f(); + + struct D { +void g(); + }; + + T h(); + + template + T i(); +}; + +template<> +struct A { + struct B { +void e(); + }; + + int f(); + + struct D { +void g(); + }; + + template + int i(); +}; + +template<> +struct A { + int* h(); +}; + +class C { + int private_int; + + template + friend struct A::B; + // expected-warning@-1 {{dependent nested name specifier 'A::' for friend class declaration is not supported; turning off access control for 'C'}} + + template + friend void A::f(); + // expected-warning@-1 {{dependent nested name specifier 'A::' for friend class declaration is not supported; turning off access control for 'C'}} + + // FIXME: this is ill-formed, because A::D does not end with a simple-template-id + template + friend void A::D::g(); + // expected-warning@-1 {{dependent nested name specifier 'A::D::' for friend class declaration is not supported; turning off access control for 'C'}} + + template + friend int *A::h(); + // expected-warning@-1 {{dependent nested name specifier 'A::' for friend class declaration is not supported; turning off access control for 'C'}} + + template + template + friend T A::i(); + // expected-warning@-1 {{dependent nested name specifier 'A::' for friend class declaration is not supported; turning off access control for 'C'}} +}; + +C c; + +template +void A::B::e() { (void)c.private_int; } shafik wrote: I guess these work b/c as the diagnostic above said "we turn off access control"? https://github.com/llvm/llvm-project/pull/94288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add tests for Core issues about friend templates (PR #94288)
https://github.com/shafik commented: LGTM https://github.com/llvm/llvm-project/pull/94288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add tests for Core issues about friend templates (PR #94288)
https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/94288 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Correctly diagnose a static function overloading a non-static function (PR #93460)
https://github.com/shafik commented: Thank you for the fix! https://github.com/llvm/llvm-project/pull/93460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Improve ast-dumper text printing of TemplateArgument (PR #93431)
@@ -947,6 +947,26 @@ void TextNodeDumper::dumpDeclRef(const Decl *D, StringRef Label) { }); } +void TextNodeDumper::dumpTemplateArgument(const TemplateArgument ) { + llvm::SmallString<128> Str; + { +llvm::raw_svector_ostream SS(Str); +TA.print(PrintPolicy, SS, /*IncludeType=*/true); + } + OS << " '" << Str << "'"; shafik wrote: I am going to fight over this but I feel like it makes less sense for integer literals and char literals. Expressions, type etc sure. Consistency for consistency sake while tempting does not feel convincing to me. https://github.com/llvm/llvm-project/pull/93431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] require template arg list after template kw (PR #80801)
@@ -2995,13 +2996,23 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec , ParsedType ObjectType, SS, ObjectType, ObjectHadErrors, TemplateKWLoc ? *TemplateKWLoc : SourceLocation(), Id, IdLoc, EnteringContext, Result, TemplateSpecified); -else if (TemplateSpecified && - Actions.ActOnTemplateName( - getCurScope(), SS, *TemplateKWLoc, Result, ObjectType, - EnteringContext, Template, - /*AllowInjectedClassName*/ true) == TNK_Non_template) - return true; +if (TemplateSpecified) { + TemplateNameKind TNK = + Actions.ActOnTemplateName(getCurScope(), SS, *TemplateKWLoc, Result, +ObjectType, EnteringContext, Template, +/*AllowInjectedClassName*/ true); shafik wrote: ```suggestion /*AllowInjectedClassName=*/true); ``` nitpick https://github.com/llvm/llvm-project/pull/80801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] allow `` `@$ `` in raw string delimiters in C++26 (PR #93216)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
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)
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)
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)
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