[clang] [clang] Implement P2582R1: CTAD from inherited constructors (PR #98788)
https://github.com/mizvekov commented: Just a couple of nits, I will do a more comprehensive review later. https://github.com/llvm/llvm-project/pull/98788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Implement P2582R1: CTAD from inherited constructors (PR #98788)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/98788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (PR #86265)
@@ -14636,6 +14645,20 @@ TreeTransform::TransformLambdaExpr(LambdaExpr *E) { /*IsInstantiation*/ true); SavedContext.pop(); + // Parts other than the capture e.g. the lambda body might still contain a + // pattern that an outer fold expression would expand. + // + // We don't have a way to propagate up the ContainsUnexpandedParameterPack + // flag from a Stmt, so we have to revisit the lambda. + if (!LSICopy.ContainsUnexpandedParameterPack) { +llvm::SmallVector UnexpandedPacks; +getSema().collectUnexpandedParameterPacksFromLambda(NewCallOperator, +UnexpandedPacks); +// FIXME: Should we call Sema::DiagnoseUnexpandedParameterPacks() instead? +// Unfortunately, that requires the LambdaScopeInfo to exist, which has been +// removed by ActOnFinishFunctionBody(). +LSICopy.ContainsUnexpandedParameterPack = !UnexpandedPacks.empty(); + } mizvekov wrote: I am not sure I understand why we would need to call `Sema::DiagnoseUnexpandedParameterPacks` here. I think we should be able to diagnose all unexpanded parameter packs during the initial lambda parsing, so what gives? Also, I am not sure what you mean by leaving it as an NFC. Regardless, you have a copy of the LSI here, so what would stop you from pushing a new Lambda Scope and copying this LSI back into it? Something like: ```C++ LambdaScopeInfo *LSI = getSema().PushLambdaScope(); *LSI = LSICopy; Sema::FunctionScopeRAII FuncScopeCleanup(getSema()); ``` This is obviously an ugly hack, but I don't think the solution is to simply teach `ActOnFinishFunctionBody` not to pop the function scope. See this FIXME for a potentially more comprehensive fix: https://github.com/llvm/llvm-project/blob/77ac07444d32668d5826ef27c24180fb10425213/clang/lib/Sema/TreeTransform.h#L14648 https://github.com/llvm/llvm-project/pull/86265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (PR #86265)
@@ -23,3 +23,104 @@ namespace PR41576 { } static_assert(f(3, 4) == 6); // expected-note {{instantiation}} } + +namespace PR85667 { + +template +struct identity { + using type = T; +}; + +template void f() { + + static_assert([](Is... x) { +return ([I(x)] { + return I; +}() + ...); + }(1, 2) == 3); + + static_assert([](Is... x) { +return ([](auto y = Is()) { return y + 1; } + ...); + }(0, 0, 0) == 3); + + []() { +return ([]() noexcept(Is()) { return 0; }() + ...); + }.template operator()(); + + static_assert(__is_same(decltype([]() { +return ([]() -> decltype(Is()) { return {}; }(), +...); + }.template operator()()), + char)); + + []() { +return ([]() -> decltype(Is()) { return Ts(); }() + ...); +// expected-error@-1 {{unexpanded parameter pack 'Ts'}} + }.template operator()(); + + // Note that GCC and EDG reject this case currently. + // GCC says the fold expression "has no unexpanded parameter packs", while + // EDG says the constraint is not allowed on a non-template function. + // MSVC is happy with it. + []() { +([]() + requires(Is()) + {}, + ...); + }.template operator()(); + + // https://github.com/llvm/llvm-project/issues/56852 + [](Is...) { +([] { + using T = identity::type; +}(), ...); + }(1, 2); + + [](auto ...y) { +([y] { }(), ...); + }(); + + [](auto ...x) { +([&](auto ...y) { + ([x..., y] { }(), ...); +})(1); + }(2, 'b'); + +#if 0 + // https://github.com/llvm/llvm-project/issues/18873 + [](auto ...x) { // #1 +([&](auto ...y) { // #2 + ([x, y] { }(), ...); // #3 +})(1, 'a'); // #4 + }(2, 'b'); // #5 + mizvekov wrote: I am not opposed to adding disabled crash repros as tests, for future fixes, but these should be clearly marked with a FIXME. https://github.com/llvm/llvm-project/pull/86265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -6308,11 +6308,11 @@ TypeResult Sema::ActOnTypeName(Declarator &D) { CheckExtraCXXDefaultArguments(D); } - if (const AutoType *AutoT = T->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); - + if (const AutoType *AutoT = T->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); +CheckConstrainedAuto(AutoT, + Loc ? Loc.getConceptNameLoc() : SourceLocation()); + } mizvekov wrote: But my question was, what cases are we handling with this check in `ActOnTypeName`? I thought 'auto' couldn't appear here. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
mizvekov wrote: LGTM, Thanks! https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(), /*DiagID=*/0); - if (const AutoType *AutoT = R->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); + if (const AutoType *AutoT = R->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); mizvekov wrote: Yeah, we don't need to have both TypeSourceInfo and TypeLoc in clang. I am not sure the history of how the situation arose, but we can pretty much consolidate on TypeLoc at some point. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -6308,11 +6308,11 @@ TypeResult Sema::ActOnTypeName(Declarator &D) { CheckExtraCXXDefaultArguments(D); } - if (const AutoType *AutoT = T->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); - + if (const AutoType *AutoT = T->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); +CheckConstrainedAuto(AutoT, + Loc ? Loc.getConceptNameLoc() : SourceLocation()); + } mizvekov wrote: Ah, is this purely for the error cases? Ie in my example above, you will still get the error, but is that for suppressing the deprecated warning in case that auto was constrained and happened to be the only use of that concept? https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -6308,11 +6308,11 @@ TypeResult Sema::ActOnTypeName(Declarator &D) { CheckExtraCXXDefaultArguments(D); } - if (const AutoType *AutoT = T->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); - + if (const AutoType *AutoT = T->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); +CheckConstrainedAuto(AutoT, + Loc ? Loc.getConceptNameLoc() : SourceLocation()); + } mizvekov wrote: These still have the same issue, but I don't understand why we need this check here. We obviously can't write stuff like ```C++ using X = auto; ``` So I am missing in what case undeduced auto can appear here, except the weird deleted function error recovery issue. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(), /*DiagID=*/0); - if (const AutoType *AutoT = R->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); + if (const AutoType *AutoT = R->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); mizvekov wrote: But here we are not just asking for the location of constrained auto. A TypeLoc is not just a SourceLocation, it's a QualType *with* source locations. The reason you are starting with a TypeLoc is that you are also interested in the source location. This is a bit confusing because in a lot of places we pass both a QualType and a TypeLoc, which is mostly redundant. That might induce folks into thinking that a TypeLoc is just source locations on the side, which is not true. But presently, they are not always redundant though. There are some cases we store both separately, and apply some transformations, like type deduction, only to the QualType, and leave the TypeLoc alone. I think this is not ideal though, and seems to be mostly due to tech debt. As an aside, if all you need to do is mark the concepts as used, you might as well do that earlier when the AutoType is formed, for example around the calls to `ASTContext::getAutoType` in `SemaType.cpp` / `ConvertDeclSpecToType`. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix assertion failure during conversion function overload resolution. (PR #98671)
https://github.com/mizvekov approved this pull request. LGTM as well https://github.com/llvm/llvm-project/pull/98671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add deprecation warning for `-Ofast` driver option (PR #98736)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/98736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add deprecation warning for `-Ofast` driver option (PR #98736)
@@ -442,6 +442,9 @@ def warn_drv_deprecated_arg : Warning< def warn_drv_deprecated_arg_no_relaxed_template_template_args : Warning< "argument '-fno-relaxed-template-template-args' is deprecated">, InGroup; +def warn_drv_deprecated_arg_ofast : Warning< + "argument '-Ofast' is deprecated; use '-O3', possibly with '-ffast-math'">, mizvekov wrote: I think in this diagnostic it's more important to inform the user that he should replace `-Ofast` with `-O3 -ffast-math` in order to just circumvent the deprecation and keep the current behavior, and this should be clear. Throwing the option of using just `O3` as equally valid is a bit misleading, but we can, as a note, inform the user that it's possible he made the wrong choice in the first place, and advise a switch to `O3`. https://github.com/llvm/llvm-project/pull/98736 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(), /*DiagID=*/0); - if (const AutoType *AutoT = R->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); + if (const AutoType *AutoT = R->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); mizvekov wrote: Regarding the error recovery issue, in `SemaOverload.cpp`: In `FixOverloadedFunctionReference`, for both `UnresolvedLookupExpr` and `UnresolvedMemberExpr`, we must avoid building a `DeclRefExpr` which has a function type returning undeduced auto. Presently this should only happen for calls to deleted functions returning a deduced type, so should only happen when it's called from `FinishOverloadedCallExpr`, in the `OR_Deleted` case. You can replace just the `auto` in the return type with `int`, keeping pointers and references in case of `auto *` or `auto &`. That can be done using `Sema::SubstAutoType`. Eventually when we implement some kind of `ErrorTy`, for improving error recovery, we can change all such uses of 'int', using that instead. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(), /*DiagID=*/0); - if (const AutoType *AutoT = R->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); + if (const AutoType *AutoT = R->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); mizvekov wrote: Sure, but let me get back first to the other problem in this area: I think that assumption I pointed out is wrong, we should not be using `getAs` here, as that will not dig through ReferenceType and PointerType, and the following example will not be diagnosed: https://godbolt.org/z/jzEaYPq4r The `getContainedAutoTypeLoc` is what is a good fit for that, and what we should be using throughout. It will dig through the things needed for finding the AutoType, like pointers, references, and some of the necessary type sugar which can appear in source code when writing a deduced type. It does not dig through the things it doesn't need to, like `decltype`, because you can't write a deduced type with that. So something like: ```suggestion if (AutoTypeLoc TL = TInfo->getTypeLoc().getContainedAutoTypeLoc()) { const AutoType *AT = TL.getTypePtr(); ``` Should work, and it will not get confused by the error recovery issue, which can be addressed separately. Nit: We conventionally use `TL` abbreviation for TypeLoc, and `Loc` is conventional for SourceLocation. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
mizvekov wrote: > That's a pretty substantial policy change to propose, and this probably isn't > the place to propose/discuss it. If that's your intent, probably best to take > that up on discord. > I am not proposing a policy change. I believe the current policy is aimed at giving an escape hatch for projects which there is basically one or two active developers. I am pointing out that I believe we don't need for that escape hatch to apply to any parts of clang currently. > (FWIW, check some of the recent modules changes @ChuanqiXu9 has been working > on to see that reviewer bandwidth here is pretty thin (& my experience in > LLVM in general, including clang, is that reviewer bandwidth is pretty thin - > though it is something we should address & I do think it might be time to > change LLVM's post-commit review policy, but I think it'll be a substantial > amount of work)) If you feel bandwidth for modules is pretty thin, I put myself available as a reviewer, so feel free to ping me. I may not have a lot of time available for fully reviewing big patches, but I can certainly help with the smaller patches such as this one. https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
https://github.com/mizvekov requested changes to this pull request. See previous comments, I think this is primarily an error recovery issue on the decltype, we shouldn't let it escape an undeduced auto, as that would confuse further analysis. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(), /*DiagID=*/0); - if (const AutoType *AutoT = R->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); + if (const AutoType *AutoT = R->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); mizvekov wrote: Oh, looking at the reproducer, I see the problem: Our variable is getting the type of an undeduced AutoType through that decltype calling a deleted function with auto type. This is confusing our semantic analysis for the variable, as now we think the variable was declared as auto, as getAs will look through all sugar nodes, 'decltype' included. Since `getContainedAutoTypeLoc()` doesn't look through decltype, you get a crash. But we shouldn't be getting an 'auto' here in the first place. The correct fix would be to make that decltype have some error type, for error recovery purposes. Since we don't implement a generic unknown error type, 'int' would still be better. https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix crash in concept deprecation (PR #98622)
@@ -7416,10 +7416,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(), /*DiagID=*/0); - if (const AutoType *AutoT = R->getAs()) -CheckConstrainedAuto( -AutoT, -TInfo->getTypeLoc().getContainedAutoTypeLoc().getConceptNameLoc()); + if (const AutoType *AutoT = R->getAs()) { +AutoTypeLoc Loc = TInfo->getTypeLoc().getContainedAutoTypeLoc(); mizvekov wrote: These two lines are analyzing the type in very different ways, so there seems to be further bugs here. `Type::getAs` will simply desugar a type until it finds a node with type 'AutoType'. `getContainedAutoTypeLoc()` will dig through a certain list of type nodes until it finds an 'AutoType'. This list includes non-sugar nodes as well. Let's assume for now we really want `R->getAs()` in the if condition here, which would not find the AutoType in `auto *`. This seems wrong, as these can be constrained as well. But we can handle that as a separate issue. So to hit your bug, you are finding an AutoType through getAs, but not finding it through getContainedAutoTypeLoc(). My first suspicion would be that the type contains a sugar node not handled by `GetContainedAutoTypeLocVisitor`, in `AST/TypeLoc.cpp`. Can you check that? https://github.com/llvm/llvm-project/pull/98622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][NFCI] Remove records of unsatisfied atomic expressions in ConstraintSatisfaction (PR #98654)
https://github.com/mizvekov approved this pull request. LGTM, I agree that we currently don't use it, so we might as well remove it. But the overall situation is all wrong, we shouldn't be storing fully formed diagnostic strings in the AST, we should be storing them as PartialDiagnostics. But making that right requires bigger changes. https://github.com/llvm/llvm-project/pull/98654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Prevent dangling StringRefs (PR #98699)
https://github.com/mizvekov approved this pull request. LGTM. If it does fix the mentioned issue, please add a release note. https://github.com/llvm/llvm-project/pull/98699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
mizvekov wrote: > Clarifying a couple of things here I think we need the distinction between simple NFC, for trivial, non-controversial and non-surprising changes like some renames, small whitespace / formatting change, documentation changes, and complex NFC, which requires some thinking and has more risk of not actually being NFC. So in practical terms, I propose a complex NFC change should only be excused of changes to functional testing, nothing else. Also, I think we presently have sufficient review bandwidth in clang proper, even including the area of modules, so that any non-trivial change can be reviewed, and I also propose that we abstain from trying to directly commit those. https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang][AST] Move NamespaceDecl bits to DeclContext (PR #98567)
https://github.com/mizvekov approved this pull request. LGTM, as a clean up, and I see no notable performance implications. Is there any follow up work for this? https://github.com/llvm/llvm-project/pull/98567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Parse] Fix ambiguity with nested-name-specifiers that may declarative (PR #96364)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/96364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
https://github.com/mizvekov approved this pull request. LGTM, sans: * missing newline at end of test file * A similar test for template template parameter. https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform { // Find all template parameters that appear in the given DeducedArgs. // Return the indices of the template parameters in the TemplateParams. SmallVector TemplateParamsReferencedInTemplateArgumentList( -ArrayRef TemplateParams, +const TemplateParameterList* TemplateParamsList, ArrayRef DeducedArgs) { struct TemplateParamsReferencedFinder : public RecursiveASTVisitor { +const TemplateParameterList* TemplateParamList; llvm::DenseSet TemplateParams; llvm::DenseSet ReferencedTemplateParams; -TemplateParamsReferencedFinder(ArrayRef TemplateParams) -: TemplateParams(TemplateParams.begin(), TemplateParams.end()) {} +TemplateParamsReferencedFinder( +const TemplateParameterList *TemplateParamList) +: TemplateParamList(TemplateParamList), + TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) { +} bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) { - MarkAppeared(TTP->getDecl()); + // We use the index and depth to retrieve the corresponding template + // parameter from the parameter list. + // Note that Clang may not preserve type sugar during template argument + // deduction. In such cases, the TTP is a canonical TemplateTypeParamType, + // which only retains its index and depth information. + if (TTP->getDepth() == TemplateParamList->getDepth() && mizvekov wrote: The NTTP case is trickier to test, as we don't canonicalize expressions the same way we do types and templates, but in some situations, you could have an NTTP which was uniqued pointing to a specific parameter, but this is misleading and you should disregard that and look at only the depth and index. But I would expect template template parameters to behave similarly as type parameters here, and it should be just as straightforward to test, so I think it would b e worth a try. https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
@@ -99,3 +99,28 @@ BFoo b2(1.0, 2.0); // CHECK-NEXT: | | |-ParmVarDecl {{.*}} 'type-parameter-0-0' // CHECK-NEXT: | | `-ParmVarDecl {{.*}} 'type-parameter-0-0' // CHECK-NEXT: | `-CXXDeductionGuideDecl {{.*}} implicit used 'auto (double, double) -> Foo' implicit_instantiation + +namespace GH90209 { +template +struct List { + List(int); +}; + +template +struct TemplatedClass { + TemplatedClass(T1); +}; + +template +TemplatedClass(T1) -> TemplatedClass>; + +template +using ATemplatedClass = TemplatedClass>; + +ATemplatedClass test(1); +// Verify that we have a correct template parameter list for the deduction guide. +// +// CHECK: FunctionTemplateDecl {{.*}} +// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2 +// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible +} // namespace GH90209 mizvekov wrote: The GitHub editor still shows the missing newline at end of file marker. https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
mizvekov wrote: The reproducer turned out to be pretty simple: ```C++ export module a; module :private; static void f() {} void g() { f(); } ``` Compiles without that patch, otherwise produces: ``` error: no matching function for call to 'f' ``` > Oh, sorry, I took another look at the commit and it looks the change makes it > not a NFC change is this line: > [99873b3#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514](https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514) > > This shouldn't be in that commit but in this commit. It is not intentional. I > guess we can't observe that if we put that in this PR too. And that change > looks not bad. So maybe it makes something already bad to show up. Even without that change, the other changes are too complex, and there are other suspicious things you wouldn't expect in an NFC commit which doesn't call this out specifically, like the removal of that whole block with the FIXME included. I think the appropriate tag for such commits would be NFCI, and should still require PR and review. https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Handle class member access expressions with valid nested-name-specifiers that become invalid after lookup (PR #98167)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/98167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
mizvekov wrote: FYI, the commit 99873b35da7ecb905143c8a6b8deca4d4416f1a9, which lists this PR as a motivator, causes breakage building a project, which I am still looking for a reduced test case. The commit is not NFC despite what is says, just by cursory inspection of the change. Please avoid adding NFC tag to such commits in the future and create a pull request. https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix the order of addInstantiatedParameters in LambdaScopeForCallOperatorInstantiationRAII (PR #97215)
https://github.com/mizvekov approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/97215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash in Sema::FindInstantiatedDecl (PR #96509)
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: @@ -6300,7 +6300,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D, getTrivialTemplateArgumentLoc(UnpackedArg, QualType(), Loc)); } QualType T = CheckTemplateIdType(TemplateName(TD), Loc, Args); - if (T.isNull()) + if (T.isNull() || T->containsErrors()) mizvekov wrote: I don't understand the logic of this change: If there is no reason to make `CheckTemplateIdType` fail for more cases, why make this change then? Usually in error recovery, we want to carry the error information as much as we can, where practical. In this case, there is the same implementation complexity either way. This change went in the opposite direction: We are bailing out earlier for more cases, and we have no reason for it. https://github.com/llvm/llvm-project/pull/96509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Index] Add support for dependent class scope explicit specializations of function templates to USRGenerator (PR #98027)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/98027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform { // Find all template parameters that appear in the given DeducedArgs. // Return the indices of the template parameters in the TemplateParams. SmallVector TemplateParamsReferencedInTemplateArgumentList( -ArrayRef TemplateParams, +const TemplateParameterList* TemplateParamsList, mizvekov wrote: The template parameters themselves should also have a depth, not just the index. So you don't need to change everything to pass a TemplateParameterList instead of the array as before. In fact, TemplateParameterList will just return the depth of its first parameter. https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform { // Find all template parameters that appear in the given DeducedArgs. // Return the indices of the template parameters in the TemplateParams. SmallVector TemplateParamsReferencedInTemplateArgumentList( -ArrayRef TemplateParams, +const TemplateParameterList* TemplateParamsList, ArrayRef DeducedArgs) { struct TemplateParamsReferencedFinder : public RecursiveASTVisitor { +const TemplateParameterList* TemplateParamList; llvm::DenseSet TemplateParams; llvm::DenseSet ReferencedTemplateParams; -TemplateParamsReferencedFinder(ArrayRef TemplateParams) -: TemplateParams(TemplateParams.begin(), TemplateParams.end()) {} +TemplateParamsReferencedFinder( +const TemplateParameterList *TemplateParamList) +: TemplateParamList(TemplateParamList), + TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) { +} bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) { - MarkAppeared(TTP->getDecl()); + // We use the index and depth to retrieve the corresponding template + // parameter from the parameter list. + // Note that Clang may not preserve type sugar during template argument + // deduction. In such cases, the TTP is a canonical TemplateTypeParamType, + // which only retains its index and depth information. mizvekov wrote: I would remove this section of the comment, as I think this sort of implies it would be ok to use the type sugar if it were available, but if we rely on this for correct compilation of the program, it doesn't make sense to call it type sugar anymore. ```suggestion ``` https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
@@ -2653,20 +2653,34 @@ struct ConvertConstructorToDeductionGuideTransform { // Find all template parameters that appear in the given DeducedArgs. // Return the indices of the template parameters in the TemplateParams. SmallVector TemplateParamsReferencedInTemplateArgumentList( -ArrayRef TemplateParams, +const TemplateParameterList* TemplateParamsList, ArrayRef DeducedArgs) { struct TemplateParamsReferencedFinder : public RecursiveASTVisitor { +const TemplateParameterList* TemplateParamList; llvm::DenseSet TemplateParams; llvm::DenseSet ReferencedTemplateParams; -TemplateParamsReferencedFinder(ArrayRef TemplateParams) -: TemplateParams(TemplateParams.begin(), TemplateParams.end()) {} +TemplateParamsReferencedFinder( +const TemplateParameterList *TemplateParamList) +: TemplateParamList(TemplateParamList), + TemplateParams(TemplateParamList->begin(), TemplateParamList->end()) { +} bool VisitTemplateTypeParmType(TemplateTypeParmType *TTP) { - MarkAppeared(TTP->getDecl()); + // We use the index and depth to retrieve the corresponding template + // parameter from the parameter list. + // Note that Clang may not preserve type sugar during template argument + // deduction. In such cases, the TTP is a canonical TemplateTypeParamType, + // which only retains its index and depth information. + if (TTP->getDepth() == TemplateParamList->getDepth() && mizvekov wrote: Shouldn't this issue come up for the other kind of template parameters as well, besides type parameters? https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD: use index and depth to retrieve template parameter for TemplateParamsReferencedInTemplateArgumentList (PR #98013)
@@ -99,3 +99,28 @@ BFoo b2(1.0, 2.0); // CHECK-NEXT: | | |-ParmVarDecl {{.*}} 'type-parameter-0-0' // CHECK-NEXT: | | `-ParmVarDecl {{.*}} 'type-parameter-0-0' // CHECK-NEXT: | `-CXXDeductionGuideDecl {{.*}} implicit used 'auto (double, double) -> Foo' implicit_instantiation + +namespace GH90209 { +template +struct List { + List(int); +}; + +template +struct TemplatedClass { + TemplatedClass(T1); +}; + +template +TemplatedClass(T1) -> TemplatedClass>; + +template +using ATemplatedClass = TemplatedClass>; + +ATemplatedClass test(1); +// Verify that we have a correct template parameter list for the deduction guide. +// +// CHECK: FunctionTemplateDecl {{.*}} +// CHECK-NEXT: |-TemplateTypeParmDecl {{.*}} class depth 0 index 0 T2 +// CHECK-NEXT: |-TypeTraitExpr {{.*}} 'bool' __is_deducible +} // namespace GH90209 mizvekov wrote: Minor nit: there is now a missing newline at end of file https://github.com/llvm/llvm-project/pull/98013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] CTAD alias: fix transformation for require-clause expr Part2. (PR #93533)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/93533 ___ 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 rebuilding MemberExprs with invalid object expressions (PR #97455)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/97455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Diagnose use of deprecated template alias (PR #97619)
mizvekov wrote: For reference, see https://reviews.llvm.org/D136533 for similar expansion of diagnostics for uses of declarations, and how that lead to trouble in MacOS land. I'd suggest building libc++ on MacOS, to be sure this won't cause regressions and a future revert. I have a MacOS machine now, so I should be able to try to revive that patch at some point as well. https://github.com/llvm/llvm-project/pull/97619 ___ 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 rebuilding MemberExprs with invalid object expressions (PR #97455)
@@ -2896,6 +2896,9 @@ class TreeTransform { SS.Adopt(QualifierLoc); Base = BaseResult.get(); +if (Base->containsErrors()) + return ExprError(); mizvekov wrote: I am still traveling back from St Louis, so I can't double check this: Why is this point the farthest we can go if Base contains errors? It seems we can build a reference expression over it from the cases above, but why not below? https://github.com/llvm/llvm-project/pull/97455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Correctly transform dependent operands of overloaded binary operator& (PR #97596)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/97596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Treat explicit specializations of static data member templates declared without 'static' as static data members when diagnosing uses of 'auto' (PR #97425)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/97425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] qualifier should be transformed (PR #94725)
mizvekov wrote: > I mean we can transform `QualifiedTemplateName` to `DependentTemplateName` > and this is what this patch does. But that goes against the natural flow of template instantiation. We can't go back from non-dependent into dependent. We can't transform a regular TemplateName back into a DependentTemplateName, just as we can't go back to DependentTemplateSpecializationType from a TemplateSpecializationType. That's why I find this so confusing... > If we build a `DependentTemplateName` instead of `QualifiedTemplateName` > earlier would affect the code in partial specialization like: > > ```c++ > template > struct A::B > ``` Can you elaborate on that? That still seems like the right path to me, you probably found another problem, that happens some times. https://github.com/llvm/llvm-project/pull/94725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] qualifier should be transformed (PR #94725)
mizvekov wrote: > @mizvekov After looking into the code, I think we should transform qualifier > in TST. Consider the following code: So the NNS is already dependent, that's great. I still don't understand why you are saying we didn't or can't build a DependentTemplateSpecializationType instead. Yes, the template name needs to stay a DependentTemplateName for that. https://github.com/llvm/llvm-project/pull/94725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
mizvekov wrote: After we fork for clang-20, we can entirely remove `frelaxed-template-template-args`, and so won't need to worry about testing that non-conforming mode. Would it be reasonable to postpone that until then? https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Change style of superseded issues on C++ DR status page (PR #96051)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/96051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94981 >From b238961ba3a174d7dc211caf36ff8fd6c8429a76 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 11 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 63 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 153 +--- clang/lib/AST/ASTDiagnostic.cpp | 45 +++-- clang/lib/AST/ASTImporter.cpp | 15 ++ clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/Decl.cpp| 3 +- clang/lib/AST/ItaniumMangle.cpp | 20 ++- clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 163 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/AST/TypePrinter.cpp | 3 +- clang/lib/Sema/SemaDeclCXX.cpp| 9 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 158 + .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 62 +-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 28 files changed, 629 insertions(+), 257 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate); #undef TEMPLATE_KIND } llvm_unreachable("Unhandled NameKind enum"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index a366f1331c2d3..e6d16af2495fe 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1120,6 +1120,7 @@ class CollectExtraHighlightings case TemplateName::SubstTemplateTemplateParm: case TemplateName::SubstTemplateTemplateParmPack: case TemplateName::UsingTemplate: +case TemplateName::DeducedTemplate: // Names that could be resolved to a TemplateDecl are handled elsewhere. break; } diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index de86cb5e9d7fc..837bcacbc0bfc 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet SubstTemplateTemplateParmPacks; + mutable llvm::ContextualFoldingSet + DeducedTemplates; mutable llvm::ContextualFoldingSet Arr
[clang] [Clang] Instantiate local constexpr functions eagerly (PR #95660)
https://github.com/mizvekov approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/95660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Instantiate local constexpr functions eagerly (PR #95660)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/95660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Instantiate local constexpr functions eagerly (PR #95660)
@@ -18112,7 +18112,8 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func, if (FirstInstantiation || TSK != TSK_ImplicitInstantiation || Func->isConstexpr()) { - if (isa(Func->getDeclContext()) && + if (!Func->isConstexpr() && mizvekov wrote: Nit: Slight simplification, you might as well move this branch after the `else if (Func->isConstexpr())` below instead of testing it's negation. https://github.com/llvm/llvm-project/pull/95660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
@@ -9219,7 +9222,8 @@ class Sema final : public SemaBase { /// \returns true if an error occurred, false otherwise. bool CheckTemplateArgumentList( TemplateDecl *Template, SourceLocation TemplateLoc, - TemplateArgumentListInfo &TemplateArgs, bool PartialTemplateArgs, + TemplateArgumentListInfo &TemplateArgs, mizvekov wrote: Yeah. One issue I have often had with these functions with large amount of both defaulted and non-defaulted parameters, is that you would want to extend it by changing the signature, then arguments would match parameters incorrectly, but this would not cause a hard error on all of the call sites. I could have easily added DefaultArgs as defaulted empty here, but chose not to due to this reason. Besides that, overloading functions with such huge numbers of parameters creates some confusion as well. I'd slightly prefer if we avoided that, but don't have strong enough feelings to go on a crusade against it. https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] qualifier should be transformed (PR #94725)
https://github.com/mizvekov requested changes to this pull request. Needs changes as discussed. https://github.com/llvm/llvm-project/pull/94725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] qualifier should be transformed (PR #94725)
mizvekov wrote: I think this is missing one detail: We now have the same qualifier in two places: The elaborated type node attached to the TST, and in the name of the TST itself. While this is not ideal situation, I think it makes sense to just drop the TemplateName qualifier in the transform for now, so we don't keep transforming it twice, until we make the situation consistent again. Right now the NNS in the TST name is not printed, its only purpose there is to carry the template keyword information, which shouldn't be affected here. The fact that this change somehow affects program semantics is still unexpected and unexplained. As I said on the other PR, after we have built a TST, per current AST design, we shouldn't need the nested name specifier anymore beyond type sugar. So either we built this TST too early, as in it should have stayed as a DependentTemplateSpecializationType longer or some such, or there is something that needs to be rectified in the AST design, but I can't imagine what. If I had to take a guess, there is something wrong in the dependency computation for the NNS. Have you checked that? https://github.com/llvm/llvm-project/pull/94725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit (PR #94515)
@@ -2402,10 +2405,9 @@ Expr *VarDecl::getInit() { auto *Eval = getEvaluatedStmt(); - return cast_if_present( - Eval->Value.isOffset() - ? Eval->Value.get(getASTContext().getExternalSource()) - : Eval->Value.get(nullptr)); + return cast(Eval->Value.isOffset() +? Eval->Value.get(getASTContext().getExternalSource()) +: Eval->Value.get(nullptr)); mizvekov wrote: Small nit: It seems this could move the conditional into the argument for `Eval->Value.get`. https://github.com/llvm/llvm-project/pull/94515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit (PR #94515)
https://github.com/mizvekov approved this pull request. This is much better, thanks! LGTM https://github.com/llvm/llvm-project/pull/94515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Check whether EvaluatedStmt::Value is valid in VarDecl::hasInit (PR #94515)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/94515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Don't print extra space when dumping template names (PR #95213)
https://github.com/mizvekov approved this pull request. I think what I tried to do here is generally consistent: The convention on the Text node dumper is to add a space at the beginning of the field you want to append. I think the true issue here is the label: It breaks convention by adding a space after the colon. I think it would be better to fix that, but I don't know the impact. Since right now this field is only added after the label, this makes little practical difference, so I leave it up to you, either way is fine. Thanks for the fix! https://github.com/llvm/llvm-project/pull/95213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
mizvekov wrote: FYI https://github.com/itanium-cxx-abi/cxx-abi/issues/184 is the tracking issue for the mangling rules we need here. We will probably end up with something quite different than what I coded here so far. https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
@@ -9219,7 +9222,8 @@ class Sema final : public SemaBase { /// \returns true if an error occurred, false otherwise. bool CheckTemplateArgumentList( TemplateDecl *Template, SourceLocation TemplateLoc, - TemplateArgumentListInfo &TemplateArgs, bool PartialTemplateArgs, + TemplateArgumentListInfo &TemplateArgs, mizvekov wrote: I am not seeing a worthwhile tradeoff. Just the function signature is hugely complicated, and would need to be duplicated. The implementation would be mostly the same, with a small block which would be omitted in one of the implementations. What did you have in mind? https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
@@ -9434,6 +9505,32 @@ ASTContext::getSubstTemplateTemplateParmPack(const TemplateArgument &ArgPack, return TemplateName(Subst); } +/// Retrieve the template name that represents a template name +/// deduced from a specialization. +TemplateName +ASTContext::getDeducedTemplateName(TemplateName Underlying, + DefaultArguments DefaultArgs) const { + if (!DefaultArgs) +return Underlying; + + auto &Self = const_cast(*this); mizvekov wrote: Actually in this case the relevant hash tables were already declared mutable, but there were a couple of profile functions not marked const. Fixed. https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94981 >From f05e8590c7fae599d0658829949fa907942e83f2 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 11 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 63 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 153 +--- clang/lib/AST/ASTDiagnostic.cpp | 45 +++-- clang/lib/AST/ASTImporter.cpp | 15 ++ clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/Decl.cpp| 3 +- clang/lib/AST/ItaniumMangle.cpp | 20 ++- clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 163 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/AST/TypePrinter.cpp | 3 +- clang/lib/Sema/SemaDeclCXX.cpp| 9 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 158 + .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 62 +-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 28 files changed, 629 insertions(+), 257 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate); #undef TEMPLATE_KIND } llvm_unreachable("Unhandled NameKind enum"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index a366f1331c2d3..e6d16af2495fe 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1120,6 +1120,7 @@ class CollectExtraHighlightings case TemplateName::SubstTemplateTemplateParm: case TemplateName::SubstTemplateTemplateParmPack: case TemplateName::UsingTemplate: +case TemplateName::DeducedTemplate: // Names that could be resolved to a TemplateDecl are handled elsewhere. break; } diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index f1f20fca477a4..c9376dc02fcfc 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet SubstTemplateTemplateParmPacks; + mutable llvm::ContextualFoldingSet + DeducedTemplates; mutable llvm::ContextualFoldingSet Arr
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
https://github.com/mizvekov closed https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/95202 >From 8bd63f109c2bc1888b4d8dbd5e880900bbb4cef7 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Wed, 12 Jun 2024 00:42:48 -0300 Subject: [PATCH] [clang] fix broken canonicalization of DeducedTemplateSpecializationType This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root cause of #61317. A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST. The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules. The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either. --- No changelog entry because #61317 was already claimed as fixed in previous release. --- clang/include/clang/AST/ASTContext.h | 7 +++ clang/include/clang/AST/TemplateName.h | 4 +- clang/include/clang/AST/Type.h | 11 ++-- clang/lib/AST/ASTContext.cpp | 25 ++--- clang/lib/AST/TemplateName.cpp | 9 clang/unittests/AST/CMakeLists.txt | 1 + clang/unittests/AST/ProfilingTest.cpp | 73 ++ 7 files changed, 107 insertions(+), 23 deletions(-) create mode 100644 clang/unittests/AST/ProfilingTest.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index a1d1d1c51cd41..53ece996769a8 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase { QualType DeducedType, bool IsDependent) const; +private: + QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template, +QualType DeducedType, +bool IsDependent, +QualType Canon) const; + +public: /// Return the unique reference to the type for the specified TagDecl /// (struct/union/class/enum) decl. QualType getTagDeclType(const TagDecl *Decl) const; diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 988a55acd2252..24a7fde76195d 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -346,7 +346,9 @@ class TemplateName { /// error. void dump() const; - void Profile(llvm::FoldingSetNodeID &ID); + void Profile(llvm::FoldingSetNodeID &ID) { +ID.AddPointer(Storage.getOpaqueValue()); + } /// Retrieve the template name as a void pointer. void *getAsVoidPointer() const { return Storage.getOpaqueValue(); } diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 9eb3f6c09e3d3..fab233b62d8d1 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public DeducedType, DeducedTemplateSpecializationType(TemplateName Template, QualType DeducedAsType, -bool IsDeducedAsDependent) +bool IsDeducedAsDependent, QualType Canon) : DeducedType(DeducedTemplateSpecialization, DeducedAsType, toTypeDependence(Template.getDependence()) | (IsDeducedAsDependent ? TypeDependence::DependentInstantiation : TypeDependence::None), -DeducedAsType.isNull() ? QualType(this, 0) - : DeducedAsType.getCanonicalType()), +Canon), Template(Template) {} public: /// Retrieve the name of the template that we are deducing. TemplateName getTemplateName() const { return Template;} - void Profile(llvm::FoldingSetNodeID &ID) { + void Profile(llvm::FoldingSetNodeID &ID) const { Profile(ID, getTemplateName(), getDeducedType(), isDependentType()); } static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template, QualType Deduced, bool IsDependent) { Template.Profile(ID); -QualType CanonicalType = -Deduced.isNull() ? Deduced : Deduced.getCanonicalType(); -ID.AddPointer(CanonicalType.getAsOpaquePtr()); +Deduced.Profile(ID); ID.AddBoolean(IsDependent || Template.isDependent()); } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index bf74e56a14799..34aa399fda2f8
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
@@ -9434,6 +9505,32 @@ ASTContext::getSubstTemplateTemplateParmPack(const TemplateArgument &ArgPack, return TemplateName(Subst); } +/// Retrieve the template name that represents a template name +/// deduced from a specialization. +TemplateName +ASTContext::getDeducedTemplateName(TemplateName Underlying, + DefaultArguments DefaultArgs) const { + if (!DefaultArgs) +return Underlying; + + auto &Self = const_cast(*this); mizvekov wrote: This is done throughout the ASTContext and is not novel here. We wish to be able to create a new node from a const ASTContext reference. These creation functions mostly behave const-ish, they just use hash tables in the ASTContext for caching / identity purposes. The other alternative is to declare these hash tables as mutable, though this would necessitate a large noisy change to update everything at once. https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
@@ -6371,6 +6376,70 @@ ASTContext::getCanonicalTemplateName(const TemplateName &Name) const { canonArgPack, subst->getAssociatedDecl()->getCanonicalDecl(), subst->getFinal(), subst->getIndex()); } + case TemplateName::DeducedTemplate: { +assert(IgnoreDeduced == false); +DeducedTemplateStorage *DTS = Name.getAsDeducedTemplateName(); +DefaultArguments DefArgs = DTS->getDefaultArguments(); +TemplateName Underlying = DTS->getUnderlying(); + +bool NonCanonical = false; +TemplateName CanonUnderlying = +getCanonicalTemplateName(Underlying, /*IgnoreDeduced=*/true); +NonCanonical |= CanonUnderlying != Underlying; +auto CanonArgs = +getCanonicalTemplateArguments(*this, DefArgs.Args, NonCanonical); +{ + unsigned NumArgs = CanonArgs.size() - 1; + auto handleParamDefArg = [&](const TemplateArgument &ParamDefArg, + unsigned I) { +auto CanonParamDefArg = getCanonicalTemplateArgument(ParamDefArg); +TemplateArgument &CanonDefArg = CanonArgs[I]; +if (CanonDefArg.structurallyEquals(CanonParamDefArg)) + return; +if (I == NumArgs) + CanonArgs.pop_back(); +NonCanonical = true; + }; + auto handleParam = [&](auto *TP, int I) -> bool { +if (!TP->hasDefaultArgument()) + return true; +handleParamDefArg(TP->getDefaultArgument().getArgument(), I); +return false; + }; + + ArrayRef Params = CanonUnderlying.getAsTemplateDecl() + ->getTemplateParameters() + ->asArray(); + assert(CanonArgs.size() <= Params.size()); + for (int I = NumArgs; I >= 0; --I) { mizvekov wrote: The code is correct, but the variable name is not right. Should have been `LastArgIndex` or some such. https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
@@ -6371,6 +6376,70 @@ ASTContext::getCanonicalTemplateName(const TemplateName &Name) const { canonArgPack, subst->getAssociatedDecl()->getCanonicalDecl(), subst->getFinal(), subst->getIndex()); } + case TemplateName::DeducedTemplate: { +assert(IgnoreDeduced == false); +DeducedTemplateStorage *DTS = Name.getAsDeducedTemplateName(); +DefaultArguments DefArgs = DTS->getDefaultArguments(); +TemplateName Underlying = DTS->getUnderlying(); mizvekov wrote: They are pointer sized, it's the same as a QualType, and we never take those by reference either. https://github.com/llvm/llvm-project/pull/94981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/95202 >From dadb9244bee22bc303af154b47f527b973940b40 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Wed, 12 Jun 2024 00:42:48 -0300 Subject: [PATCH] [clang] fix broken canonicalization of DeducedTemplateSpecializationType This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root cause of #61317. A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST. The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules. The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either. --- No changelog entry because #61317 was already claimed as fixed in previous release. --- clang/include/clang/AST/ASTContext.h | 7 +++ clang/include/clang/AST/TemplateName.h | 4 +- clang/include/clang/AST/Type.h | 11 ++-- clang/lib/AST/ASTContext.cpp | 25 ++--- clang/lib/AST/TemplateName.cpp | 9 clang/unittests/AST/CMakeLists.txt | 1 + clang/unittests/AST/ProfilingTest.cpp | 75 ++ 7 files changed, 109 insertions(+), 23 deletions(-) create mode 100644 clang/unittests/AST/ProfilingTest.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index a1d1d1c51cd41..53ece996769a8 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1771,6 +1771,13 @@ class ASTContext : public RefCountedBase { QualType DeducedType, bool IsDependent) const; +private: + QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template, +QualType DeducedType, +bool IsDependent, +QualType Canon) const; + +public: /// Return the unique reference to the type for the specified TagDecl /// (struct/union/class/enum) decl. QualType getTagDeclType(const TagDecl *Decl) const; diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 988a55acd2252..24a7fde76195d 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -346,7 +346,9 @@ class TemplateName { /// error. void dump() const; - void Profile(llvm::FoldingSetNodeID &ID); + void Profile(llvm::FoldingSetNodeID &ID) { +ID.AddPointer(Storage.getOpaqueValue()); + } /// Retrieve the template name as a void pointer. void *getAsVoidPointer() const { return Storage.getOpaqueValue(); } diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 9eb3f6c09e3d3..fab233b62d8d1 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public DeducedType, DeducedTemplateSpecializationType(TemplateName Template, QualType DeducedAsType, -bool IsDeducedAsDependent) +bool IsDeducedAsDependent, QualType Canon) : DeducedType(DeducedTemplateSpecialization, DeducedAsType, toTypeDependence(Template.getDependence()) | (IsDeducedAsDependent ? TypeDependence::DependentInstantiation : TypeDependence::None), -DeducedAsType.isNull() ? QualType(this, 0) - : DeducedAsType.getCanonicalType()), +Canon), Template(Template) {} public: /// Retrieve the name of the template that we are deducing. TemplateName getTemplateName() const { return Template;} - void Profile(llvm::FoldingSetNodeID &ID) { + void Profile(llvm::FoldingSetNodeID &ID) const { Profile(ID, getTemplateName(), getDeducedType(), isDependentType()); } static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template, QualType Deduced, bool IsDependent) { Template.Profile(ID); -QualType CanonicalType = -Deduced.isNull() ? Deduced : Deduced.getCanonicalType(); -ID.AddPointer(CanonicalType.getAsOpaquePtr()); +Deduced.Profile(ID); ID.AddBoolean(IsDependent || Template.isDependent()); } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index bf74e56a14799..34aa399fda2f8
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
mizvekov wrote: I don't think regression tests are the right level here, we risk creating an unstable test. I have added a new unittest module for Profiling tests. I hope some day we will have time to add one test case for each property of each AST node. Though I still think the first line of defense here should have been code review. https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/95202 >From 5c4fa3ce2ce23fdaf877b71b2775244d15a149d3 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Wed, 12 Jun 2024 00:42:48 -0300 Subject: [PATCH] [clang] fix broken canonicalization of DeducedTemplateSpecializationType This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root cause of #61317. A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST. The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules. The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either. --- No changelog entry because #61317 was already claimed as fixed in previous release. --- clang/include/clang/AST/ASTContext.h | 4 ++ clang/include/clang/AST/TemplateName.h | 4 +- clang/include/clang/AST/Type.h | 11 ++-- clang/lib/AST/ASTContext.cpp | 25 ++--- clang/lib/AST/TemplateName.cpp | 9 clang/unittests/AST/CMakeLists.txt | 1 + clang/unittests/AST/ProfilingTest.cpp | 75 ++ 7 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 clang/unittests/AST/ProfilingTest.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index a1d1d1c51cd41..5985887000d44 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1770,6 +1770,10 @@ class ASTContext : public RefCountedBase { QualType getDeducedTemplateSpecializationType(TemplateName Template, QualType DeducedType, bool IsDependent) const; + QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template, +QualType DeducedType, +bool IsDependent, +QualType Canon) const; /// Return the unique reference to the type for the specified TagDecl /// (struct/union/class/enum) decl. diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 988a55acd2252..24a7fde76195d 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -346,7 +346,9 @@ class TemplateName { /// error. void dump() const; - void Profile(llvm::FoldingSetNodeID &ID); + void Profile(llvm::FoldingSetNodeID &ID) { +ID.AddPointer(Storage.getOpaqueValue()); + } /// Retrieve the template name as a void pointer. void *getAsVoidPointer() const { return Storage.getOpaqueValue(); } diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 9eb3f6c09e3d3..fab233b62d8d1 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -6050,30 +6050,27 @@ class DeducedTemplateSpecializationType : public DeducedType, DeducedTemplateSpecializationType(TemplateName Template, QualType DeducedAsType, -bool IsDeducedAsDependent) +bool IsDeducedAsDependent, QualType Canon) : DeducedType(DeducedTemplateSpecialization, DeducedAsType, toTypeDependence(Template.getDependence()) | (IsDeducedAsDependent ? TypeDependence::DependentInstantiation : TypeDependence::None), -DeducedAsType.isNull() ? QualType(this, 0) - : DeducedAsType.getCanonicalType()), +Canon), Template(Template) {} public: /// Retrieve the name of the template that we are deducing. TemplateName getTemplateName() const { return Template;} - void Profile(llvm::FoldingSetNodeID &ID) { + void Profile(llvm::FoldingSetNodeID &ID) const { Profile(ID, getTemplateName(), getDeducedType(), isDependentType()); } static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template, QualType Deduced, bool IsDependent) { Template.Profile(ID); -QualType CanonicalType = -Deduced.isNull() ? Deduced : Deduced.getCanonicalType(); -ID.AddPointer(CanonicalType.getAsOpaquePtr()); +Deduced.Profile(ID); ID.AddBoolean(IsDependent || Template.isDependent()); } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index bf74e56a14799..34aa399fda2f8 10064
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
mizvekov wrote: > Or, are you saying it is too hard to get reduced? > > I don't have a reduced test case. It's not impossible to reduce. Though we usually do a poor job of preserving TemplateNames in other places, which makes this harder to test. I have patches on my pipeline dealing with this though. https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
mizvekov wrote: It still fixes the original bug report though. Otherwise adding a test which directly tests all observable effects of the profiling fix would be a bit arbitrary, as we don't have any unit tests for the gazillion ways this could go wrong for the other AST nodes. This usually gets caught incidentally in diagnostics for unrelated tests. In this case that incidental test was in libc++. https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/95202 This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root cause of #61317. A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST. The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules. The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either. --- No changelog entry because #61317 was already claimed as fixed in previous release. >From 71a827e8ff6dcc47f1fd434a19070e6f841ee27a Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Wed, 12 Jun 2024 00:42:48 -0300 Subject: [PATCH] [clang] fix broken canonicalization of DeducedTemplateSpecializationType This reverts the functional elements of commit 3e78fa860235431323aaf08c8fa922d75a7cfffa and redoes it, by fixing the true root cause of #61317. A TemplateName can be non-canonical; profiling it based on the canonical name would result in inconsistent preservation of as-written information in the AST. The true problem in #61317 is that we would not consider the methods with requirements expression which contain DTSTs as the same in relation to merging of declarations when importing modules. The expressions would never match because they contained DTSTs pointing to different redeclarations of the same class template, but since canonicalization for them was broken, their canonical types would not match either. --- No changelog entry because #61317 was already claimed as fixed in previous release. --- clang/include/clang/AST/ASTContext.h | 4 clang/include/clang/AST/TemplateName.h | 4 +++- clang/include/clang/AST/Type.h | 9 +++-- clang/lib/AST/ASTContext.cpp | 25 +++-- clang/lib/AST/TemplateName.cpp | 9 - 5 files changed, 29 insertions(+), 22 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index a1d1d1c51cd41..5985887000d44 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1770,6 +1770,10 @@ class ASTContext : public RefCountedBase { QualType getDeducedTemplateSpecializationType(TemplateName Template, QualType DeducedType, bool IsDependent) const; + QualType getDeducedTemplateSpecializationTypeInternal(TemplateName Template, +QualType DeducedType, +bool IsDependent, +QualType Canon) const; /// Return the unique reference to the type for the specified TagDecl /// (struct/union/class/enum) decl. diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 988a55acd2252..24a7fde76195d 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -346,7 +346,9 @@ class TemplateName { /// error. void dump() const; - void Profile(llvm::FoldingSetNodeID &ID); + void Profile(llvm::FoldingSetNodeID &ID) { +ID.AddPointer(Storage.getOpaqueValue()); + } /// Retrieve the template name as a void pointer. void *getAsVoidPointer() const { return Storage.getOpaqueValue(); } diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 9eb3f6c09e3d3..f557697da74c6 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -6050,14 +6050,13 @@ class DeducedTemplateSpecializationType : public DeducedType, DeducedTemplateSpecializationType(TemplateName Template, QualType DeducedAsType, -bool IsDeducedAsDependent) +bool IsDeducedAsDependent, QualType Canon) : DeducedType(DeducedTemplateSpecialization, DeducedAsType, toTypeDependence(Template.getDependence()) | (IsDeducedAsDependent ? TypeDependence::DependentInstantiation : TypeDependence::None), -DeducedAsType.isNull() ? QualType(this, 0) - : DeducedAsType.getCanonicalType()), +Canon), Template(Template) {} public: @@ -6071,9 +6070,7 @@ class DeducedTemplateSpecializationType : public DeducedType, static void Profile(llvm::FoldingSetNodeID &ID, TemplateName
[clang] [Clang] Initialize AtLeastAsSpecialized to prevent undefined behavior in Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs() (PR #95195)
@@ -6447,7 +6447,7 @@ bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs( if (Inst.isInvalid()) return false; - bool AtLeastAsSpecialized; + bool AtLeastAsSpecialized = false; runWithSufficientStackSpace(Info.getLocation(), [&] { mizvekov wrote: runWithSufficientStackSpace is a small helper used to prevent stack exhaustion. https://github.com/llvm/llvm-project/pull/95195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Initialize AtLeastAsSpecialized to prevent undefined behavior in Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs() (PR #95195)
mizvekov wrote: There is no potential UB, this is a false positive: this lambda will always be executed before runWithSufficientStackSpace returns. https://github.com/llvm/llvm-project/pull/95195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StructuralEquivalence] improve NTTP and CXXDependentScopeMemberExpr comparison (PR #95190)
https://github.com/mizvekov approved this pull request. LGTM, Thanks! https://github.com/llvm/llvm-project/pull/95190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94981 >From d12c7d50b67cd669f09b3701ccf34154876786c9 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 8 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 59 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 129 -- clang/lib/AST/ASTDiagnostic.cpp | 24 +-- clang/lib/AST/ASTImporter.cpp | 15 ++ clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/ItaniumMangle.cpp | 11 ++ clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 157 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 128 -- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 60 +-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 25 files changed, 564 insertions(+), 208 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate); #undef TEMPLATE_KIND } llvm_unreachable("Unhandled NameKind enum"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index a366f1331c2d3..e6d16af2495fe 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1120,6 +1120,7 @@ class CollectExtraHighlightings case TemplateName::SubstTemplateTemplateParm: case TemplateName::SubstTemplateTemplateParmPack: case TemplateName::UsingTemplate: +case TemplateName::DeducedTemplate: // Names that could be resolved to a TemplateDecl are handled elsewhere. break; } diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 8bce4812f0d48..8818314de9364 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet SubstTemplateTemplateParmPacks; + mutable llvm::ContextualFoldingSet + DeducedTemplates; mutable llvm::ContextualFoldingSet ArrayParameterTypes; @@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase { unsigned Index,
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94981 >From 68791782b7a1a0eafa98950f6e03aa1585be5223 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 8 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 59 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 129 -- clang/lib/AST/ASTDiagnostic.cpp | 24 +-- clang/lib/AST/ASTImporter.cpp | 92 +- clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/ItaniumMangle.cpp | 11 ++ clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 157 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 128 -- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 60 +-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 25 files changed, 594 insertions(+), 255 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate); #undef TEMPLATE_KIND } llvm_unreachable("Unhandled NameKind enum"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index a366f1331c2d3..e6d16af2495fe 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1120,6 +1120,7 @@ class CollectExtraHighlightings case TemplateName::SubstTemplateTemplateParm: case TemplateName::SubstTemplateTemplateParmPack: case TemplateName::UsingTemplate: +case TemplateName::DeducedTemplate: // Names that could be resolved to a TemplateDecl are handled elsewhere. break; } diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 8bce4812f0d48..8818314de9364 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet SubstTemplateTemplateParmPacks; + mutable llvm::ContextualFoldingSet + DeducedTemplates; mutable llvm::ContextualFoldingSet ArrayParameterTypes; @@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase { unsigned Index,
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94981 >From ea98dec85a9817eb4e35ce97389433e4a5b9676d Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 8 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 59 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 129 -- clang/lib/AST/ASTDiagnostic.cpp | 24 +-- clang/lib/AST/ASTImporter.cpp | 92 +- clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/ItaniumMangle.cpp | 11 ++ clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 157 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 134 --- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 60 +-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 25 files changed, 600 insertions(+), 255 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate); #undef TEMPLATE_KIND } llvm_unreachable("Unhandled NameKind enum"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index a366f1331c2d3..e6d16af2495fe 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1120,6 +1120,7 @@ class CollectExtraHighlightings case TemplateName::SubstTemplateTemplateParm: case TemplateName::SubstTemplateTemplateParmPack: case TemplateName::UsingTemplate: +case TemplateName::DeducedTemplate: // Names that could be resolved to a TemplateDecl are handled elsewhere. break; } diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 8bce4812f0d48..8818314de9364 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet SubstTemplateTemplateParmPacks; + mutable llvm::ContextualFoldingSet + DeducedTemplates; mutable llvm::ContextualFoldingSet ArrayParameterTypes; @@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase { unsigned Index,
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94981 >From 015a05707caad5d39909bc68a5371161de464d9d Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 8 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 59 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 129 -- clang/lib/AST/ASTDiagnostic.cpp | 24 +-- clang/lib/AST/ASTImporter.cpp | 92 +- clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/ItaniumMangle.cpp | 11 ++ clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 157 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 134 --- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 44 +++-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 25 files changed, 584 insertions(+), 255 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate); #undef TEMPLATE_KIND } llvm_unreachable("Unhandled NameKind enum"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index a366f1331c2d3..e6d16af2495fe 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -1120,6 +1120,7 @@ class CollectExtraHighlightings case TemplateName::SubstTemplateTemplateParm: case TemplateName::SubstTemplateTemplateParmPack: case TemplateName::UsingTemplate: +case TemplateName::DeducedTemplate: // Names that could be resolved to a TemplateDecl are handled elsewhere. break; } diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 8bce4812f0d48..8818314de9364 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -262,6 +262,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet SubstTemplateTemplateParmPacks; + mutable llvm::ContextualFoldingSet + DeducedTemplates; mutable llvm::ContextualFoldingSet ArrayParameterTypes; @@ -2247,6 +2249,9 @@ class ASTContext : public RefCountedBase { unsigned Index,
[clang] [clang-tools-extra] [clang] Implement CWG2398 provisional TTP matching to class templates (PR #94981)
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/94981 This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. >From 28bc152e72d458e91aee90d46763eea04091ebe2 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 20 May 2024 01:15:03 -0300 Subject: [PATCH] [clang] Implement CWG2398 provisional TTP matching to class templates This extends default argument deduction to cover class templates as well, and also applies outside of partial ordering, adding to the provisional wording introduced in https://github.com/llvm/llvm-project/pull/89807. This solves some ambuguity introduced in P0522 regarding how template template parameters are partially ordered, and should reduce the negative impact of enabling `-frelaxed-template-template-args` by default. Given the following example: ```C++ template struct A; template struct B; template class TT1, class T5> struct B>; // #1 template struct B>; // #2 template struct B>; ``` Prior to P0522, `#2` was picked. Afterwards, this became ambiguous. This patch restores the pre-P0522 behavior, `#2` is picked again. As the consequences are not restricted to partial ordering, the following code becomes valid: ```C++ template struct A {}; A v; template class TT> void f(TT); // OK: TT picks 'float' as the default argument for the second parameter. void g() { f(v); } ``` Also, since 'f' deduced from `A` is different from 'f' deduced from `A`, this implements an additional mangling rule. --- Since this changes provisional implementation of CWG2398 which has not been released yet, and already contains a changelog entry, we don't provide a changelog entry here. --- clang-tools-extra/clangd/DumpAST.cpp | 1 + .../clangd/SemanticHighlighting.cpp | 1 + clang/include/clang/AST/ASTContext.h | 8 +- clang/include/clang/AST/ASTImporter.h | 5 + clang/include/clang/AST/DependenceFlags.h | 5 + clang/include/clang/AST/PropertiesBase.td | 17 ++ clang/include/clang/AST/TemplateName.h| 59 ++- clang/include/clang/Sema/Sema.h | 10 +- clang/lib/AST/ASTContext.cpp | 129 -- clang/lib/AST/ASTDiagnostic.cpp | 24 +-- clang/lib/AST/ASTImporter.cpp | 92 +- clang/lib/AST/ASTStructuralEquivalence.cpp| 3 + clang/lib/AST/ItaniumMangle.cpp | 11 ++ clang/lib/AST/ODRHash.cpp | 1 + clang/lib/AST/TemplateName.cpp| 157 ++ clang/lib/AST/TextNodeDumper.cpp | 12 ++ clang/lib/AST/Type.cpp| 3 +- clang/lib/Sema/SemaTemplate.cpp | 63 +-- clang/lib/Sema/SemaTemplateDeduction.cpp | 133 --- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 24 +-- .../CXX/temp/temp.decls/temp.alias/p2.cpp | 5 +- clang/test/CodeGenCXX/mangle-cwg2398.cpp | 11 ++ clang/test/SemaTemplate/cwg2398.cpp | 43 +++-- clang/tools/libclang/CIndex.cpp | 3 + clang/unittests/AST/ASTImporterTest.cpp | 17 ++ 25 files changed, 582 insertions(+), 255 deletions(-) create mode 100644 clang/test/CodeGenCXX/mangle-cwg2398.cpp diff --git a/clang-tools-extra/clangd/DumpAST.cpp b/clang-tools-extra/clangd/DumpAST.cpp index 9a525efb938e8..e605f82e91fe4 100644 --- a/clang-tools-extra/clangd/DumpAST.cpp +++ b/clang-tools-extra/clangd/DumpAST.cpp @@ -187,6 +187,7 @@ class DumpVisitor : public RecursiveASTVisitor { TEMPLATE_KIND(SubstTemplateTemplateParm); TEMPLATE_KIND(SubstTemplateTemplateParmPack); TEMPLATE_KIND(UsingTemplate); + TEMPLATE_KIND(DeducedTemplate);
[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)
https://github.com/mizvekov closed https://github.com/llvm/llvm-project/pull/94905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/94905 >From f918649a9208c2250873eb63641e106478371b43 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sat, 8 Jun 2024 11:07:27 -0300 Subject: [PATCH] [clang] improve TemplateArgument and TemplateName dump methods These will work as AST Text node dumpers, as usual, instead of AST printers. Note that for now, the TemplateName dumper is using the TemplateArgument dumper through an implicit conversion. This can be fixed in a later pass. --- clang/include/clang/AST/TemplateBase.h | 2 +- clang/include/clang/AST/TemplateName.h | 2 +- clang/lib/AST/ASTDumper.cpp| 34 ++ clang/lib/AST/TemplateBase.cpp | 9 --- clang/lib/AST/TemplateName.cpp | 11 - 5 files changed, 36 insertions(+), 22 deletions(-) diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h index fea2c8ccfee67..0eaa4b0eedb35 100644 --- a/clang/include/clang/AST/TemplateBase.h +++ b/clang/include/clang/AST/TemplateBase.h @@ -459,7 +459,7 @@ class TemplateArgument { bool IncludeType) const; /// Debugging aid that dumps the template argument. - void dump(raw_ostream &Out) const; + void dump(raw_ostream &Out, const ASTContext &Context) const; /// Debugging aid that dumps the template argument to standard error. void dump() const; diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 489fccb2ef74d..988a55acd2252 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -340,7 +340,7 @@ class TemplateName { Qualified Qual = Qualified::AsWritten) const; /// Debugging aid that dumps the template name. - void dump(raw_ostream &OS) const; + void dump(raw_ostream &OS, const ASTContext &Context) const; /// Debugging aid that dumps the template name to standard /// error. diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp index c8973fdeda352..f0603880c32dd 100644 --- a/clang/lib/AST/ASTDumper.cpp +++ b/clang/lib/AST/ASTDumper.cpp @@ -360,3 +360,37 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream &OS) const { ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors()); P.Visit(this); } + +//===--===// +// TemplateName method implementations +//===--===// + +// FIXME: These are actually using the TemplateArgument dumper, through +// an implicit conversion. The dump will claim this is a template argument, +// which is misleading. + +LLVM_DUMP_METHOD void TemplateName::dump() const { + ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false); + Dumper.Visit(*this); +} + +LLVM_DUMP_METHOD void TemplateName::dump(llvm::raw_ostream &OS, + const ASTContext &Context) const { + ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors()); + Dumper.Visit(*this); +} + +//===--===// +// TemplateArgument method implementations +//===--===// + +LLVM_DUMP_METHOD void TemplateArgument::dump() const { + ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false); + Dumper.Visit(*this); +} + +LLVM_DUMP_METHOD void TemplateArgument::dump(llvm::raw_ostream &OS, + const ASTContext &Context) const { + ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors()); + Dumper.Visit(*this); +} diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index 46f7b79b272ef..2e6839e948d9d 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -577,15 +577,6 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out, } } -void TemplateArgument::dump(raw_ostream &Out) const { - LangOptions LO; // FIXME! see also TemplateName::dump(). - LO.CPlusPlus = true; - LO.Bool = true; - print(PrintingPolicy(LO), Out, /*IncludeType*/ true); -} - -LLVM_DUMP_METHOD void TemplateArgument::dump() const { dump(llvm::errs()); } - //===--===// // TemplateArgumentLoc Implementation //===--===// diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp index 4fc25cb34803e..11544dbb56e31 100644 --- a/clang/lib/AST/TemplateName.cpp +++ b/clang/lib/AST/TemplateName.cpp @@ -360,14 +360,3 @@ const StreamingDiagnostic &clang::operator<<(const StreamingDiagnostic &DB, OS.flush(); return DB << NameStr; } - -void TemplateName::dump(raw_ostream &OS) const { - LangOptions LO; // FIXME! - LO.CPlusPlus = true; - LO.Bool = true; - print(OS, Pri
[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)
@@ -360,3 +360,34 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream &OS) const { ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors()); P.Visit(this); } + +//===--===// +// TemplateName method implementations +//===--===// + +// FIXME: These are using the TemplateArgument dumper. mizvekov wrote: Well you are going to hit dump on a TemplateName, and it's going to dump a TemplateArgument of template kind, which is misleading and incorrect, but still helpful in the narrow context this is a debugging aid not actually used in production. https://github.com/llvm/llvm-project/pull/94905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: improve TemplateArgument and TemplateName dump methods (PR #94905)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/94905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] improve TemplateArgument and TemplateName dump methods (PR #94905)
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/94905 These will work as AST Text node dumpers, as usual, instead of AST printers. Note that for now, the TemplateName dumper is using the TemplateArgument dumper through an implicit conversion. This can be fixed in a later pass. >From fb159ba1974e9311818892950c301f590bbe6360 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sat, 8 Jun 2024 11:07:27 -0300 Subject: [PATCH] [clang] improve TemplateArgument and TemplateName dump methods These will work as AST Text node dumpers, as usual, instead of AST printers. Note that for now, the TemplateName dumper is using the TemplateArgument dumper through an implicit conversion. This can be fixed in a later pass. --- clang/include/clang/AST/TemplateBase.h | 2 +- clang/include/clang/AST/TemplateName.h | 2 +- clang/lib/AST/ASTDumper.cpp| 31 ++ clang/lib/AST/TemplateBase.cpp | 9 clang/lib/AST/TemplateName.cpp | 11 - 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h index fea2c8ccfee67..0eaa4b0eedb35 100644 --- a/clang/include/clang/AST/TemplateBase.h +++ b/clang/include/clang/AST/TemplateBase.h @@ -459,7 +459,7 @@ class TemplateArgument { bool IncludeType) const; /// Debugging aid that dumps the template argument. - void dump(raw_ostream &Out) const; + void dump(raw_ostream &Out, const ASTContext &Context) const; /// Debugging aid that dumps the template argument to standard error. void dump() const; diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h index 489fccb2ef74d..988a55acd2252 100644 --- a/clang/include/clang/AST/TemplateName.h +++ b/clang/include/clang/AST/TemplateName.h @@ -340,7 +340,7 @@ class TemplateName { Qualified Qual = Qualified::AsWritten) const; /// Debugging aid that dumps the template name. - void dump(raw_ostream &OS) const; + void dump(raw_ostream &OS, const ASTContext &Context) const; /// Debugging aid that dumps the template name to standard /// error. diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp index c8973fdeda352..869527241f2c8 100644 --- a/clang/lib/AST/ASTDumper.cpp +++ b/clang/lib/AST/ASTDumper.cpp @@ -360,3 +360,34 @@ LLVM_DUMP_METHOD void ConceptReference::dump(raw_ostream &OS) const { ASTDumper P(OS, Ctx, Ctx.getDiagnostics().getShowColors()); P.Visit(this); } + +//===--===// +// TemplateName method implementations +//===--===// + +// FIXME: These are using the TemplateArgument dumper. +LLVM_DUMP_METHOD void TemplateName::dump() const { + ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false); + Dumper.Visit(*this); +} + +LLVM_DUMP_METHOD void TemplateName::dump(llvm::raw_ostream &OS, + const ASTContext &Context) const { + ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors()); + Dumper.Visit(*this); +} + +//===--===// +// TemplateArgument method implementations +//===--===// + +LLVM_DUMP_METHOD void TemplateArgument::dump() const { + ASTDumper Dumper(llvm::errs(), /*ShowColors=*/false); + Dumper.Visit(*this); +} + +LLVM_DUMP_METHOD void TemplateArgument::dump(llvm::raw_ostream &OS, + const ASTContext &Context) const { + ASTDumper Dumper(OS, Context, Context.getDiagnostics().getShowColors()); + Dumper.Visit(*this); +} diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index 46f7b79b272ef..2e6839e948d9d 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -577,15 +577,6 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out, } } -void TemplateArgument::dump(raw_ostream &Out) const { - LangOptions LO; // FIXME! see also TemplateName::dump(). - LO.CPlusPlus = true; - LO.Bool = true; - print(PrintingPolicy(LO), Out, /*IncludeType*/ true); -} - -LLVM_DUMP_METHOD void TemplateArgument::dump() const { dump(llvm::errs()); } - //===--===// // TemplateArgumentLoc Implementation //===--===// diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp index 4fc25cb34803e..11544dbb56e31 100644 --- a/clang/lib/AST/TemplateName.cpp +++ b/clang/lib/AST/TemplateName.cpp @@ -360,14 +360,3 @@ const StreamingDiagnostic &clang::operator<<(const StreamingDiagnostic &DB, OS.flush(); return DB << NameStr; } - -void TemplateName::dump(raw_ost
[clang] e090bac - [clang] NFC: add new cwg2398 tests
Author: Matheus Izvekov Date: 2024-06-09T13:44:45-03:00 New Revision: e090bac638e56ff9db87e622cdf925f2b99dfc30 URL: https://github.com/llvm/llvm-project/commit/e090bac638e56ff9db87e622cdf925f2b99dfc30 DIFF: https://github.com/llvm/llvm-project/commit/e090bac638e56ff9db87e622cdf925f2b99dfc30.diff LOG: [clang] NFC: add new cwg2398 tests Added: Modified: clang/test/SemaTemplate/cwg2398.cpp Removed: diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp index f7f69e9d4268a..7675d4287cb88 100644 --- a/clang/test/SemaTemplate/cwg2398.cpp +++ b/clang/test/SemaTemplate/cwg2398.cpp @@ -200,8 +200,120 @@ namespace consistency { template struct A, B, B>; // new-error@-1 {{ambiguous partial specializations}} } // namespace t2 + namespace t3 { +template struct A; + +template class TT1, + class T1, class T2, class T3, class T4> +struct A, TT1, typename nondeduced>::type> {}; +// new-note@-1 {{partial specialization matches}} + +template class UU1, + class U1, class U2> +struct A, UU1, typename nondeduced>::type>; +// new-note@-1 {{partial specialization matches}} + +template struct A, B, B>; +// new-error@-1 {{ambiguous partial specializations}} + } // namespace t3 + namespace t4 { +template struct A; + +template class TT1, + class T1, class T2, class T3, class T4> +struct A, TT1, typename nondeduced>::type> {}; +// new-note@-1 {{partial specialization matches}} + +template class UU1, + class U1, class U2> +struct A, UU1, typename nondeduced>::type>; +// new-note@-1 {{partial specialization matches}} + +template struct A, B, B>; +// new-error@-1 {{ambiguous partial specializations}} + } // namespace t4 + namespace t5 { +template struct A; + +template class TT1, + class T1, class T2, class T3, class T4> +struct A, TT1> {}; +// new-note@-1 {{partial specialization matches}} + +template class UU1, + class U1, class U2> +struct A, UU1>; +// new-note@-1 {{partial specialization matches}} + +template struct A, B>; +// new-error@-1 {{ambiguous partial specializations}} + } // namespace t5 + namespace t6 { +template struct A; + +template class TT1, + class T1, class T2, class T3> +struct A, TT1> {}; +// new-note@-1 {{partial specialization matches}} + +template class UU1, + class U1, class U2> +struct A, UU1>; +// new-note@-1 {{partial specialization matches}} + +template struct A, B>; +// new-error@-1 {{ambiguous partial specializations}} + } // namespace t6 } // namespace consistency +namespace classes { + namespace canon { +template struct A {}; + +template class TT> auto f(TT a) { return a; } +// old-note@-1 2{{template template argument has diff erent template parameters}} +// new-note@-2 2{{substitution failure: too few template arguments}} + +A v1; +A v2; + +using X = decltype(f(v1)); +// expected-error@-1 {{no matching function for call}} + +using X = decltype(f(v2)); +// expected-error@-1 {{no matching function for call}} + } // namespace canon + namespace expr { +template struct A { + static constexpr auto val = E1; +}; +template class TT> void f(TT v) { + // old-note@-1 {{template template argument has diff erent template parameters}} + // new-note@-2 {{substitution failure: too few template arguments}} + static_assert(v.val == 3); +}; +void test() { + f(A()); + // expected-error@-1 {{no matching function for call}} +} + } // namespace expr + namespace packs { +template struct A { + static constexpr auto val = sizeof...(T2s); +}; + +template class TT> void f(TT v) { + // old-note@-1 {{template template argument has diff erent template parameters}} + // new-note@-2 {{deduced type 'A<[...], (no argument), (no argument), (no argument)>' of 1st parameter does not match adjusted type 'A<[...], void, void, void>' of argument [with TT = A]}} + static_assert(v.val == 3); +}; +void test() { + f(A()); + // expected-error@-1 {{no matching function for call}} +} + } // namespace packs +} // namespace classes + namespace regression1 { template struct map {}; template class foo {}; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Enable LLDB tests in Linux pre-merge CI (PR #94208)
https://github.com/mizvekov approved this pull request. https://github.com/llvm/llvm-project/pull/94208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Enable LLDB tests in Linux pre-merge CI (PR #94208)
mizvekov wrote: @Endilll This looks good, thanks! How will this affect test capacity? Right now, the Linux bots are lagging, while the Windows bot is breezing through. This is the opposite of the usual. Are we under-provisioned on Linux CI resources? How much worse will it get when we add this extra workload? https://github.com/llvm/llvm-project/pull/94208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] always use resolved arguments for default argument deduction (PR #94756)
https://github.com/mizvekov closed https://github.com/llvm/llvm-project/pull/94756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -390,29 +390,37 @@ bool Sema::isAcceptableNestedNameSpecifier(const NamedDecl *SD, /// (e.g., Base::), perform name lookup for that identifier as a /// nested-name-specifier within the given scope, and return the result of that /// name lookup. -NamedDecl *Sema::FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS) { - if (!S || !NNS) -return nullptr; +bool Sema::LookupFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS, + UnresolvedSetImpl &R) { + if (!S) +return false; while (NNS->getPrefix()) NNS = NNS->getPrefix(); - if (NNS->getKind() != NestedNameSpecifier::Identifier) -return nullptr; - - LookupResult Found(*this, NNS->getAsIdentifier(), SourceLocation(), - LookupNestedNameSpecifierName); + // FIXME: This is a rather nasty hack! Ideally we should get the results mizvekov wrote: I don't think this is a hack per se, I think this is just a consequence of not having a special NNS kind for this situation, and representing it with a DTST. The alternative that I see is to implement a new NNS prefix which is composed by an identifier followed by template arguments. It can still be represented internally with the type, if that's cheaper, but having a special accessor, and making `NNS->getAsIdentifier()` work for it, would be nicer. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -548,6 +575,7 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S, NestedNameSpecInfo &IdInfo, // Perform unqualified name lookup in the current scope. LookupName(Found, S); } +#endif mizvekov wrote: A left-over. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/mizvekov approved this pull request. LGTM save a few minor cleanups needed. I think this patch has an incredible amount of noise due to amount of cosmetic changes caused by reformatting, specially due to changing parameters and such. This is fine for now and I am not suggesting you go back and do it for this patch, but I think it would have been helpful to pre-clang-format the file, and offload more of the minor changes into a previous patch. https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
@@ -2923,10 +2920,9 @@ class TreeTransform { } return getSema().BuildMemberReferenceExpr(Base, BaseType, OpLoc, isArrow, - SS, TemplateKWLoc, - FirstQualifierInScope, - R, ExplicitTemplateArgs, - /*S*/nullptr); + SS, TemplateKWLoc, R, + ExplicitTemplateArgs, + /*S*/ nullptr); mizvekov wrote: ```suggestion /*S=*/nullptr); ``` https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [Clang] Implement resolution for CWG1835 (PR #92957)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/92957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] always use resolved arguments for default argument deduction (PR #94756)
mizvekov wrote: @kadircet @yozhu FYI this fixes the problem you reported. https://github.com/llvm/llvm-project/pull/94756 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] always use resolved arguments for default argument deduction (PR #94756)
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/94756 This fixes a regression introduced with the changes in https://github.com/llvm/llvm-project/pull/93433 around preservation of TemplateName sugar in template type deduction. Since the argument side TST is non-canonical, we have to extract the arguments from it's canonical type. This was done for the deduction of the TST arguments, but we missed it for the default arguments used in the deduction of the TST name. >From df6049fd91c95e341dd80a61e5bd173ce5837131 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Fri, 7 Jun 2024 10:32:12 -0300 Subject: [PATCH] [clang] always use resolved arguments for default argument deduction This fixes a regression introduced with the changes in https://github.com/llvm/llvm-project/pull/93433 around preservation of TemplateName sugar in template type deduction. Since the argument side TST is non-canonical, we have to extract the arguments from it's canonical type. This was done for the deduction of the TST arguments, but we missed it for the default arguments used in the deduction of the TST name. --- clang/lib/Sema/SemaTemplateDeduction.cpp | 13 ++--- clang/test/SemaTemplate/cwg2398.cpp | 16 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp index 1011db2d2830d..befeb38e1fe5b 100644 --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -712,13 +712,6 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams, if (const auto *TD = TNA.getAsTemplateDecl(); TD && TD->isTypeAlias()) return TemplateDeductionResult::Success; -// Perform template argument deduction for the template name. -if (auto Result = -DeduceTemplateArguments(S, TemplateParams, TNP, TNA, Info, -SA->template_arguments(), Deduced); -Result != TemplateDeductionResult::Success) - return Result; - // FIXME: To preserve sugar, the TST needs to carry sugared resolved // arguments. ArrayRef AResolved = @@ -726,6 +719,12 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams, ->castAs() ->template_arguments(); +// Perform template argument deduction for the template name. +if (auto Result = DeduceTemplateArguments(S, TemplateParams, TNP, TNA, Info, + AResolved, Deduced); +Result != TemplateDeductionResult::Success) + return Result; + // Perform template argument deduction on each template // argument. Ignore any missing/extra arguments, since they could be // filled in by default arguments. diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp index 45e74cce3a98c..f7f69e9d4268a 100644 --- a/clang/test/SemaTemplate/cwg2398.cpp +++ b/clang/test/SemaTemplate/cwg2398.cpp @@ -201,3 +201,19 @@ namespace consistency { // new-error@-1 {{ambiguous partial specializations}} } // namespace t2 } // namespace consistency + +namespace regression1 { + template struct map {}; + template class foo {}; + + template class MapType, typename Value> + Value bar(MapType map); + + template class MapType, typename Value> + Value bar(MapType> map); + + void aux() { +map> input; +bar(input); + } +} // namespace regression1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
mizvekov wrote: > so after this patch, clang seems to be crashing on: Thanks for the reproducer. Yeah this is about default argument deduction, which doesn't involve default arguments as written in source. I confirm the crash and it's indeed caused by changes in this patch. The fix is very simple and I will be posting a patch shortly. https://github.com/llvm/llvm-project/pull/93433 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema][TemplateDeduction] Skip pack expansion type at the end of default template argument list if unneeded (PR #94659)
https://github.com/mizvekov requested changes to this pull request. Please include test case. https://github.com/llvm/llvm-project/pull/94659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits