[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 31 23:04:47 PDT 2022


ChuanqiXu added a comment.

In D129488#3760982 <https://reviews.llvm.org/D129488#3760982>, @ilya-biryukov wrote:

> @aaron.ballman, that's my reading of the standard as well. Do you think we should proceed with the current approach or is there another direction worth pursuing to make source_location work?
>
> In D129488#3760178 <https://reviews.llvm.org/D129488#3760178>, @ChuanqiXu wrote:
>
>> There is another example we shouldn't make this specific for std::source_location::current(): https://github.com/llvm/llvm-project/issues/57459. I guess we can solve the issue too if we evaluate default argument at the caller position.
>
> I think you're right and it would probably work automatically if we were to recreate the default argument expression on every call where it appears.
> However, going this route for the particular purpose of checking module visibility looks like an overkill. FWIW, see my attempt at this to fix `source_location::current()` with immediate invocations D132941 <https://reviews.llvm.org/D132941>.
> It's complicated, makes default arguments slower and encounters new failures modes, e.g. there new errors with lambdas in default arguments.

It is just an example. I just wanted to say we could solve more problems if we chose that way. The issue of the module may not so painful and it shouldn't be hard to handle it specially. (Although specially handling looks not good, this is the reason why I didn't fix it). But after all, I am not objecting (nor approving) this for the modules example.

---

For this patch, I agree with @jyknight, it'd better to be constexpr instead of consteval. I feel it may be better to reflect this with wg21 instead of hacking in compilers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488



More information about the cfe-commits mailing list