balazske added a comment.
If the test is changed to print AST:
TEST_P(ASTImporterOptionSpecificTestBase, ImportDeductionGuide) {
TranslationUnitDecl *FromTU = getTuDecl(
R"(
template<class> class A { };
template<class T> class B {
template<class T1, typename = A<T>> B(T1);
};
template<class T>
B(T, T) -> B<int>;
)",
Lang_CXX17);
// Get the implicit deduction guide for (non-default) constructor of 'B'.
auto *FromDG1 = FirstDeclMatcher<FunctionTemplateDecl>().match(
FromTU, functionTemplateDecl(templateParameterCountIs(3)));
FromTU->dumpColor();
llvm::errs()<<cast<Decl>(FromDG1->getTemplateParameters()->getParam(0)->getDeclContext())<<",";
llvm::errs()<<cast<Decl>(FromDG1->getTemplateParameters()->getParam(1)->getDeclContext())<<",";
llvm::errs()<<cast<Decl>(FromDG1->getTemplateParameters()->getParam(2)->getDeclContext())<<"\n";
}
The output is:
...
|-ClassTemplateDecl 0x17931e0 <input.cc:2:7, col:33> col:29 A
| |-TemplateTypeParmDecl 0x1793090 <col:16> col:21 class depth 0 index 0
| `-CXXRecordDecl 0x1793150 <col:23, col:33> col:29 class A definition
| |-DefinitionData empty aggregate standard_layout trivially_copyable pod
trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| | |-DefaultConstructor exists trivial constexpr needs_implicit
defaulted_is_constexpr
| | |-CopyConstructor simple trivial has_const_param needs_implicit
implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment simple trivial has_const_param needs_implicit
implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| `-CXXRecordDecl 0x1793420 <col:23, col:29> col:29 implicit class A
|-ClassTemplateDecl 0x17935f0 <line:3:7, line:5:7> line:3:31 B
| |-TemplateTypeParmDecl 0x17934c8 <col:16, col:22> col:22 referenced class
depth 0 index 0 T
| |-CXXRecordDecl 0x1793560 <col:25, line:5:7> line:3:31 class B definition
| | |-DefinitionData empty standard_layout trivially_copyable
has_user_declared_ctor can_const_default_init
| | | |-DefaultConstructor defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit
implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment simple trivial has_const_param needs_implicit
implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | |-CXXRecordDecl 0x1793830 <col:25, col:31> col:31 implicit referenced
class B
| | `-FunctionTemplateDecl 0x1793cb0 <line:4:11, col:51> col:47 B<T>
| | |-TemplateTypeParmDecl 0x17938c0 <col:20, col:26> col:26 referenced
class depth 1 index 0 T1
| | |-TemplateTypeParmDecl 0x1793a00 <col:30, <scratch space>:2:1>
input.cc:4:39 typename depth 1 index 1
| | | `-TemplateArgument type 'A<T>'
| | | `-TemplateSpecializationType 0x1793980 'A<T>' dependent A
| | | `-TemplateArgument type 'T'
| | | `-TemplateTypeParmType 0x1793520 'T' dependent depth 0 index 0
| | | `-TemplateTypeParm 0x17934c8 'T'
| | `-CXXConstructorDecl 0x1793c08 <col:47, col:51> col:47 B<T> 'void (T1)'
| | `-ParmVarDecl 0x1793ad8 <col:49> col:51 'T1'
| `-ClassTemplateSpecializationDecl 0x16faa28 <line:3:7, line:5:7> line:3:31
class B
| `-TemplateArgument type 'int'
| `-BuiltinType 0x1728a70 'int'
|-FunctionTemplateDecl 0x16fb050 <line:4:11, col:51> col:47 implicit
<deduction guide for B>
| |-TemplateTypeParmDecl 0x17934c8 <line:3:16, col:22> col:22 referenced
class depth 0 index 0 T
| |-TemplateTypeParmDecl 0x16fac88 <line:4:20, col:26> col:26 class depth 0
index 1 T1
| |-TemplateTypeParmDecl 0x16fad38 <col:30, <scratch space>:2:1>
input.cc:4:39 typename depth 0 index 2
| | `-TemplateArgument type 'A<T>'
| | `-TemplateSpecializationType 0x16fae00 'A<T>' dependent A
| | `-TemplateArgument type 'T'
| | `-TemplateTypeParmType 0x1793520 'T' dependent depth 0 index 0
| | `-TemplateTypeParm 0x17934c8 'T'
| `-CXXDeductionGuideDecl 0x16faf98 <col:47, col:51> col:47 implicit
<deduction guide for B> 'auto (type-parameter-0-1) -> B<T>'
| `-ParmVarDecl 0x16faea8 <col:49> col:51 'type-parameter-0-1'
|-FunctionTemplateDecl 0x16fb268 <line:3:7, col:31> col:31 implicit
<deduction guide for B>
| |-TemplateTypeParmDecl 0x17934c8 <col:16, col:22> col:22 referenced class
depth 0 index 0 T
| `-CXXDeductionGuideDecl 0x16fb1b0 <col:31> col:31 implicit <deduction guide
for B> 'auto (B<T>) -> B<T>'
| `-ParmVarDecl 0x16fb148 <col:31> col:31 'B<T>'
`-FunctionTemplateDecl 0x16fb380 <line:6:7, line:7:23> col:7 <deduction guide
for B>
|-TemplateTypeParmDecl 0x16fa860 <line:6:16, col:22> col:22 referenced
class depth 0 index 0 T
`-CXXDeductionGuideDecl 0x16fb2d0 <line:7:7, col:23> col:7 <deduction guide
for B> 'auto (T, T) -> B<int>'
|-ParmVarDecl 0x16fa930 <col:9> col:10 'T'
`-ParmVarDecl 0x16fa9a8 <col:12> col:13 'T'
0x16fb1b0,0x16faf98,0x16faf98
In file included from output.cc:1:
input.cc:4:11: warning: template parameter lists have a different number of
parameters (1 vs 3) [-Wodr]
template<class T1, typename = A<T>> B(T1);
^
input.cc:6:7: note: template parameter list also declared here
template<class T>
The interesting is the printed DeclContext: The first value is the deduction
guide 'auto (B<T>) -> B<T>'. Not the class declaration and not the "correct"
deduction guide. I think that when the deduction guide 'auto (B<T>) -> B<T>' is
created the parameters are "adopted", and after `B<T>` the DeclContext of `T`
is changed too. This is the same object that was originally the template
parameter of the CXXRecordDecl for class `A`. Probably this is a bug in the AST
creation but currently it works this way.
Previously I did not see this ODR warning. It needs more investigation to check
why it appears.
================
Comment at: clang/lib/AST/ASTImporter.cpp:6081
+ // FunctionTemplateDecl objects are created, but in different order. In this
+ // way DeclContext of these template parameters may change relative to the
+ // "from" context. Because these DeclContext values look already not stable
----------------
martong wrote:
> ?
Probably better:
In this way the DeclContext of these template parameters is not necessary the
same as in the "from" context.
================
Comment at: clang/lib/AST/ASTImporter.cpp:6082-6085
+ // "from" context. Because these DeclContext values look already not stable
+ // and unimportant this change looks acceptable.
+ // For these reasons the old DeclContext must be saved to change the lookup
+ // table later.
----------------
martong wrote:
> I think this sentence does not provide any meaningful information and does
> not increase the understand-ability. Plus the word `change` is overloaded,
> first I though you meant the patch itself...
It is still good to have an explanation of why the DeclContext is not exactly
preserved at import. And the DeclContext is really not "stable", not easily
predictable from the source code.
================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:7336
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportDeductionGuide) {
+ TranslationUnitDecl *FromTU = getTuDecl(
----------------
martong wrote:
> Does this test provide an assertion failure in the base?
We get the "trying to remove not contained Decl" in ASTImporterLookupTable.
================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:7348-7353
+ // Get the implicit deduction guide for (non-default) constructor of 'B'.
+ auto *FromDG1 = FirstDeclMatcher<FunctionTemplateDecl>().match(
+ FromTU, functionTemplateDecl(templateParameterCountIs(3)));
+ // User defined deduction guide.
+ auto *FromDG2 = FirstDeclMatcher<CXXDeductionGuideDecl>().match(
+ FromTU, cxxDeductionGuideDecl(unless(isImplicit())));
----------------
martong wrote:
> Could you please formulate expectations (assertions) on the DeclContext's of
> the two template parameters? I'd expect them to be different.
I left this out because the "instability" mentioned above. It is possible to
make the assertions for this exact code. We can better say that it comes from a
set of possible values, these are the current `DeductionGuideDecl`, or another
one for the same class, or the class itself (but I am not sure in this any
more). It is possible to get different value for different template parameters
of the same template.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114418/new/
https://reviews.llvm.org/D114418
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
