- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.1k
 
Patch empty implicit parens on error recovery #22835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Patch empty implicit parens on error recovery #22835
Conversation
| 
           Requires a tweak: an implicit param list is not supplied implicitly if an arg list is present (but defaults may be supplied in the explicit application). It must not continue implicit application when rewriting. Ha, in other words,   | 
    
6fe89ef    to
    cfa8e9f      
    Compare
  
    | 
           Added a test case for rewrite of   | 
    
c184805    to
    83fda42      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I just added two small comments about the code.
| 
           The spurious failure is cc timeout on posCC, negAll, runAll.  | 
    
ef81b4a    to
    da154fc      
    Compare
  
    | case InfixTuple // r f (x1, ..., xN) where N != 1; needs to be treated specially for an error message in typedApply | ||
| case Regular // r.f(x) | ||
| case Using // r.f(using x) | ||
| case InfixTuple // r f (x1, ..., xN) where N != 1; needs to be treated specially for an error message in typedApply | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This keeps the line under 120 chars, not a standard but nice to have when feasible. class Apply line wraps at that length; it would be nice to alias constructorOnly to cO or something like in chemistry K.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Sorry for taking so long, this PR got lost in my notifications 🙈
da154fc    to
    dc178d0      
    Compare
  
    
Offer a rewrite for an erroneous application
f()where the parameter list isimplicitand a parameter lacks a default arg.This was never official syntax, but was accepted from 3.3.4/3.4.2 due to a bug. The rewrite is for 3.7-migration, using the existing
MigrationVersion.ImplicitParamsWithoutUsing, since that is a similar bit of syntax.Fixes #22792