Skip to content
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.

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

[clang] Add frontend flag to enable support for broken external resugarers #103219

Merged
merged 4 commits into from
Aug 13, 2024

Conversation

ymand
Copy link
Collaborator

@ymand ymand commented Aug 13, 2024

Forked from #102510 by mizvekov. Changes are captured as a fixup commit.

There are some external projects that can't rely on our own sugar
propagation for templated entities, because they need to resugar types
which only exist within their framework, and so are entirely invisible
to our internal tooling.

This new flag is meant to prevent our transforms from removing any Subst*
nodes.

For this, this is wired only to template type alias subsititutions.

Note that our AST does represent enough information to correctly
resugar template type alias, so any users of this are limited in their
capacity to reconstruct the parameter substitutions fully.

mizvekov and others added 2 commits August 13, 2024 14:59
…arers

There are some external projects that can't rely on our own sugar
propagation for templated entities, because they need to resugar types
which only exist within their framework, and so are entirely invisible
to our internal tooling.

This new flag is meant to prevent our transforms from removing any Subst*
nodes.

For this, this is wired only to template type alias subsititutions.

Note that our AST does represent enough information to correctly
resugar template type alias, so any users of this are either defective
or working under extremely limited scenarios.
…l resugarers

rename flag and adjust the comments
@ymand ymand marked this pull request as ready for review August 13, 2024 15:02
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 13, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 13, 2024

@llvm/pr-subscribers-clang

Author: Yitzhak Mandelbaum (ymand)

Changes

Forked from #102510 by mizvekov. Changes are captured as a fixup commit.

There are some external projects that can't rely on our own sugar
propagation for templated entities, because they need to resugar types
which only exist within their framework, and so are entirely invisible
to our internal tooling.

This new flag is meant to prevent our transforms from removing any Subst*
nodes.

For this, this is wired only to template type alias subsititutions.

Note that our AST does represent enough information to correctly
resugar template type alias, so any users of this are limited in their
capacity to reconstruct the parameter substitutions fully.


Full diff: https://github.com/llvm/llvm-project/pull/103219.diff

