Possible bug: DeclRefExpr wrong NonTypeTemplateParm decl

[deadlocklogic]

Consider:

namespace ns1 {
	template <typename TA1, bool TA2>
	struct Template1 {};
	template <typename TB1, bool TB2>
	struct Template2 {};
	template <typename U1, bool U2, bool U3>
	struct Template2<Template1<U1, U2>, U3> {};
}
namespace ns2 {
	template <typename TA1, bool TA2>
	struct Template1 {};
	template <typename TB1, bool TB2>
	struct Template2 {};
	template <typename U1, bool U3, bool U2>
	struct Template2<Template1<U1, U2>, U3> {};
}

The difference between the class templates in the 2 namespaces is: typename U1, bool U3, bool U2 instead of typename U1, bool U2, bool U3 in the class template partial specialization.
A repro dump is provided.
While the expected difference is just the template parameter order, this is not the case.
The AST difference is:

|   |-TemplateArgument type 'Template1<type-parameter-0-0, TA2>'
|   | `-TemplateSpecializationType 'Template1<type-parameter-0-0, TA2>' dependent Template1
|   |   |-TemplateArgument type 'type-parameter-0-0'
|   |   | `-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0
|   |   `-TemplateArgument expr
|   |     `-DeclRefExpr <line:2:31> 'bool' NonTypeTemplateParm 0xcf3a360 'TA2' 'bool'
|   |-TemplateArgument expr
|   | `-DeclRefExpr <line:11:38> 'bool' NonTypeTemplateParm 0xcf3ada8 'U3' 'bool'

|   |-TemplateArgument type 'Template1<type-parameter-0-0, U2>'
|   | `-TemplateSpecializationType 'Template1<type-parameter-0-0, U2>' dependent Template1
|   |   |-TemplateArgument type 'type-parameter-0-0'
|   |   | `-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0
|   |   `-TemplateArgument expr
|   |     `-DeclRefExpr <col:33> 'bool' NonTypeTemplateParm 0xcf5a4f8 'U2' 'bool'
|   |-TemplateArgument expr
|   | `-DeclRefExpr <col:38> 'bool' NonTypeTemplateParm 0xcf5a490 'U3' 'bool'

Why the clang::NonTypeTemplateParm TA2 is present? Even though it is not in the current semantic context.
I really think this is a bug, and the second result should be the correct one.
Thanks.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (deadlocklogic)

