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

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/lib/AST/Expr.cpp:1387
+
+  auto CreateDependentType = [&Ctx]() {
+    ASTContext::GetBuiltinTypeError Error;
----------------
hokein wrote:
> this can be removed, dependent type is a builtin type, so you could just use `Ctx.DependentTy`.
useful, I did not observe that


================
Comment at: clang/unittests/Tooling/SourceCodeTest.cpp:630
+  f(t);
+  // CalleeType in getCallReturntype is Overload and dependent
+}
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95244