4 Files Affected:

  • (modified) clang/include/clang/Basic/LangOptions.def (+1)
  • (modified) clang/include/clang/Driver/Options.td (+6)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+9-3)
  • (added) clang/test/AST/ast-dump-support-broken-external-resugarers.cpp (+18)
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index ca9c00a1473bbd..6086f0ee73d27c 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -162,6 +162,7 @@ LANGOPT(CoroAlignedAllocation, 1, 0, "prefer Aligned Allocation according to P20
 LANGOPT(DllExportInlines  , 1, 1, "dllexported classes dllexport inline methods")
 LANGOPT(RelaxedTemplateTemplateArgs, 1, 1, "C++17 relaxed matching of template template arguments")
 LANGOPT(ExperimentalLibrary, 1, 0, "enable unstable and experimental library features")
+LANGOPT(RetainSubstTemplateTypeParmTypeAstNodes, 1, 0, "retain SubstTemplateTypeParmType nodes in the AST's representationof alias template specializations")
 
 LANGOPT(PointerAuthIntrinsics, 1, 0, "pointer authentication intrinsics")
 LANGOPT(PointerAuthCalls  , 1, 0, "function pointer authentication")
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 75320cafaefa5f..6df3a6a5943a97 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3455,6 +3455,12 @@ defm relaxed_template_template_args : BoolFOption<"relaxed-template-template-arg
   PosFlag<SetTrue, [], [], "Enable">,
   NegFlag<SetFalse, [], [CC1Option], "Disable">,
   BothFlags<[], [ClangOption], " C++17 relaxed template template argument matching">>;
+defm retain_subst_template_type_parm_type_ast_nodes : BoolFOption<"retain-subst-template-type-parm-type-ast-nodes",
+  LangOpts<"RetainSubstTemplateTypeParmTypeAstNodes">, DefaultFalse,
+  PosFlag<SetTrue, [], [CC1Option], "Enable">,
+  NegFlag<SetFalse, [], [], "Disable">,
+  BothFlags<[], [], " retain SubstTemplateTypeParmType nodes in the AST's representation"
+  " of alias template specializations">>;
 defm sized_deallocation : BoolFOption<"sized-deallocation",
   LangOpts<"SizedDeallocation">, Default<cpp14.KeyPath>,
   PosFlag<SetTrue, [], [], "Enable C++14 sized global deallocation functions">,
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 29e7978ba5b1f8..876921a6b311d4 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3332,10 +3332,16 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
     if (Pattern->isInvalidDecl())
       return QualType();
 
-    // Only substitute for the innermost template argument list.
+    // Only substitute for the innermost template argument list.  NOTE: Some
+    // external resugarers rely on leaving a Subst* node here.  Make the
+    // substitution non-final in that case.  Note that these external resugarers
+    // will still miss some information in this representation, because we don't
+    // provide enough context in the Subst* nodes in order to tell different
+    // template type alias specializations apart.
     MultiLevelTemplateArgumentList TemplateArgLists;
-    TemplateArgLists.addOuterTemplateArguments(Template, SugaredConverted,
-                                               /*Final=*/true);
+    TemplateArgLists.addOuterTemplateArguments(
+        Template, SugaredConverted,
+        /*Final=*/!getLangOpts().RetainSubstTemplateTypeParmTypeAstNodes);
     TemplateArgLists.addOuterRetainedLevels(
         AliasTemplate->getTemplateParameters()->getDepth());
 
diff --git a/clang/test/AST/ast-dump-support-broken-external-resugarers.cpp b/clang/test/AST/ast-dump-support-broken-external-resugarers.cpp
new file mode 100644
index 00000000000000..de1a1051a09933
--- /dev/null
+++ b/clang/test/AST/ast-dump-support-broken-external-resugarers.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -fretain-subst-template-type-parm-type-ast-nodes -ast-dump -ast-dump-filter=dump %s | FileCheck -strict-whitespace %s
+
+namespace t1 {
+template<class T> using X = T;
+using dump = X<int>;
+
+// CHECK-LABEL: Dumping t1::dump:
+// CHECK-NEXT:  TypeAliasDecl
+// CHECK-NEXT:  `-ElaboratedType
+// CHECK-NEXT:    `-TemplateSpecializationType
+// CHECK-NEXT:      |-name: 'X':'t1::X' qualified
+// CHECK-NEXT:      | `-TypeAliasTemplateDecl
+// CHECK-NEXT:      |-TemplateArgument
+// CHECK-NEXT:      | `-BuiltinType {{.+}} 'int'
+// CHECK-NEXT:      `-SubstTemplateTypeParmType 0x{{[0-9]+}} 'int' sugar class depth 0 index 0 T
+// CHECK-NEXT:        |-TypeAliasTemplate {{.+}} 'X'
+// CHECK-NEXT:        `-BuiltinType {{.+}} 'int'
+} // namespace t1

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

CC @mizvekov for awareness

@ymand ymand merged commit 661dda9 into llvm:main Aug 13, 2024
4 of 5 checks passed
bwendling pushed a commit to bwendling/llvm-project that referenced this pull request Aug 15, 2024
…arers (llvm#103219)

Forked from llvm#102510 by
[mizvekov](https://github.com/mizvekov). Changes are captured as a fixup
commit.

There are some external projects that can't rely on our own sugar
propagation for templated entities, because they need to resugar types
which only exist within their framework, and so are entirely invisible
to our internal tooling.

This new flag is meant to prevent our transforms from removing any
Subst*
nodes.

For this, this is wired only to template type alias subsititutions.

Note that our AST does represent enough information to correctly
resugar template type alias, so any users of this are limited in their 
capacity to reconstruct the parameter substitutions fully.

---------

Co-authored-by: Matheus Izvekov <mizvekov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants