[PATCH] D95244: [clang][AST] Handle overload callee type in CallExpr::getCallReturnType.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang/unittests/Tooling/SourceCodeTest.cpp:630
+  f(t);
+  // CalleeType in getCallReturntype is Overload and dependent
+}
----------------
balazske wrote:
> hokein wrote:
> > CalleeType is not a specific term in `getCallReturnType`, just `CalleeType is overload and dependent`, I think we can also verify this in the Visitor.OnCall callback. 
> > 
> > IIUC, the patch fixes three different crashes
> > - calling getCallReturntype on the `f(t)` expr 
> > - calling getCallReturntype on the `f_overload()` expr 
> > - calling getCallReturntype on the `a.f_overload(p);` expr
> > 
> > I think it would be much clear to express them in the `OnCall` callback (rather than calling `getCallReturnType` on every Expr). One option is to use the annotation, and identify the expression by location like below. An other benefit is that we can unify the test code (instead of duplicating them) without hurting the code readability.
> > 
> > ```
> > template<class T, class F>
> > void templ(const T& t, F f) {
> >   $crash1[[f]](t);
> >   // CalleeType in getCallReturntype is Overload and dependent
> > }
> > 
> > struct A {
> >   void f_overload(int);
> >   void f_overload(double);
> > };
> > 
> > void f() {
> >   int i = 0;
> >   templ(i, [](const auto &p) {
> >     $crash2[[f_overload]](p); // UnresolvedLookupExpr
> >     // CalleeType in getCallReturntype is Overload and not dependent
> >   });
> > 
> >   templ(i, [](const auto &p) {
> >     A a;
> >     a.$crash3[[f_overload]](p); // UnresolvedMemberExpr
> >     // CalleeType in getCallReturntype is BoundMember and has overloads
> >   });
> >   
> > }
> > ```
> >  
> The current test finds every `CallExpr` and calls the `getCallReturnType` on it. The test should verify that this function works, so at least calling it is needed. An additional check could be to verify that the "Callee" is really of the specific kind (`UnresolvedLookupExpr` and the others), this can be added. I do not get it how the annotations can be used here, it is possible only to get the position of the code in the string but how to use this value?
yes, I'd like to avoid the current test pattern where we invoke `getCallReturnType` on every `CallExpr`, and don't verify the call-expr kind.

you can find the corresponding expr by matching the annotation locations, mostly can be done in the `OnCall` callback, like

```
Visitor.OnCall = [](..Expr) {
   // check if location of Expr is one of the annotation locations
   //  - no: return
   //  - yes: find out the which one -- let's say, crash2, then we know the Expr should be UnresolvedLookupExpr, and the CalleeType is Overload and not dependent, and verify them.
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95244