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

I was also under impression that this should fall out of the C++ semantics, but after playing around with it I am not so sure anymore.

> Isn't it wrong to eagerly evaluate _other_ calls in default args, as well?
> ISTM that source_location is simply _exposing_ the bug in where we evaluate these expressions, but that it's actually a more general problem?

Yes, it will evaluate calls to other `consteval` functions and and I don't think there is a way to notice this if there are no errors in the `consteval` functions. `constexpr` and `consteval` functions should produce the same value when called with the same argument IIUC.
`source_location::current()` is special in that regard and it breaks that assumption. E.g. the fact that `CXXDefaultArgExpr` references the same expression from `ParmVarDecl`.
We could delay evaluation of other `consteval` functions until the default argument is actually used relatively cheaply for other functions, but `source_location::current()` is the only one that requires a complete revamp of how `CXXDefaultArgExpr` works (see D132941 <https://reviews.llvm.org/D132941> for an attempt, it's not pretty, unless there is a simple way I missed).

I am also torn on how to interpret the standard. It might be that evaluating immediate function in default arguments in actually required.
`[dcl.fct.default]p5` mentions we need to do full semantic checking of the initializer for default arguments:

  The default argument has the same semantic constraints as the initializer in a declaration of a variable of the
  parameter type, using the copy-initialization semantics (9.4). The names in the default argument are bound,
  and the semantic constraints are checked, at the point where the default argument appears.

Additionally, the standard requires all immediate invocations to be evaluated (unless they are in immediate function context, which is not the case if default arguments are not for consteval function).
Hence, we should also evaluate the immediate invocations as part of the semantic checking we do for initializer of the default argument.

All major compilers certainly evaluate the `consteval` functions in the default arguments even without calls to the function that has those parameters:
https://gcc.godbolt.org/z/qTzrf6Msx

It might be an implementation quirk and a clarification would be nice. I am leaning towards thinking this behavior is actually standard-compliant.
This does not contradict the fact that default arguments must be evaluated in the context of the caller.
One can't see the difference for `consteval` functions as they must be replaced by constants.

Both GCC and MSVC seem to special-case `std::source_location::current()` from what I can tell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488