-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Always diagnose request evaluator cycles #24213
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
Conversation
NFC for now, but this problem causes some tests to fail once we enable cycle diagnostics.
If there are no braces, the start and end location of the range is the same token. When performing a lookup at this token's location, don't consider it to be "inside" the braces. NFC for now, but this problem causes some tests to fail once we enable cycle diagnostics.
This comment has been minimized.
This comment has been minimized.
c3084ad
to
713816a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
If there are no braces, the start and end location of the range is the same token, so don't create a TypeOrExtensionBody scope at all. NFC for now, but this problem causes some tests to fail once we enable cycle diagnostics.
713816a
to
3f5a06b
Compare
@swift-ci Please test |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@swift-ci Please test source compatibility |
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.
Very excited for this!
@swift-ci Please test source compatibility |
@@ -1886,8 +1890,7 @@ bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag, ValueDecl *decl, | |||
return false; | |||
} | |||
|
|||
// Determine the Objective-C name of the declaration. | |||
ObjCSelector name = *decl->getObjCRuntimeName(); | |||
auto name = *nameOpt; | |||
auto targetName = *targetNameOpt; |
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 a little confused by these two lines when I read them. I guess the idea is that these should only be None
when the declaration is implicit or a subscript? If so, maybe there should be a comment here or a sentence in the doc comment to that effect.
Turns out only minor fixes were needed, so let's enable this and make sure we don't regress.