aaron.ballman added a comment. In D129488#3758798 <https://reviews.llvm.org/D129488#3758798>, @ilya-biryukov wrote:
> 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. I'm also starting to question that. :-) >> 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. https://eel.is/c++draft/expr.call#7.sentence-3 is what causes the argument to be evaluated at the call expression rather than at the declaration while https://eel.is/c++draft/dcl.fct.default#5 makes it clear that the semantic constraints are checked at the declaration location. I couldn't find explicit wording which says this, but I believe that semantic checking in this case also involves determining whether the immediate function is a valid or not (https://eel.is/c++draft/expr.const#14), which should explain the behavior of your example in https://gcc.godbolt.org/z/qTzrf6Msx. However, https://eel.is/c++draft/dcl.constexpr#5 says that the behavior has to be the same whether the constexpr function is runtime evaluated or constant evaluated, and a function declared `consteval` is a constexpr function (https://eel.is/c++draft/dcl.constexpr#2.sentence-1). That sure does make std::source_location::current() a rather special function -- if the call were to be evaluated at runtime (which it can't because it's an immediate function), it would give different results. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129488/new/ https://reviews.llvm.org/D129488 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
