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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@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.

I suspect this could be done relatively cheaply and simply inside the `MarkDeclarationsReferencedInExpr` call at the bottom of `Sema::CheckCXXDefaultArgExpr`.
I tried prototyping this and it seems to fit nicely with the current design. We just need to add a few notes to link between the locations of parameter initializers and calls involving default arguments.
(It is still tricky to get right in all cases since we need to enumerate all positions where complete types are required, but certainly does not look impossible).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488