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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
