rsmith added inline comments.

================
Comment at: clang/lib/Sema/SemaStmt.cpp:711
+    if (CalleeQualType.isNull()) {
+      // The function callee is invalid and already triggered an error.
+      // Avoid compounding errors.
----------------
haberman wrote:
> rsmith wrote:
> > Even in invalid code we should never see a `CallExpr` whose callee has a 
> > null type; if `Sema` can't form an `Expr` that meets the normal expression 
> > invariants during error recovery, it doesn't build one at all. I think you 
> > can remove this `if`.
> Without this if(), I crash on this test case. What do you think?
> 
> ```
> struct TestBadPMF {
>   int (TestBadPMF::*pmf)();
>   void BadPMF() {
>     [[clang::musttail]] return ((*this)->*pmf)(); // expected-error {{left 
> hand operand to ->* must be a pointer to class compatible with the right hand 
> operand, but is 'TestBadPMF'}}
>   }
> };
> ```
> 
> Dump of `CalleeExpr` is:
> 
> ```
> RecoveryExpr 0x106671e8 '<dependent type>' contains-errors lvalue
> |-ParenExpr 0x10667020 'struct TestBadPMF' lvalue
> | `-UnaryOperator 0x10667008 'struct TestBadPMF' lvalue prefix '*' cannot 
> overflow
> |   `-CXXThisExpr 0x10666ff8 'struct TestBadPMF *' this
> `-MemberExpr 0x10667050 'int (struct TestBadPMF::*)(void)' lvalue ->pmf 
> 0x10666ed0
>   `-CXXThisExpr 0x10667040 'struct TestBadPMF *' implicit this
> ```
Ah, right, while the callee will always have a non-null type, that type might 
not be a pointer type.

I think what we're missing here is a check for a dependent callee; checking for 
a dependent context isn't enough to check for error-dependent constructs. 
Probably the simplest thing would be to change the `isDependentContext()` 
checks to also check if the return expression `isInstantiationDependent()`. 
(That would only help with the error-dependent cases for now, but we'd also 
need that extra check in the future if anything like 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2277r0.html goes 
forward, allowing dependent constructs in non-dependent contexts, especially in 
combination with 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1306r1.pdf.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to