Skip to content

Fix "failed to produce diagnostic" with invalid leading dot usage when prepared overloads are enabled #85011

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 2 commits into from
Oct 21, 2025
Merged

Fix "failed to produce diagnostic" with invalid leading dot usage when prepared overloads are enabled #85011

merged 2 commits into from
Oct 21, 2025
+49 −38

Conversation

slavapestov
Copy link
Contributor

No description provided.

@slavapestov slavapestov enabled auto-merge October 20, 2025 19:16
@slavapestov slavapestov force-pushed the prepared-leading-dot-fix branch from 5133c97 to 6fad36c Compare October 20, 2025 20:39
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test
slavapestov
slavapestov commented Oct 20, 2025
View reviewed changes
test/Constraints/operator.swift Outdated
func ?= (pattern: I60954?, version: Self) { // expected-error{{operator '?=' declared in type 'I60954' must be 'static'}}
// expected-error@+2{{operator is not a known binary operator}}
// expected-error@+1{{initializer 'init(_:)' requires that 'I60954' conform to 'StringProtocol'}}
// expected-error@+1{{contextual member reference to initializer 'init(_:)' requires 'Self' constraint in the protocol extension}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is OK, the original bug report this test case came from was that the code just crashes the compiler.
Copy link
Contributor

Choose a reason for hiding this comment

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

The code is not crashing but it’s a diagnostic regression and it’s hard to say what even does it reference because there are no protocols involved here…
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps, but this was the only regression, and it's in a file about operators even though it doesn't concern operators :) I'd really like to land this soon though, it's not worth holding up any longer.

However, I have what appears to be a simple fix. It also seems to improve a diagnostic in one case in static_members_on_protocol_in_generic_context.swift. I'll push a new version of this commit later tonight!
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Leading-dot references are very common in SwiftUI and it would be great to avoid diagnostic regressions with them if possible.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also about inferring a base type from an operator reference so related to operators :)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leading-dot references are very common in SwiftUI and it would be great to avoid diagnostic regressions with them if possible.

We should probably go through the bug tracker to collect more test cases (fixed and unfixed) then. ;-)
@slavapestov
Sema: Add a couple of passing diagnostic tests to static_members_on_p…
8d24710 
…rotocol_in_generic_context.swift It looks like we didn't test leading dot with instance members.
@slavapestov
Sema: Clean up leading dot fix logic in simplifyConformsToConstraint()
1b6ac2c 
Tests still pass without this code, including the tests I just added in the previous commit.
@slavapestov slavapestov force-pushed the prepared-leading-dot-fix branch from 6fad36c to 1b6ac2c Compare October 21, 2025 01:51
@slavapestov
Copy link
Contributor Author

@xedin Please take a look at the new version. It's a bit of a hack because if the overload hasn't been resolved yet, I conservatively assume it's a Self == Foo member, but only if the failed requirement is actually a Self: P requirement. If the operator is resolved, it does the old thing of actually checking if its static.
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test
@xedin
Copy link
Contributor

xedin commented Oct 21, 2025

Why can’t we delay and re-active the constraint in this case anymore? I am just wondering what is the interaction between that and prepared overloads…
@slavapestov
Copy link
Contributor Author

Because we’re simplifying an existing constraint, so TMF_GenerateConstraints is not set because we’re not coming from matchTypes(), which means we don’t record the fix and return Error, which causes “failed to produce diagnostic”. In fact I think there’s a way to trigger this without prepared overloads as well, you just have to end up here from the main solver loop and not addConstraintImpl().
@xedin
Copy link
Contributor

xedin commented Oct 21, 2025

Can we return Unsolved in this case?
@slavapestov
Copy link
Contributor Author

I already tried that, it didn’t work.
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test macOS
@xedin
Copy link
Contributor

xedin commented Oct 21, 2025

Okay sounds good, I think the workaround looks okay!
@slavapestov
Copy link
Contributor Author

@xedin We can certainly revisit it later, I'd like to clean up TMF_GenerateConstraints soon anyway since I think preserving constraint identity as much as possible will be important for more advanced optimizations
@slavapestov
Copy link
Contributor Author

@swift-ci Please clean smoke test macOS
@slavapestov slavapestov merged commit 50cea0a into swiftlang:main Oct 21, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@slavapestov @xedin