-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CSGen] Turn invalid decls into holes #34584
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
Changes from all commits
f1fd528
409aa35
7e854ef
1c65a55
8e95096
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1227,9 +1227,11 @@ namespace { | |
if (knownType) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just above this, within the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for code completion we'd want to preserve as much structure as possible so replacing everything with a single hole might not be the best way... I was thinking more of a second case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is, could we remove it and make the below behave the same way, preserving any wrapping types? (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't see your reply before I sent that, sorry. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I could try to unify logic here to preserve some more structure for diagnostics, I'll give it a try! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into this some more and it looks like for solver and diagnostics in particular it make more sense to keep invalid declarations represented as type variables so they could assume any time from context instead otherwise we'd risk diagnosing issues which are effectively already diagnosed while validating declarations... |
||
// If the known type has an error, bail out. | ||
if (knownType->hasError()) { | ||
auto *hole = CS.createTypeVariable(locator, TVO_CanBindToHole); | ||
(void)CS.recordFix(AllowRefToInvalidDecl::create(CS, locator)); | ||
if (!CS.hasType(E)) | ||
CS.setType(E, knownType); | ||
return nullptr; | ||
CS.setType(E, hole); | ||
return hole; | ||
} | ||
|
||
if (!knownType->hasHole()) { | ||
|
@@ -1258,14 +1260,13 @@ namespace { | |
} | ||
} | ||
|
||
// If we're referring to an invalid declaration, don't type-check. | ||
// | ||
// FIXME: If the decl is in error, we get no information from this. | ||
// We may, alternatively, want to use a type variable in that case, | ||
// and possibly infer the type of the variable that way. | ||
// If declaration is invalid, let's turn it into a potential hole | ||
// and keep generating constraints. | ||
if (!knownType && E->getDecl()->isInvalid()) { | ||
CS.setType(E, E->getDecl()->getInterfaceType()); | ||
return nullptr; | ||
auto *hole = CS.createTypeVariable(locator, TVO_CanBindToHole); | ||
(void)CS.recordFix(AllowRefToInvalidDecl::create(CS, locator)); | ||
CS.setType(E, hole); | ||
return hole; | ||
} | ||
|
||
// Create an overload choice referencing this declaration and immediately | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5 | ||
// REQUIRES: objc_interop | ||
// REQUIRES: OS=macosx | ||
|
||
import SwiftUI | ||
|
||
var foo = doesntExist // expected-error {{cannot find 'doesntExist' in scope}} | ||
|
||
struct ContentView: View { | ||
var body: some View { | ||
VStack { | ||
Text(verbatim: foo) | ||
} | ||
} | ||
} |
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.
When would the solver actually emit this error?
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.
In cases when actual problem with declaration hasn't been diagnosed due to a bug (hopefully never).
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.
I was thinking about adding bit about asking to report it just like we do with actual fallback diagnostic.