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;
----------------
lichray wrote:
> 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}}>` :)
That discussion looks OT... For now, not shrinking `"\0\0\0"` down to `""`
follows the existing logic, i.e., we are printing `{0, 0, 0}` even though it's
an array.
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
