Skip to content

[CSDiag] Start chipping away at FailureDiagnosis::diagnoseAmbiguity #29415

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

Merged
merged 2 commits into from
Jan 24, 2020

Conversation

hborla
Copy link
Member

@hborla hborla commented Jan 24, 2020

Remove some special cases in FailureDiagnosis::diagnoseAmbiguity, including:

  • Invalid use of _. Find solutions using holes for the type of the invalid underscore.
  • Empty collection literals without contextual type
  • Nil literal without contextual type

@hborla hborla requested a review from xedin January 24, 2020 03:21
@hborla
Copy link
Member Author

hborla commented Jan 24, 2020

@swift-ci please smoke test

@hborla
Copy link
Member Author

hborla commented Jan 24, 2020

Next to go is ambiguity due to unresolved member references for cases like:

func f(_: Int) {}
func f(_: String) {}

f(.nothing)

Currently working on generalizing the strategy in diagnoseAmbiguityWithFixes to handle this

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I just have a slight concern about a new hole.

.highlight(E->getSourceRange());
return;
}

// Diagnose ".foo" expressions that lack context specifically.
if (auto UME =
dyn_cast<UnresolvedMemberExpr>(E->getSemanticsProvidingExpr())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, I think this should be obsolete as well because diagnostics for _ = .foo have been ported already as missing base type for unresolved member.

Copy link
Member Author

@hborla hborla Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't yet :/ it's still handling cases like this, where the solver does find multiple solutions with fixes but it isn't yet diagnosed in diagnoseAmbiguityWithFixes :

func f(_: Int) {}
func f(_: String) {}

f(.nothing)

I'm currently working on this case though!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that comment on top threw me off :)

@hborla hborla merged commit af847c2 into swiftlang:master Jan 24, 2020
@hborla hborla deleted the ambiguity-special-cases branch January 24, 2020 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants