Skip to content

[Clang][P1061] Fix template arguments in local classes #121225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 13, 2025
Merged

[Clang][P1061] Fix template arguments in local classes #121225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,7 @@ Bug Fixes to C++ Support
consistently treat the initializer as manifestly constant-evaluated.
(#GH135281)
- Fix a crash in the presence of invalid base classes. (#GH147186)
- Fix a crash with NTTP when instantiating local class.

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
6 changes: 5 additions & 1 deletion clang/lib/Sema/SemaTemplateInstantiate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4412,8 +4412,12 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation,
// No need to instantiate in-class initializers during explicit
// instantiation.
if (Field->hasInClassInitializer() && TSK == TSK_ImplicitInstantiation) {
// Handle local classes which could have substituted template params.
CXXRecordDecl *ClassPattern =
Instantiation->getTemplateInstantiationPattern();
Instantiation->isLocalClass()
? Instantiation->getInstantiatedFromMemberClass()
: Instantiation->getTemplateInstantiationPattern();
Copy link
Contributor

@mizvekov mizvekov Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main point of getTemplateInstantiationPattern is to figure out the instantiation pattern as the name suggests, it would be better to fix the problem within it.

Copy link
Contributor Author

@ricejasonf ricejasonf Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments for that states

  /// Retrieve the record declaration from which this record could be
  /// instantiated. Returns null if this class is not a template instantiation.
  const CXXRecordDecl *getTemplateInstantiationPattern() const;

So the "pattern" as I read it, is not an instantiation itself but the template (or partial specialization). In the case of the field in struct L we need the intermediate instantiated struct to get the instantiated field.

Copy link
Contributor

@mizvekov mizvekov Jul 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the pattern is the corpus of the template, the parts which needs to be have template parameters replaced in order to produce an instantiation.

I think what I said still stands.


DeclContext::lookup_result Lookup =
ClassPattern->lookup(Field->getDeclName());
FieldDecl *Pattern = Lookup.find_first<FieldDecl>();
Expand Down
14 changes: 14 additions & 0 deletions clang/test/SemaCXX/local-class-template-param-crash.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %clang_cc1 -fsyntax-only %s -verify
// expected-no-diagnostics

template <int i>
int g() {
return [] (auto) -> int {
struct L {
int m = i;
};
return 0;
} (42);
Comment on lines +6 to +11
Copy link
Contributor

@zyn0217 zyn0217 Dec 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is some fallout of not decoupling instantiating lambda body from the lambda transformation. If we had been able to avoid the body transform when transforming the lambda for the call expression, we could also have avoided instantiating an offending intermediate L, right? @erichkeane

Copy link
Contributor

@cor3ntin cor3ntin Dec 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems about right.
But the fix is probably fine? Maybe we can add a FIXME in the comment for now.

Copy link
Contributor

@zyn0217 zyn0217 Dec 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably also worth an assertion for which we’re within a lambda.

}

int v = g<1>();