zyn0217 wrote:
@cor3ntin I'm stuck here for hours and I think I need some help.
So let me explain what happens before #95660. Consider the example that doesn't
compile before #95660:
```cpp
template <typename F> constexpr int func(F f) {
constexpr bool y = f(1UL);
return y;
}
int main() {
auto predicate = [](auto v) /*implicit constexpr*/ -> bool {
return v == 1;
};
return func(predicate);
}
```
we would encounter an error saying that `undefined function operator()<> cannot
be used in a constant expression`, which is bogus because the lambda body has
been clearly defined. The issue lies in `MarkFunctionReferenced()`, where we
deferred the instantiation of functions that belong to *local classes*. The
logic there may originate from a time when lambdas were not supported, and then
when lambda comes into play, this logic would also impact these lambdas that
are defined within a function.
We won't have the corresponding lambda bodies as we have deferred the
instantiation until the end of the function definition. Therefore, when we
instantiate `func()`, which would entail the evaluation of `f(1)`, which in
turn is the evaluation of the lambda, we would fail immediately because, at
this time, we couldn't see the lambda body.
If I understand correctly, our constant evaluator is designed to be oblivious
of `Sema` and we almost always eagerly evaluate expressions for constexprs.
Following this principle, my patch #95660 does the thing that lets constexpr
functions always get instantiated despite the declaration location. And it just
works for the case above.
However, as you can see, this also subtly caused such a regression. Again, I'll
demonstrate an example:
```cpp
template <typename F> struct recursive_lambda {
template <typename... Args> auto operator()(Args &&...args) const {
return fn(*this, args...);
}
F fn;
};
template <typename F> recursive_lambda(F) -> recursive_lambda<F>;
struct Tree {
Tree *left, *right;
};
int sumSize(Tree *tree) {
auto accumulate =
recursive_lambda{[&](auto &self_fn, Tree *element_node) -> int {
return 1 + self_fn(tree->left) + self_fn(tree->right);
}};
accumulate(tree);
}
```
In this example, we want to deduce the return type of
`recursive_lambda::operator()` with `[Args = Tree*]`. To that end, we have to
instantiate `fn()` which is the lambda defined within `sumSize`. With my patch,
the lambda body gets instantiated immediately, so we end up instantiating
`self_fn()` for its return type. As that function is being instantiated, the
flag `willHaveBody` has been set to true so that we won't instantiate it
endlessly. We bail out of `InstantiateFunctionDefinition` early, and because we
don't have a deduced return type at the time, we diagnose in
`DeduceReturnType().`
But what will happen if we defer that lambda's instantiation? That means we
don't have to instantiate `self_fn` for its return type, and hence, we would
successfully build up a body for `recursive_lambda::operator()` and a return
type for it - we don't have to look into the lambda body because the lambda has
an explicit type.
So far, I have thought of some approaches:
1. Revert 95660. Instantiate the function as needed before we perform the
evaluation. This is what this patch aims to do; however, since we don't have a
common entry for constant evaluation, we probably have to add this
"prerequisite function" to many places, which I feel hesitant to do.
2. Don't revert 95660. Just teach `DeduceReturnType` to not diagnose if the
function is being instantiated. This makes things work but causes some other
regressions, unfortunately.
3. Don't revert 95660. In addition, introspect the evaluation context to help
decide if we should instantiate. This doesn't work because all the cases above
have the same "PotentiallyEvaluated" flag set.
4. Revert 95660 and give up. This is likely a design issue, and we admit we
struggle to resolve them perfectly.
This has afflicted me for half the night, and I really need some guidance!
https://github.com/llvm/llvm-project/pull/98758
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
