cor3ntin marked 20 inline comments as done. cor3ntin added inline comments.
================ Comment at: clang/docs/ReleaseNotes.rst:507-508 ``-std=gnu++14`` to their build settings to restore the previous behaviour. +- Implemented DR2631. Invalid ``consteval`` calls in default arguments and default + member initializers are diagnosed when and if the default is used. ---------------- aaron.ballman wrote: > cor3ntin wrote: > > cor3ntin wrote: > > > aaron.ballman wrote: > > > > Should this be listed as a potentially breaking change as this now 1) > > > > potentially changes the value people were getting for > > > > `source_location::current()` as a default argument, and 2) potentially > > > > causes instantiation differences due to the change in ODR use? > > > No because the current value of `source_location::current()` is invalid > > > per library wording ( i should reference the issue though) and 2/ i don't > > > think there are any difference in odr use except the point of diagnostic. > > > Or at least i don't think you could observe it? > > The wording for source location > > http://eel.is/c++draft/support.srcloc#cons-2 > > > > Without that issue resolution this calls for magic. > That's correct from a standards perspective, but I'm talking about the > reality which is that this is a behavioral change that our users could notice > and could impact code outside of library calls too. > > FWIW, even with this issue resolution, library is calling for magic (magical > thinking) IMO -- it's a constant evaluated call that doesn't produce constant > results and thus isn't suitable to be called immediately... even in an > immediate context... despite being called an immediate function. > > But thinking about it more... I think we don't need to call it out as a > potentially breaking change. Real consteval calls give the same answer every > time you call them with the same input, so this really should only impact > people using `source_location::current()` because that doesn't match the > consteval model. I think there are several things to consider. * This is a bug fix https://github.com/llvm/llvm-project/issues/56379 * libc++ does not ship `<source_location>` (specifically because of this bug) * `<experimental/source_location>` is not affected (because the current call is not `consteval`) I think we should call out the issue either way though. ================ Comment at: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp:29-30 + return defaulted; + }() // expected-note {{in the default initalizer of 'defaulted'}} +) { + return 0; ---------------- aaron.ballman wrote: > I think it'd be useful to show that use in an unevaluated context still works. agreed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