Consider: ```C++ namespace ns1 { template <typename TA1, bool TA2> struct Template1 {}; template <typename TB1, bool TB2> struct Template2 {}; template <typename U1, bool U2, bool U3> struct Template2<Template1<U1, U2>, U3> {}; } namespace ns2 { template <typename TA1, bool TA2> struct Template1 {}; template <typename TB1, bool TB2> struct Template2 {}; template <typename U1, bool U3, bool U2> struct Template2<Template1<U1, U2>, U3> {}; } ``` The difference between the class templates in the 2 namespaces is: `typename U1, bool U3, bool U2` instead of `typename U1, bool U2, bool U3` in the class template partial specialization. A repro [dump](https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QAJgAMfAQQCqAZ0wAFAB6cpvAFYgupWkwahkAUgkAhcxdLL6yAngGVG6AMKpaAVxYNJpVwAZPAZMADkfACNMYhAAZn0AB1RFQicGD29ff2TUxwFg0IiWaNiE20x7fIYhAiZiAkyfPwkKqvTa%2BoJC8KiY%2BP1FOoam7Nahrp7i0oGASltUL2Jkdg5mNkVEphWAagZFLh3TAHYrKQBBUykATgJMFkTDO6O4twIAT0TGVkwdgBVzvodpFUJ5/ucJKY4tgrpcbkNiF4HP97o8mHcuLCrtcTmc4TjjgARKF47F3B5PX5Qt6fb5sf4WIEgsF/CyQ6FY%2BEERHIv6oymQi7Y3GcgnEuKkm7ktHPakfL7rX7yJmg2g7eStYGq9VxKEwoVcnkEFEU9GYdluPmmjHU5WkdXs7D2%2BS6jkGgmSsUkrFE2GKzbbX77CRHU6i6WUl40hU/cEqlkQvWihFI41WmWYTHukXZ30S8P8s1R%2BV036s%2BNq1mO5Pc1MmjOC/E5pt5z0Roty2mK9UVnX25lqjVJ90p3mFu4W9OUzGvO0OvXO136luek7i0m%2Bg3nELGlhMEIQWahjeEjjzWicACsvD83F4qE4bms1h2ikWyypEgSvAImjP8wAaxAS8ABYADopBAgA2a5rmOOIAA5rikOI4muS9LwMTgQN4Fg9CkGRby0UgHw4XhFBAGRfw4LR5jgWAYEQFBUAeOgYnISg0FY%2BhYmQQxjC4BCuBkGhaDuYgKIgSI/1ISIQnqd5OB4WT5OId4AHlIm0TAHCU3guLYQR1IYWhFJo3gsEiLxgDcMRaAou9SCwPdjHEcynLwYgdMcAA3TAHOIzA1B0rw7hkndKj0gw8EiYgFI8LAou5PA8McvziBBZRCXuIxgFoEJQHM%2BYqEMYBFAANTwTAAHd1IVKL%2BEEEQxHYLgQNkQQlFUDR3N0fR%2BJMZ9LGiyIKMgeZUESaoHIAWiGdAoUJMxLGsaQdhmgB1BhUBmrwGAA7bqoYGb0WSqy7kUe90uIPAsDGw82m89IXAYdxPGaPQAleqY%2BliJIUjSARRj8f68nSH6Sn6QZKiegROhGd7smh9o4eGboQl6SG/tsNHgc%2BiYGghmYuHmN8lhWPRzyvG8ZNInY1AQqCZqgkCdj43KdkEsCuAgnYIFwQgSCOL8SZ/P9ZkA%2BI4jA1DZbl%2BWsI4HDSDw1CZfljXUKg0giPvThyMo0hqNo0h6KYxYCESUKOIgTB8CIG70E%2BxrhFEcQ2o6hRlHUGS%2BtIaq4sSPSqY4a8ddpzh1NCq3jVQKh6cZ5nWfZ4xOYQ7neYgDxuJiYWElmMWivmBBMCYLBYgei8ldwvRL25qDjlAhDLwIhChIQuJWl1kj9dsQ3jbPU3GIgJAmCGG2uMSNjiDCH5OAZpmWbZga04zwL7ZIW7nbkZr3fal2up93r8Zhhxntt168eOBCvvQIn%2BhAmRckBjJEb8a/SGf6p79iR/HrP1GXQr6YTsLDGoaMf4gD/gTRob8QCNxxpMDG0wH5SHmNyTAmAt5oMVmHbupFzhDB2JVGqucF5J2XhzLmPMpB8wFg7POQJs5Tx4owguRtxYh2VnhS8PMQKwS4IJLgUFLzXxAh3cO7lSIGyopw0gQEQJ13QnEAiEgoLCWuFBKCUhtZVziDTKRvcB4S0VhIGuwlCIRzIhwoupB0qpGcCBIAA) is provided. While the expected difference is just the template parameter order, this is not the case. The AST difference is: ``` | |-TemplateArgument type 'Template1<type-parameter-0-0, TA2>' | | `-TemplateSpecializationType 'Template1<type-parameter-0-0, TA2>' dependent Template1 | | |-TemplateArgument type 'type-parameter-0-0' | | | `-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0 | | `-TemplateArgument expr | | `-DeclRefExpr <line:2:31> 'bool' NonTypeTemplateParm 0xcf3a360 'TA2' 'bool' | |-TemplateArgument expr | | `-DeclRefExpr <line:11:38> 'bool' NonTypeTemplateParm 0xcf3ada8 'U3' 'bool' ``` ``` | |-TemplateArgument type 'Template1<type-parameter-0-0, U2>' | | `-TemplateSpecializationType 'Template1<type-parameter-0-0, U2>' dependent Template1 | | |-TemplateArgument type 'type-parameter-0-0' | | | `-TemplateTypeParmType 'type-parameter-0-0' dependent depth 0 index 0 | | `-TemplateArgument expr | | `-DeclRefExpr <col:33> 'bool' NonTypeTemplateParm 0xcf5a4f8 'U2' 'bool' | |-TemplateArgument expr | | `-DeclRefExpr <col:38> 'bool' NonTypeTemplateParm 0xcf5a490 'U3' 'bool' ``` Why the `clang::NonTypeTemplateParm` `TA2` is present? Even though it is not in the current semantic context. I really think this is a bug, and the second result should be the correct one. Thanks.

[shafik]

CC @erichkeane

[deadlocklogic]

@shafik @erichkeane Any prognosis at least? Is this a bug or not?
Thanks.

[cor3ntin]

@mizvekov

[mizvekov]

This is a known limitation of how we deal with canonicalization of template arguments of expression kind.

Expressions in clang are not uniqued and we don't build a canonical form for them
Also, uniquing them in their current form would yield little benefit, since they carry their source locations.

So when we need to build a canonical TemplateSpecializationType which has a template argument of expression kind, we choose the first expression encountered as a placeholder, and the TemplateSpecializationType is uniqued based on a canonical profiling of that expression.

If you later try to build another canonical TemplateSpecializationType which has a different expression, but which is canonically the same as the previous expression, then we won't build a new type and just return an existing one.

I suspect what is occurring in your example is that the canonical TST from the specialization argument of the partial specialization of ns1::Template2 is reusing the same TST that was built earlier for the injected specialization type of ns1::Template1. They have the same template arguments in the same order.

The issue does not occur in ns2 because there they don't have the same order.

