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