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

[Nikolai Kosjar via cfe-dev]

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