Skip to content

[ConstraintSystem] Handle ambiguities caused by requirement failures #63222

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 31, 2023
Merged

[ConstraintSystem] Handle ambiguities caused by requirement failures #63222

merged 4 commits into from
Jan 31, 2023

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Jan 25, 2023 •
edited
Loading

Aggregate all requirement failures (regardless of kind) that belong
to the same locator and diagnose them as an ambiguity (if there is
more than one overload) or as the singular failure if all solutions
point to the same overload.

Resolves: #56173
Resolves: rdar://79357256
Resolves: rdar://104135510

LucianoPAlmeida
LucianoPAlmeida reviewed Jan 26, 2023
View reviewed changes
@LucianoPAlmeida
Copy link
Contributor

@xedin Looking at the logic, I wonder if that could be also fixing #56173? It was some time ago but I do remember seeing requirement fixes, so maybe is possible ...

@xedin
Copy link
Contributor Author

xedin commented Jan 26, 2023

@LucianoPAlmeida it might indeed fix that, I add a test-case.

@LucianoPAlmeida
Copy link
Contributor

@LucianoPAlmeida it might indeed fix that, I add a test-case.

Awesome!

@xedin
Copy link
Contributor Author

xedin commented Jan 26, 2023

@LucianoPAlmeida Case from this comment is indeed improved by these changes, the rest are diagnosed property as well.

@xedin xedin force-pushed the diag-ambiguous-requirement-failures branch 2 times, most recently from da8555e to 57a29c7 Compare January 26, 2023 18:43
@xedin
Copy link
Contributor Author

xedin commented Jan 26, 2023

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Jan 26, 2023

@swift-ci please test Linux platform

1 similar comment
@xedin
Copy link
Contributor Author

xedin commented Jan 26, 2023

@swift-ci please test Linux platform

xedin added 3 commits January 26, 2023 15:55
@xedin
[CSFix] NFC: Add a common base class for all requirement failures
edb6ef0
@xedin
[CSFix] Move diagnoseForAmbiguity to RequirementFix
9b688e9 
The logic is common for all types of fixes that represent
a requirement failure.
@xedin
[ConstraintSystem] Handle ambiguities caused by requirement failures
c7ba055 
Aggregate all requirement failures (regardless of kind) that belong
to the same locator and diagnose them as an ambiguity (if there is
more than one overload) or as the singular failure if all solutions
point to the same overload.
@xedin
Copy link
Contributor Author

xedin commented Jan 27, 2023

@swift-ci please clean test Linux platform

LucianoPAlmeida
LucianoPAlmeida reviewed Jan 27, 2023
View reviewed changes
@xedin xedin force-pushed the diag-ambiguous-requirement-failures branch from 57a29c7 to 09cab70 Compare January 27, 2023 16:14
@xedin
Copy link
Contributor Author

xedin commented Jan 27, 2023

@swift-ci please test

@xedin
[ConstraintSystem] Handle requirement failures associated with inject…
e99f0e2 
…ed result builder methods

Only way to aggregate build* methods is via source locations because
each result builder kind would create a new build* expression node.
@xedin xedin force-pushed the diag-ambiguous-requirement-failures branch from 09cab70 to e99f0e2 Compare January 27, 2023 16:47
@xedin
Copy link
Contributor Author

xedin commented Jan 27, 2023

@swift-ci please test

hborla
hborla approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@hborla hborla left a comment

Choose a reason for hiding this comment

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

Nice!

@xedin
Copy link
Contributor Author

xedin commented Jan 31, 2023

@swift-ci please smoke test macOS platform

2 similar comments
@xedin
Copy link
Contributor Author

xedin commented Jan 31, 2023

@swift-ci please smoke test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jan 31, 2023

@swift-ci please smoke test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jan 31, 2023

@swift-ci please test macOS platform

1 similar comment
@LucianoPAlmeida
Copy link
Contributor

@swift-ci please test macOS platform

@compnerd
Copy link
Member

Please test with following PRs:
swiftlang/swift-driver#1281

@swift-ci please smoke test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jan 31, 2023

Please test with following PRs:
swiftlang/swift-driver#1281

@swift-ci please smoke test macOS platform

@compnerd
Copy link
Member

@swift-ci please smoke test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jan 31, 2023

full suite is failing with unrelated failures:

Failed Tests (6):
  Swift(watchsimulator-x86_64) :: Interpreter/SDK/check_class_for_archiving_log.swift
  Swift(watchsimulator-x86_64) :: stdlib/NSSlowString.swift
  Swift(watchsimulator-x86_64) :: stdlib/StringIndex.swift
  Swift-validation(watchsimulator-x86_64) :: stdlib/String.swift
  Swift-validation(watchsimulator-x86_64) :: stdlib/StringBreadcrumbs.swift
  Swift-validation(watchsimulator-x86_64) :: stdlib/StringUTF8.swift

xcrun: error: sh -c '/Applications/Xcode-beta.app/Contents/Developer/usr/bin/xcodebuild -sdk /Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -find dyld_info 2> /dev/null' failed with exit code 17664: (null) (errno=Invalid argument)
xcrun: error: unable to find utility "dyld_info", not a developer tool or in PATH
falling back to 'xcrun dyldinfo' ...
Child process terminated with signal 6: Abort trap

@xedin xedin merged commit 8d49757 into swiftlang:main Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LucianoPAlmeida LucianoPAlmeida LucianoPAlmeida left review comments

@hborla hborla hborla approved these changes

@slavapestov slavapestov Awaiting requested review from slavapestov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SR-13776] Undiagnosed generic argument mismatch for equatable requirement
4 participants
@xedin @LucianoPAlmeida @compnerd @hborla