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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to