[PATCH] D146358: [clang][AST] Print name instead of type when diagnosing uninitialized subobject in constexpr variables

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D146358#4357106 <https://reviews.llvm.org/D146358#4357106>, @hazohelet wrote:

> In D146358#4352792 <https://reviews.llvm.org/D146358#4352792>, @erichkeane wrote:
>
>> In D146358#4351787 <https://reviews.llvm.org/D146358#4351787>, @hazohelet wrote:
>>
>>> In D146358#4350810 <https://reviews.llvm.org/D146358#4350810>, @erichkeane wrote:
>>>
>>>> Our downstream discovered that this causes a regression when compiling `utility` with `ast-dump`.  I've put up a minimal reproducer: https://godbolt.org/z/oaezas7Ws
>>>>
>>>> @hazohelet : Will you be able to look into this soon?
>>>
>>> Thanks for the report and the reproducer.
>>> I cannot take sufficient time for about 36 hours, but I'll be able to investigate it after that.
>>
>> That is long enough away that I unfortunately have to revert (see 34e49d3e85a6dd03856af0fb4b7f8d8ae1f4f06a <https://reviews.llvm.org/rG34e49d3e85a6dd03856af0fb4b7f8d8ae1f4f06a>). Please note that in the LLVM project reverts are common/frequent (we have a policy of 'revert often'), and should not be taken negatively!  Please feel free to re-submit this patch with a fix/the test provided, and we'll re-review promptly!
>
> Thanks for the revert!
>
> It seems that the problem is around the list initializer in template variable definition.
> Minimum reproducible example:
> `template <typename T> constexpr T foo{};`
>
> When the `TextNodeDumper` enabled through `-ast-dump` visits a constexpr `VarDecl` with initializer, it evaluates the `VarDecl` and prints the value if evaluation comes successful.
> (https://github.com/llvm/llvm-project/blob/55903151a2a505284ce3fcd955b1d394df0b55ea/clang/lib/AST/TextNodeDumper.cpp#L1825)
> The `VarDecl::evaluateValue()` does not show the notes generated during its failed evaluation. The uninitialized-subobject note is actually generated BEFORE this patch when compiling the reproducers with `-ast-dump` flag on, but is not shown to the user.
> The message should not be generated here and I assume this is an internal bug that we have to fix before resubmitting this patch.

It seems to me that perhaps the assert you added is not appropriate?  Instead, could we have the OLD diagnostic 'back' as an alternate form (when this is empty)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146358/new/

https://reviews.llvm.org/D146358