[Diagnostics] Don't filter overload set candidates based on labels in lookup #26302
Conversation
xedin
force-pushed
the
remove-label-mismatch-from-lookup
branch
from
2eb81d6 to
cae6beb
Compare
…reason This helps with: - Diagnostics because solver would get more choices to work with in diagnostic mode; - Avoid adding the same overload multiple times (retry after label mismatch and no viable candidates); - Unify overload handling/filtering in `simplfyAppliedOverloads`.
…Labels` to accept arguments directly Since `areConservativelyCompatibleArgumentLabels` is only used by `simplifyAppliedOverloads` now, it's easy to pass arguments directly instead of trying to form them from list of labels.
…f UR_LabelMismatch
…ous labels The rule is that if there is a label it's evidence that the intent is to reference a particular overload where that label would match, so let's try to de-prioritize fixes for overloads where labels didn't line up correctly and suggestion is to remove certain labels vs. fixes when labels did line up but types or requirements didn't.
…ject lookup If we're referencing AnyObject and we have argument labels, put the argument labels into the name: we don't want to look for anything else, because the cost of the general search is so high.
…c mode Don't attempt disjunction optimization in "diagnostic mode" because in such mode we'd like to attempt all of the available overloads regardless of of problems related to missing or extraneous labels and/or arguments.
xedin
force-pushed
the
remove-label-mismatch-from-lookup
branch
from
cae6beb to
97b0181
Compare
|
This is getting pretty big already so in a follow up I'm going to remove |
|
@swift-ci please test |
| @@ -444,8 +444,7 @@ let _ : Color = .rainbow // expected-error {{member 'rainbow' is a function; di | |||
| let _: Color = .overload(a : 1.0) // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}} | |||
| let _: Color = .overload(1.0) // expected-error {{ambiguous reference to member 'overload'}} | |||
| // expected-note @-1 {{overloads for 'overload' exist with these partially matching parameter lists: (a: Int), (b: Int)}} | |||
| let _: Color = .overload(1) // expected-error {{ambiguous reference to member 'overload'}} | |||
| // expected-note @-1 {{overloads for 'overload' exist with these partially matching parameter lists: (a: Int), (b: Int)}} | |||
There was a problem hiding this comment.
So, this bit could actually be a regression in practice, because notes in the same file are shown more nicely in Xcode than notes in other files, and especially if the candidates are imported and thus don't have any file. I know that's a lousy thing to make a consideration, but it may still be interesting.
There was a problem hiding this comment.
Should the notes be attached to the source location of the call then?
There was a problem hiding this comment.
Similar thing happens with requirement diagnostics at the moment - they'd get attach to the declaration of the requirement which as you mention might not be as good...
There was a problem hiding this comment.
Well, I wouldn't want to issue separate notes at the call site either. If we can't combine them into one message like before, I guess we should just take the hit. If we can, though, we could conditionalize on "are any of the overloads in other modules" or "do the diagnostic options have 'editor mode' on" or both.
There was a problem hiding this comment.
I don't think we can because that could be all sorts of different problems e.g. requirement failure, availability, labeling, type mismatch etc. on different overloads...
|
Build failed |
…a note Each candidate with incorrect labels (but everything else lined up) gets a note on its declarationm which says what is expected and what has been given.
xedin
force-pushed
the
remove-label-mismatch-from-lookup
branch
from
97b0181 to
1b4f9c3
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
This may be the cause of a regression on Windows. |
|
@compnerd I wonder if my code exposed an issue in ClangImporter which doesn't import some of the declarations properly on Windows (notes are based on declaration locations)? Unfortunately I don't have any machine running Windows to test this out so any help would be appreciated, it seems like there should indeed be 9 notes for the candidates there. |
Unify label/argument filtering optimizations in
simplifyAppliedOverloadsand remove duplicate logic from
performMemberLookup. This allowsfor better diagnostics and give more control over optimizations to the solver.
Resolves: SR-11155
Resolves: rdar://problem/53234368