lichray added inline comments.
================
Comment at: clang/lib/AST/APValue.cpp:637-639
+ // Nothing we can do about a sequence that is not null-terminated
+ if (!Data[--Size].getInt().isZero())
+ return false;
----------------
rsmith wrote:
> lichray wrote:
> > rsmith wrote:
> > > We should drop all trailing zeroes here, because array initialization
> > > from a string literal will reconstruct them.
> > Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different types.
> That's a separate bug. `APValue`'s printing is not intended to identify the
> type of the result, only to identify the value in the case where we already
> know the type. Eg, we don't print a type suffix on a pretty-printed integer
> value. For the specific case of template parameters, it's the responsibility
> of the template argument printing code to uniquely identify the template
> argument.
>
> When a template parameter has a deduced class type, we should include that
> type in the printed output in order to uniquely identify the specialization,
> but we don't, because that's not been implemented yet. When that's fixed, we
> should print those cases as `MyType<Str<char, 1>{""}>` and `MyType<Str<char,
> 4>{""}>`. This is necessary anyway, because we can't in general assume that
> the resulting value is enough to indicate what type CTAD will have selected.
>
> We already handle this properly in some cases, but see this FIXMEs:
> https://github.com/llvm/llvm-project/blob/6c2be3015e07f25912b8cd5b75214c532f568638/clang/lib/AST/TemplateBase.cpp#L433
>
> See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that the
> non-struct enum case is handled properly but the enum-in-struct case is not).
> That's a separate bug. [...]
>
> When a template parameter has a deduced class type, we should include that
> type in the printed output in order to uniquely identify the specialization,
> but we don't, because that's not been implemented yet. When that's fixed, we
> should print those cases as `MyType<Str<char, 1>{""}>` and `MyType<Str<char,
> 4>{""}>`. This is necessary anyway, because we can't in general assume that
> the resulting value is enough to indicate what type CTAD will have selected.
>
But it looks like a feature rather than a bug? String literals provided both
type and value to emphasis a structural type's value for equivalence
comparison. The reason of why `MyType<{""}>` and `MyType<"\0\0\0">` are
different looks more obvious to me.
> See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that the
> non-struct enum case is handled properly but the enum-in-struct case is not).
I'm fine if a specific NTTP, `A`, is treated differently from `auto`. But the
last case is similar to this https://godbolt.org/z/YcscTrchM Which I thought
about printing something like `Y<{(E[]){97, 98}}>` :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115031/new/
https://reviews.llvm.org/D115031
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