Ie:
Injected TST of ns1::Template1: ns1::Template1<type-parameter-0-0, expression-parameter-0-1>.
TST from partial specialization arguments of ns1::Template2: ns1::Template1<type-parameter-0-0, expression-parameter-0-1>. The same as above.

Injected TST of ns2::Template1: ns2::Template1<type-parameter-0-0, expression-parameter-0-1>.
TST from partial specialization arguments of ns2::Template2: ns2::Template1<type-parameter-0-0, expression-parameter-0-2>. Not the same, notice the difference in the last argument.

[zyn0217]

we choose the first expression encountered as a placeholder, and the TemplateSpecializationType is uniqued based on a canonical profiling of that expression.
If you later try to build another canonical TemplateSpecializationType which has a different expression, but which is canonically the same as the previous expression, then we won't build a new type and just return an existing one.

Yep, this mechanism also causes #84052.

[mizvekov]

I think one solution for the specific problem of AST dumps is to build a canonical expression printer, and use that for printing expressions off of any AST node which is uniqued.

So this ast-dump would have printed instead:
TemplateSpecializationType 'Template1<type-parameter-0-0, expr-parameter-0-1>' dependent Template1,
which is less confusing.

Ie this printer would dig down an expression and ignore any non-structural parts.

[deadlocklogic]

In my case: the main usage of this is with printing, so I must resolve every AST declaration to its fully qualified name.
I wasn't satisfied with the default DeclPrinter because it had difficulties with template arguments (type-parameter-*-*), so I wrote my own.

The controversial case of TemplateTypeParmDecl

Now consider the same example with replacing the NonTypeTemplateParmDecl with TemplateTypeParmDecl, the AST behaves correctly and as expected without any other dependencies on the outer context.

[mizvekov]

The default DeclPrinter is not at fault here. You are asking it to print a canonical type, and that's the information there.

If you are dumping the whole AST of a program, then you are going to find canonical types and references to canonical template parameters. There is no helping that.

If you are talking about seeing these 'type-parameters--' in the as-written portion of compiler issued diagnostics, then that's a bug, and we have to fix these one by one.

[deadlocklogic]

If you are talking about seeing these 'type-parameters--' in the as-written portion of compiler issued diagnostics, then that's a bug, and we have to fix these one by one.

Yes indeed, these are pretty annoying while printing a type but wouldn't solve the problem, as ns::Template2 in:

namespace ns1 {
	template <typename TA1, bool TA2>
	struct Template1 {};
	template <typename TB1, bool TB2>
	struct Template2 {};
	template <typename U1, bool U2, bool U3>
	struct Template2<Template1<U1, U2>, U3> {};
}

is printed as ns1::Template2<ns1::Template1<type-parameter-0-0, TA2>, U3> instead of ns1::Template2<ns1::Template1<type-parameter-0-0, U2>, U3> (independently on how expressions are represented in the AST, and disregarding the fact of unresolved arguments 'type-parameters--').

Ie this printer would dig down an expression and ignore any non-structural parts.

So the current DeclPrinter behavior is kind of broken: when I need to print types I am not really concerned on how expressions are expressed in the AST, rather than getting a valid string (which hopefully can be processed/parsed/used in code generation etc).

Expressions in clang are not uniqued and we don't build a canonical form for them
Also, uniquing them in their current form would yield little benefit, since they carry their source locations.

So when we need to build a canonical TemplateSpecializationType which has a template argument of expression kind, we choose the first expression encountered as a placeholder, and the TemplateSpecializationType is uniqued based on a canonical profiling of that expression.

So currently, what is the best way to obtain a valid fully qualified notation of a decl (considering template arguments in case this is a template)?

Actually I am working on a tool and which needs to construct parallel raw templates but these synthetized templates may appear in a different namespace/context.
So using a modified default type/decl printer, I can obtain at best:

namespace custom_ns {
	template <typename TA1, bool TA2>
	struct Template1 {};
	template <typename TB1, bool TB2>
	struct Template2 {};
	template <typename U1, bool U2, bool U3>
	struct Template2<::ns::Template1<U1, TA2 /*This is so unwanted/ breaks the compilation*/>, U3> {};
}

Anything to do here, or I have to wait for a better printer, or maybe fixing the mechanism as mentioned in tagged issue?

[mizvekov]

Anything to do here, or I have to wait for a better printer, or maybe fixing the mechanism as mentioned in tagged issue?

I believe this old patch of mine gets you almost there: mizvekov@252f292

It's on my TODO list to rebase and create a PR for this soon.

Except that, in that patch, I chose not to preserve the as-written expression, as that would mean we would basically never be able to unique those CanonicalTemplateSpecialziationTypes.

But that patch would have those TSTs carry a converted argument list which is fully sugared.

For example, that list would have a converted argument list, in which this argument would be represented by a Declaration kind of TemplateArgument pointing to the NTTP decl as written.

Though again in that patch, the converted argument list is not wired to the AST dumper.