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