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
