[cfe-dev] Error recovery when Sema finds only one (non-viable) candidate?

[Sam McCall via cfe-dev]

We were discussing this idea further today.
There are some limitations:
 - we'd weakening current invariants, which could break existing code in
rarely-tested cases. An invalid flag helps fix these problems once you find
them, but not before then.
 - callexpr is only one place we have this type of problem, with whole
subtrees of the AST being discarded. Breaking invariants on many types of
nodes would add lots of complexity and make the AST harder to reason about.

An alternative would be to add some *new* type of node, that would go into
the AST instead of CallExpr.
e.g. RecoveryExpr, with:
 - a source range
 - StmtClass of the node we were trying to create (CallExpr)
 - children (in this case, Exprs for the callee and parameters)
This addresses the above concerns somewhat:
 - As a new node type, only code aware of the concept will interact with it
directly. We only have to worry about code that handles Expr generically,
and generic things like RecursiveASTVisitor (which can skip such nodes by
default).
 - The node can likely be generic enough to handle cases other than
CallExpr.(

I don't have experience adding new node types, but will try to prototype
this.

On Wed, Apr 24, 2019 at 2:38 PM Nikolai Kosjar <Nikolai.Kosjar at qt.io> wrote:

> On 4/24/19 11:16 AM, Sam McCall via cfe-dev wrote:
> > Consider the following broken code:
> >
> >    int foo(int);
> >    void bar() { return foo(); }
> >
> > Currently it produces two diagnostics:
> >    - no matching call for foo
> >    - note: foo(int) not viable, wrong number of args
> > And it produces no AST for the call (bar's CompoundStmt is empty).
> >
> > I'd really like it recover and produce a CallExpr to the one candidate,
> > similar to what happens for typo-correction.
> > My own motivation is to get go-to-definition in libclang/clangd to work
> > better with broken code, though I think there are other benefits.
>
> Another case is when you are typing a function call and need to lookup
> the function documentation - hitting F1/Help is difficult to implement
> without any hint/candidate of the not-yet-completed function call.
>
> > The new behavior might be something like:
> >   - only call for foo is not viable
> >   - note: foo(int) not viable, wrong number of args
> >   - bar() shouldn't return a value
> > And a CallExpr emitted with no args (and maybe an 'invalid' flag).
> >
> > Q1: does this seem desirable in principle?
>
>  From an IDE point of view, it definitely is!
>
> > There are some practical issues:
> >   - A naive implementation (https://reviews.llvm.org/D61057) sometimes
> > emits pre-recovery "not viable: X" diagnostics and post-recovery "error:
> > X" for the same reason X. e.g. for cuda device->host calls.
> >   - Post recovery diagnostics can be noisy in more subtle ways. Test
> > failures for naive implementation:
> > https://gist.github.com/sam-mccall/bb73940e27a79ee41523e52048872f26
> >
> > Maybe it's best to start with only simple cases, e.g. recovering only
> > from too many/too few arguments doesn't break any tests, and covers a
> > lot of broken-code cases.
>
> Sounds good.
>
> > Q2: does anyone have strong opinions about how this should work in
> > practice, see hidden pitfalls, and/or want to review code?
>
> I'm wondering whether the introduction of the invalid flag could lead to
> inconsistencies somewhere (maybe analyzers/checkers?). For example, with
> the new behavior a CallExpr would exist that points to a particular
> candidate, having e.g. N function parameters. But the actual function
> has M != N arguments (let's pretend that default arguments do not exist
> for the sake of the example). It might make sense to have this new
> behavior opt-in.
>
> Besides having an (invalid) CallExpr, it would be also very useful to
> have the function arguments handled. For example, if a function call is
> invalid due to whatever reason, but the arguments "hello" and "there"
> still could be resolved/looked-up in the current scope, the invalid
> CallExpr should have appropriate DeclRefExprs children for these arguments.
>
> This would enable go-to-definition, help/f1 and highlighting for the
> arguments of not yet fully finished function calls.
>
> I've contributed only some simple patches so far, but I could help with
> testing and reviewing where I can.
>
> > Cheers, Sam
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190507/f61ffc31/attachment.html>