[PATCH] D127487: [Sema] Fix assertion failure when instantiating requires expression

Fri Jun 10 07:57:38 PDT 2022

royjacobson added inline comments.

================
Comment at: clang/lib/Sema/SemaConcept.cpp:352
+      [this](const Expr *AtomicExpr) -> ExprResult {
+        // We only do this to immitate lvalue-to-rvalue conversion.
+        return PerformContextuallyConvertToBool(const_cast<Expr*>(AtomicExpr));
----------------
erichkeane wrote:
> ilya-biryukov wrote:
> > erichkeane wrote:
> > > Can you explain this more?  How does this work, and why don't we do that directly instead?
> > That's entangled with `calculateConstraintSatisfaction`. I actually tried to do it directly, but before passing expressions to this function `calculateConstraintSatisfaction` calls `IgnoreParenImpCasts()`, which strips away the lvalue-to-rvalue conversion.
> > And we need this conversion so that the evaluation that runs after this callback returns actually produces an r-value.
> > 
> > Note that the other call to `calculateConstraintSatisfaction` also calls `PerformContextuallyConvertToBool` after doing template substitution into the constraint expression.
> > 
> > I don't have full context on why it's the way it is, maybe there is a more fundamental change that helps with both cases.
> Hmm... my understanding is we DO need these to be a boolean expression eventually, since we have to test them as a bool, so that is why the other attempts the converesion.  If you think of any generalizations of this, it would be appreciated, I'll think it through as well.
Note we already have a related bug about this https://github.com/llvm/llvm-project/issues/54524

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127487