[clang] Reland: Instantiate alias templates with sugar #101858
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis makes use of the changes introduced in D134604, in order to instantiate alias templates witn a final sugared substitution. This comes at no additional relevant cost. Differential Revision: https://reviews.llvm.org/D136565 Patch is 31.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101858.diff 15 Files Affected:
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 8d6d4223d7260..69f6df46c87ce 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1325,7 +1325,7 @@ class Foo final {})cpp";
HI.LocalScope = "";
HI.Kind = index::SymbolKind::TypeAlias;
HI.Definition = "template <typename T> using AA = A<T>";
- HI.Type = {"A<T>", "type-parameter-0-0"}; // FIXME: should be 'T'
+ HI.Type = {"A<T>", "T"};
HI.TemplateParameters = {
{{"typename"}, std::string("T"), std::nullopt}};
}},
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp
index 3fdc24b94a6cb..721c55b1fb538 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unused-local-non-trivial-variable.cpp
@@ -76,7 +76,7 @@ T qux(T Generic) {
async::Future<T> TemplateType;
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: unused local variable 'TemplateType' of type 'async::Future<T>' [bugprone-unused-local-non-trivial-variable]
a::Future<T> AliasTemplateType;
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: unused local variable 'AliasTemplateType' of type 'a::Future<T>' (aka 'Future<type-parameter-0-0>') [bugprone-unused-local-non-trivial-variable]
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: unused local variable 'AliasTemplateType' of type 'a::Future<T>' (aka 'Future<T>') [bugprone-unused-local-non-trivial-variable]
[[maybe_unused]] async::Future<Units> MaybeUnused;
return Generic;
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 720862ec0bffa..1346a4a3f0012 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3334,8 +3334,8 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
// Only substitute for the innermost template argument list.
MultiLevelTemplateArgumentList TemplateArgLists;
- TemplateArgLists.addOuterTemplateArguments(Template, CanonicalConverted,
- /*Final=*/false);
+ TemplateArgLists.addOuterTemplateArguments(Template, SugaredConverted,
+ /*Final=*/true);
TemplateArgLists.addOuterRetainedLevels(
AliasTemplate->getTemplateParameters()->getDepth());
diff --git a/clang/test/AST/ast-dump-template-decls.cpp b/clang/test/AST/ast-dump-template-decls.cpp
index f0a6204ce3cfa..9f578e5afe561 100644
--- a/clang/test/AST/ast-dump-template-decls.cpp
+++ b/clang/test/AST/ast-dump-template-decls.cpp
@@ -123,8 +123,6 @@ using type2 = typename C<int>::type1<void>;
// CHECK-NEXT: TemplateArgument type 'void'
// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
// CHECK-NEXT: FunctionProtoType 0x{{[^ ]*}} 'void (int)' cdecl
-// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'void' sugar class depth 0 index 0 U
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'type1'
// CHECK-NEXT: BuiltinType 0x{{[^ ]*}} 'void'
// CHECK-NEXT: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 index 0 T
// CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'C'
@@ -132,39 +130,24 @@ using type2 = typename C<int>::type1<void>;
} // namespace PR55886
namespace PR56099 {
-template <typename... As> struct Y;
-template <typename... Bs> using Z = Y<Bs...>;
-template <typename... Cs> struct foo {
- template <typename... Ds> using bind = Z<Ds..., Cs...>;
-};
-using t1 = foo<int, short>::bind<char, float>;
-// CHECK: TemplateSpecializationType 0x{{[^ ]*}} 'Y<char, float, int, short>' sugar
-// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar typename depth 0 index 0 ... Bs pack_index 3
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar typename depth 0 index 0 ... Bs pack_index 2
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar typename depth 0 index 0 ... Bs pack_index 1
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar typename depth 0 index 0 ... Bs pack_index 0
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'Z'
-
template <typename... T> struct D {
- template <typename... U> using B = int(int (*...p)(T, U));
+ template <typename... U> struct bind {
+ using bound_type = int(int (*...p)(T, U));
+ };
};
-using t2 = D<float, char>::B<int, short>;
-// CHECK: TemplateSpecializationType 0x{{[^ ]*}} 'B<int, short>' sugar alias{{$}}
-// CHECK-NEXT: name: 'D<float, char>::B':'PR56099::D<float, char>::B' qualified
+template struct D<float, char>::bind<int, short>;
+// CHECK: TypeAliasDecl 0x{{[^ ]*}} <line:{{[1-9]+}}:5, col:45> col:11 bound_type 'int (int (*)(float, int), int (*)(char, short))'
// CHECK: FunctionProtoType 0x{{[^ ]*}} 'int (int (*)(float, int), int (*)(char, short))' cdecl
// CHECK: FunctionProtoType 0x{{[^ ]*}} 'int (float, int)' cdecl
// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar typename depth 0 index 0 ... T pack_index 1
// CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'D'
// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar typename depth 0 index 0 ... U pack_index 1
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'B'
+// CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'bind'
// CHECK: FunctionProtoType 0x{{[^ ]*}} 'int (char, short)' cdecl
// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar typename depth 0 index 0 ... T pack_index 0
// CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'D'
// CHECK: SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar typename depth 0 index 0 ... U pack_index 0
-// CHECK-NEXT: TypeAliasTemplate 0x{{[^ ]*}} 'B'
+// CHECK-NEXT: ClassTemplateSpecialization 0x{{[^ ]*}} 'bind'
} // namespace PR56099
namespace subst_default_argument {
@@ -178,9 +161,7 @@ using test1 = D<E, int>;
// CHECK-NEXT: |-name: 'A':'subst_default_argument::A' qualified
// CHECK-NEXT: | `-ClassTemplateDecl {{.+}} A
// CHECK-NEXT: |-TemplateArgument type 'int'
-// CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar class depth 0 index 1 D2
-// CHECK-NEXT: | |-TypeAliasTemplate 0x{{[^ ]*}} 'D'
-// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int'
+// CHECK-NEXT: | `-BuiltinType 0x{{[^ ]*}} 'int'
// CHECK-NEXT: `-RecordType 0x{{[^ ]*}} 'subst_default_argument::A<int>'
// CHECK-NEXT: `-ClassTemplateSpecialization 0x{{[^ ]*}} 'A'
} // namespace subst_default_argument
diff --git a/clang/test/CXX/temp/temp.deduct.guide/p3.cpp b/clang/test/CXX/temp/temp.deduct.guide/p3.cpp
index cdc073066f3e3..c5404847beb06 100644
--- a/clang/test/CXX/temp/temp.deduct.guide/p3.cpp
+++ b/clang/test/CXX/temp/temp.deduct.guide/p3.cpp
@@ -33,7 +33,7 @@ template<template<typename> typename TT> struct E { // expected-note 2{{template
};
A(int) -> int; // expected-error {{deduced type 'int' of deduction guide is not a specialization of template 'A'}}
-template<typename T> A(T) -> B<T>; // expected-error {{deduced type 'B<T>' (aka 'A<type-parameter-0-0>') of deduction guide is not written as a specialization of template 'A'}}
+template <typename T> A(T)->B<T>; // expected-error {{deduced type 'B<T>' (aka 'A<T>') of deduction guide is not written as a specialization of template 'A'}}
template<typename T> A(T*) -> const A<T>; // expected-error {{deduced type 'const A<T>' of deduction guide is not a specialization of template 'A'}}
// A deduction-guide shall be declared in the same scope as the corresponding
diff --git a/clang/test/Misc/diag-template-diffing-cxx11.cpp b/clang/test/Misc/diag-template-diffing-cxx11.cpp
index eefeb0b1117c7..c32a3fd56fc1b 100644
--- a/clang/test/Misc/diag-template-diffing-cxx11.cpp
+++ b/clang/test/Misc/diag-template-diffing-cxx11.cpp
@@ -257,25 +257,22 @@ int f9(S9<int, char, U9<const double>>);
int k9 = f9(V9<double>());
// CHECK-ELIDE-NOTREE: no matching function for call to 'f9'
-// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 'S9<[2 * ...], S9<[2 * ...], double>>' to 'S9<[2 * ...], S9<[2 * ...], const double>>' for 1st argument
+// CHECK-ELIDE-NOTREE: candidate function not viable: no known conversion from 'S9<[2 * ...], U9<double>>' to 'S9<[2 * ...], U9<const double>>' for 1st argument
// CHECK-NOELIDE-NOTREE: no matching function for call to 'f9'
-// CHECK-NOELIDE-NOTREE: candidate function not viable: no known conversion from 'S9<int, char, S9<int, char, double>>' to 'S9<int, char, S9<int, char, const double>>' for 1st argument
+// CHECK-NOELIDE-NOTREE: candidate function not viable: no known conversion from 'S9<int, char, U9<double>>' to 'S9<int, char, U9<const double>>' for 1st argument
// CHECK-ELIDE-TREE: no matching function for call to 'f9'
// CHECK-ELIDE-TREE: candidate function not viable: no known conversion from argument type to parameter type for 1st argument
// CHECK-ELIDE-TREE: S9<
-// CHECK-ELIDE-TREE: [2 * ...],
-// CHECK-ELIDE-TREE: S9<
-// CHECK-ELIDE-TREE: [2 * ...],
-// CHECK-ELIDE-TREE: [double != const double]>>
+// CHECK-ELIDE-TREE: [2 * ...],
+// CHECK-ELIDE-TREE: U9<
+// CHECK-ELIDE-TREE: [(no qualifiers) != const] double>>
// CHECK-NOELIDE-TREE: no matching function for call to 'f9'
// CHECK-NOELIDE-TREE: candidate function not viable: no known conversion from argument type to parameter type for 1st argument
// CHECK-NOELIDE-TREE: S9<
-// CHECK-NOELIDE-TREE: int,
-// CHECK-NOELIDE-TREE: char,
-// CHECK-NOELIDE-TREE: S9<
-// CHECK-NOELIDE-TREE: int,
-// CHECK-NOELIDE-TREE: char,
-// CHECK-NOELIDE-TREE: [double != const double]>>
+// CHECK-NOELIDE-TREE: int,
+// CHECK-NOELIDE-TREE: char,
+// CHECK-NOELIDE-TREE: U9<
+// CHECK-NOELIDE-TREE: [(no qualifiers) != const] double>>
template<typename ...A> class class_types {};
void set10(class_types<int, int>) {}
diff --git a/clang/test/SemaCXX/nullability.cpp b/clang/test/SemaCXX/nullability.cpp
index d52ba4efaccdb..ca45a0d26617b 100644
--- a/clang/test/SemaCXX/nullability.cpp
+++ b/clang/test/SemaCXX/nullability.cpp
@@ -98,7 +98,7 @@ void test_accepts_nonnull_null_pointer_literal(X *x) {
accepts_nonnull_6(nullptr); // expected-warning{{null passed to a callee that requires a non-null argument}}
}
-template<void FP(_Nonnull int*)>
+template<void FP(_Nonnull int*)>
void test_accepts_nonnull_null_pointer_literal_template() {
FP(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
}
@@ -197,6 +197,6 @@ void testNullabilityCompletenessWithTemplate() {
namespace GH60344 {
class a;
-template <typename b> using c = b _Nullable; // expected-error {{'_Nullable' cannot be applied to non-pointer type 'GH60344::a'}}
+template <typename b> using c = b _Nullable; // expected-error {{'_Nullable' cannot be applied to non-pointer type 'a'}}
c<a>; // expected-note {{in instantiation of template type alias 'c' requested here}}
}
diff --git a/clang/test/SemaCXX/sizeless-1.cpp b/clang/test/SemaCXX/sizeless-1.cpp
index 368a3eeb6955b..688bbf058e4ca 100644
--- a/clang/test/SemaCXX/sizeless-1.cpp
+++ b/clang/test/SemaCXX/sizeless-1.cpp
@@ -325,7 +325,7 @@ void template_fn_rvalue_ref(T &&) {}
#if __cplusplus >= 201103L
template <typename T>
-using array_alias = T[1]; // expected-error {{array has sizeless element type '__SVInt8_t'}}
+using array_alias = T[1]; // expected-error {{array has sizeless element type 'svint8_t' (aka '__SVInt8_t')}}
extern array_alias<int> *array_alias_int_ptr;
extern array_alias<svint8_t> *array_alias_int8_ptr; // expected-note {{in instantiation of template type alias 'array_alias' requested here}}
#endif
diff --git a/clang/test/SemaHLSL/VectorOverloadResolution.hlsl b/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
index 37d5068c3067c..b320abdd81182 100644
--- a/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
+++ b/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
@@ -22,7 +22,7 @@ void Fn2(int16_t2 S);
// CHECK: CallExpr {{.*}} 'void'
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(int64_t2)' <FunctionToPointerDecay>
// CHECK-NEXT: DeclRefExpr {{.*}} 'void (int64_t2)' lvalue Function {{.*}} 'Fn2' 'void (int64_t2)'
-// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<long, 2>' <IntegralCast>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<int64_t, 2>' <IntegralCast>
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int2':'vector<int, 2>' <LValueToRValue>
// CHECK-NEXT: DeclRefExpr {{.*}} 'int2':'vector<int, 2>' lvalue ParmVar {{.*}} 'I' 'int2':'vector<int, 2>'
@@ -36,7 +36,7 @@ void Fn3( int64_t2 p0);
// CHECK: CallExpr {{.*}} 'void'
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(int64_t2)' <FunctionToPointerDecay>
// CHECK-NEXT: DeclRefExpr {{.*}} 'void (int64_t2)' lvalue Function {{.*}} 'Fn3' 'void (int64_t2)'
-// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<long, 2>' <FloatingToIntegral>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<int64_t, 2>' <FloatingToIntegral>
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'half2':'vector<half, 2>' <LValueToRValue>
// CHECK-NEXT: DeclRefExpr {{.*}} 'half2':'vector<half, 2>' lvalue ParmVar {{.*}} 'p0' 'half2':'vector<half, 2>'
// CHECKIR-LABEL: Call3
@@ -49,7 +49,7 @@ void Call3(half2 p0) {
// CHECK: CallExpr {{.*}} 'void'
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(int64_t2)' <FunctionToPointerDecay>
// CHECK-NEXT: DeclRefExpr {{.*}} 'void (int64_t2)' lvalue Function {{.*}} 'Fn3' 'void (int64_t2)'
-// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<long, 2>' <FloatingToIntegral>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<int64_t, 2>' <FloatingToIntegral>
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float2':'vector<float, 2>' <LValueToRValue>
// CHECK-NEXT: DeclRefExpr {{.*}} 'float2':'vector<float, 2>' lvalue ParmVar {{.*}} 'p0' 'float2':'vector<float, 2>'
// CHECKIR-LABEL: Call4
@@ -65,8 +65,8 @@ void Fn4( float2 p0);
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float2)' <FunctionToPointerDecay>
// CHECK-NEXT: DeclRefExpr {{.*}} 'void (float2)' lvalue Function {{.*}} 'Fn4' 'void (float2)'
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<float, 2>' <IntegralToFloating>
-// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int64_t2':'vector<long, 2>' <LValueToRValue>
-// CHECK-NEXT: DeclRefExpr {{.*}} 'int64_t2':'vector<long, 2>' lvalue ParmVar {{.*}} 'p0' 'int64_t2':'vector<long, 2>'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int64_t2':'vector<int64_t, 2>' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int64_t2':'vector<int64_t, 2>' lvalue ParmVar {{.*}} 'p0' 'int64_t2':'vector<int64_t, 2>'
// CHECKIR-LABEL: Call5
// CHECKIR: {{.*}} = sitofp <2 x i64> {{.*}} to <2 x float>
void Call5(int64_t2 p0) {
diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp
index 043a4ccf56e36..d03c783313dd7 100644
--- a/clang/test/SemaTemplate/deduction-guide.cpp
+++ b/clang/test/SemaTemplate/deduction-guide.cpp
@@ -161,10 +161,10 @@ using DT = D<int, int>;
// CHECK: `-SubstTemplateTypeParmPackType {{.*}} 'U' dependent contains_unexpanded_pack typename depth 1 index 0 ... U
// CHECK: |-TypeAliasTemplate {{.*}} 'B'
// CHECK: `-TemplateArgument pack
-// CHECK: |-TemplateArgument type 'type-parameter-0-1'
+// CHECK: |-TemplateArgument type 'U1':'type-parameter-0-1'
// CHECK-NOT: Subst
// CHECK: | `-TemplateTypeParmType
-// CHECK: `-TemplateArgument type 'type-parameter-0-2'
+// CHECK: `-TemplateArgument type 'U2':'type-parameter-0-2'
// CHECK-NOT: Subst
// CHECK: `-TemplateTypeParmType
@@ -351,9 +351,9 @@ X x = {{1, 2}};
// CHECK-LABEL: Dumping GH64625::<deduction guide for X>:
// CHECK-NEXT: FunctionTemplateDecl {{.+}} <{{.+}}:[[#@LINE - 7]]:1, col:27> col:27 implicit <deduction guide for X>
// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} <col:11, col:17> col:17 referenced class depth 0 index 0 T
-// CHECK: |-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit <deduction guide for X> 'auto (T (&&)[2]) -> X<T>' aggregate
+// CHECK: |-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit <deduction guide for X> 'auto (T (&&)[2]) -> X<T>' aggregate
// CHECK-NEXT: | `-ParmVarDecl {{.+}} <col:27> col:27 'T (&&)[2]'
-// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit used <deduction guide for X> 'auto (int (&&)[2]) -> GH64625::X<int>' implicit_instantiation aggregate
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:27> col:27 implicit used <deduction guide for X> 'auto (int (&&)[2]) -> GH64625::X<int>' implicit_instantiation aggregate
// CHECK-NEXT: |-TemplateArgument type 'int'
// CHECK-NEXT: | `-BuiltinType {{.+}} 'int'
// CHECK-NEXT: `-ParmVarDecl {{.+}} <col:27> col:27 'int (&&)[2]'
@@ -361,7 +361,7 @@ X x = {{1, 2}};
// CHECK-NEXT: |-InjectedClassNameType {{.+}} 'X<T>' dependent
// CHECK-NEXT: | `-CXXRecord {{.+}} 'X'
// CHECK-NEXT: `-RValueReferenceType {{.+}} 'T (&&)[2]' dependent
-// CHECK-NEXT: `-ConstantArrayType {{.+}} 'T[2]' dependent 2
+// CHECK-NEXT: `-ConstantArrayType {{.+}} 'T[2]' dependent 2
// CHECK-NEXT: `-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0
// CHECK-NEXT: `-TemplateTypeParm {{.+}} 'T'
@@ -372,37 +372,37 @@ template <class T, class U> struct TwoArrays {
TwoArrays ta = {{1, 2}, {3, 4, 5}};
// CHECK-LABEL: Dumping GH64625::<deduction guide for TwoArrays>:
-// CHECK-NEXT: FunctionTemplateDecl {{.+}} <{{.+}}:[[#@LINE - 7]]:1, col:36> col:36 implicit <deduction guide for TwoArrays>
-// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} <col:11, col:17> col:17 referenced class depth 0 index 0 T
-// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} <col:20, col:26> col:26 referenced class depth 0 index 1 U
-// CHECK: |-CXXDeductionGuideDecl {{.+}} <col:36> col:36 implicit <deduction guide for TwoArrays> 'auto (T (&&)[2], U (&&)[3]) -> TwoArrays<T, U>' aggregate
-// CHECK-NEXT: | |-ParmVarDecl {{.+}} <col:36> col:36 'T (&&)[2]'
-// CHECK-NEXT: | `-ParmVarDecl {{.+}} <col:36> col:36 'U (&&)[3]'
-// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:36> col:36 implicit used <deduction guide for TwoArrays> 'auto (int (&&)[2], int (&&)[3]) -> GH64625::TwoArrays<int, int>' implicit_instantiation aggregate
-// CHECK-NEXT: |-TemplateArgument type 'int'
-// CHECK-NEXT: | `-BuiltinType {{.+}} 'int'
-// CHECK-NEXT: |-TemplateArgument type 'int'
-// CHECK-NEXT: | `-BuiltinType {{.+}} 'int'
-// CHECK-NEXT: |-ParmVarDecl {{.+}} <col:36> col:36 'int (&&)[2]'
-// CHECK-NEXT: `-ParmVarDecl {{.+}} <col:36> col:36 'int (&&)[3]'
-// CHECK-NEXT: FunctionProtoType {{.+}} 'auto (T (&&)[2], U (&&)[3]) -> TwoArrays<T, U>' dependent trailing_return
-// CHECK-NEXT: |-InjectedClassNameType {{.+}} 'TwoArrays<T, U>' dependent
-// CHECK-NEXT: | `-CXXRecord {{.+}} 'TwoArrays'
-// CHECK-NEXT: |-RValueReferenceType {{.+}} 'T (&&)[2]' dependent
-// CHECK-NEXT: | `-ConstantArrayType {{.+}} 'T[2]' dependent 2
-// CHECK-NEXT: | `-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0
-// CHECK-NEXT: | `-TemplateTypeParm {{.+}} 'T'
-// CHECK-NEXT: `-RValueReferenceType {{.+}} 'U (&&)[3]' dependent
-// CHECK-NEXT: `-ConstantArrayType {{.+}} 'U[3]' dependent 3
-// CHECK-NEXT: `-TemplateTypeParmType {{.+}} 'U' dependent depth 0 index 1
+// CHECK-NEXT: FunctionTemplateDecl {{.+}} <{{.+}}:[[#@LINE - 7]]:1, col:36> col:36 implicit <deduction guide for TwoArrays>
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} <col:11, col:17> col:17 referenced class depth 0 index 0 T
+// CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} <col:20, col:26> col:26 referenced class depth 0 index 1 U
+// CHECK: |-CXXDeductionGuideDecl {{.+}} <col:36> col:36 implicit <deduction guide for TwoArrays> 'auto (T (&&)[2], U (&&)[3]) -> TwoArrays<T, U>' aggregate
+// CHECK-NEXT: | |-ParmVarDecl {{.+}} <col:36> col:36 'T (&&)[2]'
+// CHECK-NEXT: | `-ParmVarDecl {{.+}} <col:36> col:36 'U (&&)[3]'
+// CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} <col:36> col:36 impli...
[truncated]
|
|
@alexfh I can't observe anymore the performance regression you reported on the original phab DR (q2.cc test case). The original reduction doesn't compile anymore starting from clang-18, so I took the same base test and applied it to current libc++, but the memory usage and wall time are pretty close. Let me know if you have an additional test case. |
mizvekov
force-pushed
the
users/mizvekov/clang-sugar-substitute-concepts
branch
from
9919bb4 to
6a172a9
Compare
This makes use of the changes introduced in D134604, in order to instantiate alias templates witn a final sugared substitution. This comes at no additional relevant cost. Since we don't track / unique them in specializations, we wouldn't be able to resugar them later anyway. Differential Revision: https://reviews.llvm.org/D136565
mizvekov
force-pushed
the
users/mizvekov/clang-sugar-substitute-template-alias
branch
from
4615439 to
1c6bfce
Compare
mizvekov
deleted the
users/mizvekov/clang-sugar-substitute-template-alias
branch
|
When instantiating such an alias: template <typename T>
using Identity = T;before the patch: After the patch: @mizvekov, do you have any idea how to get back the lost |
So from my undertstanding, IWYU only needs the SubstTemplateTypeParmType for resugaring purposes, in order to recover the type as written by the user. But with this patch we are doing the substitution already with sugar, so there is no need to resugar, so IWYU shouldn't need the SubstTemplateTypeParmType here anymore. Does that make sense? |
|
No, IWYU has some complex logic to figure out which type components the type alias author intends to provide, and which should be |
|
The basic premise here was implemented back in D134604, and this has been for a few years applied in some cases, like substitution of default arguments. We leave a Subst* node behind with the purpose of somewhere down the line changing it back to what the user wrote. So we don't leave a Subst* node behind when we don't need to resugar, or in the future when we implement clang's own resugarer, we wouldn't leave it behind after resugaring as well. This reduces the cost of this sugared substitution and resugaring, and would help achieving the goal of merging our own resugarer in the future. From past conversations with IWYU maintainer, this was desirable, since IWYU rolls its own resugarer, which is faulty and difficult to maintain. Now this other use case was never put forward before, and it looks like there might be some alternative way of doing this. |
|
One possibility here is that if you have the type, then you will have a template specialization type for the template alias, and that gives you the template arguments used to specialize the alias. Which should answer this need. But of course, if you have a lossy semantic adjustment which makes you lose top level sugar, then you might not have the template specialization type anymore, but this will also apply to Subst* nodes, which can also be lost in these cases. But you might have noticed already that type sugar cannot be reliably depended upon. @zygoloid thoughts? |
This can probably go, thank you! |
I agree that we don't need For example, we (Google) have a tool for inferring and checking nullability contracts (https://github.com/google/crubit/tree/main/nullability), and a tool for lifetime contracts (https://github.com/google/crubit/tree/main/lifetime_annotations). These tool rely on So for all practical purposes, the removal of To be clear: our tool right now is partially broken (when alias templates are involved), and I am not sure how to fix it when |
Can we extend Clang in other ways where this propagation can happen within our transforms?
To be clear, it has been broken as of a couple of years already for some cases, we already perform this 'Final' substitution when instantiating default arguments for templates, for example. This patch extends that to alias templates. As for my first question, can you think of an alternative way we can go about this? What would you need to be upstreamed in order for our own transforms to handle this for you? It sounds like it will be a lot of waste if we have our own resugarer within clang, but then you would be forced to implement your own external resugarer anyway. |
I don't know, but it would be a lot of work, and would likely still mean preserving the same information as Our scope is also larger than what Clang's resugarer aims to do, because for our purposes sometimes there isn't even one correct sugar to be permanently recorded in the AST. For example, we also want to propagate our annotations through instantiated function template bodies so that we can analyze function template instantiations in a context-sensitive way. For example: template<typename Container, typename E>
void Push(Container &xs, E elt) {
xs.push_back(std::move(elt));
}
void Caller(
std::vector<absl::Nullable<int*>> nullable_pointers,
std::vector<absl::Nonnull<int*>> nonnull_pointers
) {
Push(nullable_pointers, nullptr); // ok
Push(nonnull_pointers, nullptr); // warning inside of 'Push' with a template instantiation stack trace
}
Yes, and we consider those cases to be bugs, we just didn't get to them yet...
PTAL at https://github.com/google/crubit/blob/main/nullability/type_nullability.cc, especially the However if upstream Clang provided base classes that only did this, without exposing the underlying data, we would lose out on flexibility. I'm also not sure that the resugaring algorithms are exactly the same between nullability and lifetime analysis. The alternatives I can think of are all much worse than leaving Note that we are hoping to eventually upstream some of our work (see, for example, our RFC thread on lifetimes |
I wouldn't say that's outside the scope of clang's resugarer. I just don't think that can be something we can always enable, Right now the priority is to implement the stuff that won't take a big hit on perf. But that is just priority.
I don't think, from looking at the resugarer implemented in I think the first thing you would find you need is to get those default arguments resugared, as for example both libc++ and libstdc++ vector implementations derive the element type not through the first template argument, but through the allocator instead.
I see the implementation of the resugarer in
Can you explain in more concrete terms how you think the sugared substitution as implemented in this patch creates impediment relative to what you need in your project? As far as I can see, you don't need to keep the Subst* nodes after resugaring if you implement:
So then our transforms should still do the right things you need for them. You would still need to implement the semantic analysis for these annotations, even if externally. But that still should give a lot of flexibility.
I don't think the bar is that high for starting to upstream the work. As long as this is marked as an experimental extension that must be manually enabled, you can still treat this as a Greenfield thing, and do your experiments. I was aware of the lifetime annotations thread, I even commented on it. |
On resugaring bodies of function template instantiations
We keep track of propagated and inferred nullability annotations in a side data structure. Our approach allows us to save on memory by not actually re-instantiating everything with new sugar. But I do have a question - how is the built-in resugarer going to do this without maintaining a side data structure like we do? Clang currently does not have a concept of non-canonical instantiations.
But wouldn't it be better if out-of-tree users like us were not blocked on the priorities of your work? With the On (in)completeness of the out-of-tree resugarer
Actually, resugaring in nullability analysis already works for #include <vector>
#include "absl/base/nullability.h"
void Test() {
int *p = nullptr;
std::vector<absl::Nonnull<int*>> xs;
xs.push_back(p);
}
As far as I see, libc++ does not actually use the allocator to define template <class _Tp, class _Allocator /* = allocator<_Tp> */>
class _LIBCPP_TEMPLATE_VIS vector {
typedef _Tp value_type;
typedef value_type& reference;
typedef const value_type& const_reference;
/* ... */ void push_back(const_reference __x);(see https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector)
I'm not sure what you mean by "get there faster" - our implementation already works, is already deployed internally, and covers a lot of cases that are relevant in practice. We are still tweaking things of course, and we are working on inference tooling (that infers nullability contracts based on the implementation and suggests edits to the source code accordingly). That is, the difficult part and the focus at the moment is not resugaring in the analysis. I do admit that our implementation of resugaring is probably not as comprehensive as the one that you're working on, but it works for the vast majority of real-world code that people write in our monorepo - we tweaked it based on the cases that we actually observed. On upstreaming the work
We are in the process of adding Clang's However, a part of the nullability language extension is the pragma that changes the default meaning of pointers from "unknown" to "nonnull". That is, we interpret the following two files in an identical way: #include "absl/base/nullability.h"
#pragma nullability file_default nonnull
void TakePointer(int *p);#include "absl/base/nullability.h"
void TakePointer(absl::Nonnull<int *> p);Unfortunately we expect this pragma to be a controversial upstream because Clang already has a pragma for this exact purpose ( In the end, we need a separate pragma with slightly different semantics because Clang's pragma was not suitable for C++. Getting these differences reconciled can take months in discussions alone, and we don't want to spend time on this when we are still not 100% sure that the model is going to work for us in the long run. Fundamentally, your proposal to reuse the built-in resugarer depends on upstreaming an incomplete language extension, which cuts against Clang's extension policy, https://clang.llvm.org/get_involved.html:
I don't know how to demonstrate a significant user community if the extension must live in the Clang core. Of course we could fork Clang, but we would need to maintain this fork for years. We have been working on nullability since April 2020 - that's 4+ years already. The cost involved in maintaining a Clang fork for 4+ years, compared to an out-of-tree ClangTidy check, is just unpalatable. We're lucky that Clang upstream already has nullability annotations that happen to be a mostly good match (modulo the pragma and some other issues). But what about our work on lifetimes? On separating resugaring improvements and
|
I think there might have been a slight miscommunication here, my fault. The idea being considered is to just instantiate the template using the first sugaring found, using a sugared non-final substitution. That way, any semantic analysis during template instantiation would be able to produce diagnostics relative to that sugaring. But that would depend on the resugarer being fully implemented, so that any uses of that template specialization would be resugared to the correct sugar relative to the point of use. We would also need to enforce that As currently things stand, we would have no reason to produce a second instantiation for the same type for a template which already succeeded in instantiation, and which we will resugar later anyway, because that should not produce a different outcome. But external analysis should be free to pursue that if needed.
You are free to propose any AST extension that would help out-of-tree users. That proposal has to have a reasonable justification. But one thing I haven't mentioned before is the tradeoffs and reasoning behind the current approach of instantiating alias templates fully sugared, instead of resugaring them later. The main reason is that as things stood before the current patch, alias templates couldn't be reliably resugared. This is a problem because you can run into a type which has Subst nodes from different alias template specializations, and as you can't tell them apart, you could end up producing incorrect resugaring.
You are right, I misremembered, that was true only for libstdc++.
I see. We could support external resugarers too, without considering costs, that is not unreasonable. I think this patch is the wrong tree to bark on though. Tracking alias template specializations would be a lot more complex and likely higher performance impact.
If you are using this internally at google, that seems like evidence that there is an existing user community.
In the course of implementing resugaring, there were several deficiencies in upstream clang which had to be corrected. I think if I had the idea that I could implement the resugaring completely out-of-tree, these deficiencies would have remained, and consequently work arounds and incomplete resugaring would have to be accepted. Since you are also using the AssociatedDecl in the type_nnullability analysis, perhaps you have noticed we couldn't The type alias is in the same situation here. It's AssociatedDecl is not very useful if it only points to the template pattern. So I took the course of fixing it in upstream clang, with the aim of having the best outcome for an in-tree resugarer, which is what makes the most sense.
We would still need a distinction between Subst nodes that need to be resugared and those that don't. If we want to support out-of-tree resugarers, I think the least bad option here would be perhaps a flag to disable in-tree resugaring. But that doesn't solve the problem that template type aliases don't have all the necessary design bits for resugaring.
That's mostly explained above. Since Subst nodes are currently heavy on sugar, and they can appear deeply nested inside the type, they can have a large impact on uniquing these types. Since we only need to resugar one Subst node once, we need the distinction between having already handled that type, or not. For a type alias template, there is currently no declaration that can represent its specialization.
As much as possible, within reason, considering performance impact. There are some things left out because of performance concerns, like:
|
|
@mizvekov I will provide a more detailed response to the points you made in your last message separately, but for now I would like to ask you to revert the commit to unbreak us. We looked into the problem internally and it does not look we have a simple hack we can apply to recover this information. The solutions you offered so far to unblock us are going to take years to implement (like waiting for you to complete work that you consider low priority, or upstreaming our code - which means upstreaming 20 KLoC of nullability verification code which involves resolving differences around pragma semantics etc.) |
llvm/llvm-project@7f78f99 removed SubstTemplateTypeParmType nodes from an instantiated alias template type. GetProvidedTypes function relied on them to filter out substituted argument types. Now, it is done "manually" inside the added GetAliasTemplateProvidedTypes function, as discussed in llvm/llvm-project#101858. When traversing instantiated type alias internals, bare RecordType nodes appear now instead of ones wrapped in SubstTemplateTypeParmType, hence the new visiting method added into InstantiatedTemplateVisitor. (Enum types are not expected to need handling there, hence VisitRecordType instead of VisitTagType.) A test case added to cover the change inside GetProvidedTypes function (i.e. correct handling of intermediate alias templates in a chain).
llvm/llvm-project@7f78f99 removed SubstTemplateTypeParmType nodes from an instantiated alias template type. GetProvidedTypes function relied on them to filter out substituted argument types. Now, it is done "manually" inside the added GetAliasTemplateProvidedTypes function, as discussed in llvm/llvm-project#101858. When traversing instantiated type alias internals, bare RecordType nodes appear now instead of ones wrapped in SubstTemplateTypeParmType, hence the new visiting method added into InstantiatedTemplateVisitor. (Enum types are not expected to need handling there, hence VisitRecordType instead of VisitTagType.) A test case added to cover the change inside GetProvidedTypes function (i.e. correct handling of intermediate alias templates in a chain).
@gribozavr see #102510 for alternative.
That's not really true. You only need to upstream the AST affecting parts. The attributes itself and AST node which represent it. |
Thank you! We are looking at this now.
The attributes are already upstream (we are reusing nullability attributes added by Apple), as I explained above. The C++-compatible pragma is missing - and I expect it to be a lot of work to reconcile. And even then, we still need to propagate the annotations - which currently happens within the ClangTidy check. For example, we need to understand that the template parameter 0 of std::vector<absl::Nullable<int*>> MakeVectorOfNullable();
void Test() {
std::vector<int*> xs = MakeVectorOfNullable();
*xs[0]; // warning
}This propagation could happen within core Clang so that it can be integrated into the resugarer that constructs the AST, but doing so (injecting nullable annotations here) would have its own cost for the AST, and would require rearchitecting the check, to essentially run part of the logic during AST construction instead of inside of ClangTidy. |
I just want to mention that, actually, IWYU doesn't probably need any resugarer at all. A type template argument as-written is needed just to decide whether its canonical type should be reported for OTOH it is desirable to distinguish between the first and the second argument usages in cases like |
It is not only valuable to external resugarers. Another point that warrants an introduction of it is for unevaluated lambdas. These lambdas e.g. appearing as part of the |
llvm/llvm-project@7f78f99 removed SubstTemplateTypeParmType nodes from an instantiated alias template type. GetProvidedTypes function relied on them to filter out substituted argument types. Now, it is done "manually" inside the added GetAliasTemplateProvidedTypes function, as discussed in llvm/llvm-project#101858. When traversing instantiated type alias internals, bare RecordType nodes appear now instead of ones wrapped in SubstTemplateTypeParmType, hence the new visiting method added into InstantiatedTemplateVisitor. (Enum types are not expected to need handling there, hence VisitRecordType instead of VisitTagType.) A test case added to cover the change inside GetProvidedTypes function (i.e. correct handling of intermediate alias templates in a chain).
llvm/llvm-project@7f78f99 removed SubstTemplateTypeParmType nodes from an instantiated alias template type. GetProvidedTypes function relied on them to filter out substituted argument types. Now, it is done "manually" inside the added GetAliasTemplateProvidedTypes function, as discussed in llvm/llvm-project#101858. When traversing instantiated type alias internals, bare RecordType nodes appear now instead of ones wrapped in SubstTemplateTypeParmType, hence the new visiting method added into InstantiatedTemplateVisitor. (Enum types are not expected to need handling there, hence VisitRecordType instead of VisitTagType.) A test case added to cover the change inside GetProvidedTypes function (i.e. correct handling of intermediate alias templates in a chain).
So in clang we have taken a different approach, in order to not sacrifice template type alias performance, we instead put a The idea is a tradeoff where template type alias, which are very light weight, don't get the big performance hit they would otherwise if they had a per-specialization declaration. See for example CXXRecordDecl's Unfortunately there is an issue where we currently only store this lambda context when we need it for mangling reasons, but there are other reasons to depend on it as well, and code in clang currently makes the incorrect assumption that it will always be available when needed. I am working on a patch to fix that. |
Yeah, I remember we use that to store an ImplicitConceptSpecializationDecl as well, not only for the purpose of mangling but also for lambdas appearing inside a concept. We probably could do the same for the using aliases, though that still needs some tweaking to store template arguments. |
llvm/llvm-project@7f78f99 removed SubstTemplateTypeParmType nodes from an instantiated alias template type. GetProvidedTypes function relied on them to filter out substituted argument types. Now, it is done "manually" inside the added GetAliasTemplateProvidedTypes function, as discussed in llvm/llvm-project#101858. When traversing instantiated type alias internals, bare RecordType nodes appear now instead of ones wrapped in SubstTemplateTypeParmType, hence the new visiting method added into InstantiatedTemplateVisitor. (Enum types are not expected to need handling there, hence VisitRecordType instead of VisitTagType.) A test case added to cover the change inside GetProvidedTypes function (i.e. correct handling of intermediate alias templates in a chain).
This makes use of the changes introduced in D134604, in order to instantiate alias templates witn a final sugared substitution.
This comes at no additional relevant cost.
Since we don't track / unique them in specializations, we wouldn't be able to resugar them later anyway.
Differential Revision: https://reviews.llvm.org/D136565