Skip to content

[Lexer] Disallow '$' as a start of identifier, special handle '$' #6004

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 1 commit into from
Dec 21, 2016

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Dec 1, 2016

I believe, accepting `$0` as an identifier was not intentional. It was not allowed in Swift2/3.0

let foo: (Int) -> Void = {
  let `$0` = "FOO"
  print($0, `$0`)
}

foo(42) // -> 42 FOO

This behavior was introduced in #5270 which intended to allow `$` as an identifier.

I think, we shouldn't generally consider $ as a valid start character for identifiers.
Instead, handle `$` specially.

@rintaro
Copy link
Member Author

rintaro commented Dec 20, 2016

Ping, could someone please review this?
I believe the current behavior is actually harmful.

let `$0` = "FOO"
let foo: (Int) -> Void = {
  print($0, `$0`)
}

hits assertion.

virtual std::pair<bool, Expr *> (anonymous namespace)::Verifier::walkToExprPre(swift::Expr *):
Assertion `(HadError || !M.is<SourceFile*>() || M.get<SourceFile*>()->ASTStage < SourceFile::TypeChecked) && 
"OverloadedDeclRef" "in wrong phase"' failed.

@CodaFi
Copy link
Contributor

CodaFi commented Dec 20, 2016

Sorry about that, meant to approve this weeks ago. Thanks for fixing this.

@swift-ci please smoke test.

@rintaro
Copy link
Member Author

rintaro commented Dec 20, 2016

No worries. Thanks @CodaFi !

Oops. It seems, somehow, I forgot to add testcases 👎 . I will.

@rintaro rintaro force-pushed the lex-dollar-identifier-head branch from 2797692 to 040f9e8 Compare December 20, 2016 16:51
Allowing 'let `$0` = 1', introduced in 6accc59 was not intentional.
@rintaro rintaro force-pushed the lex-dollar-identifier-head branch from 040f9e8 to 7819d0e Compare December 20, 2016 16:54
// FIXME: Bad diagnostics.
`$0` = 1 // expected-error {{expected expression}}
`$$` = 2 // expected-error {{expected numeric value following '$'}}
`$abc` = 3 // expected-error {{expected numeric value following '$'}}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the Swift3 behavior. Diagnostics doesn't look nice.
We can improve this, but not for now.

@rintaro
Copy link
Member Author

rintaro commented Dec 20, 2016

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Dec 20, 2016

@swift-ci Please smoke test

@rintaro rintaro merged commit 509db74 into swiftlang:master Dec 21, 2016
@rintaro rintaro deleted the lex-dollar-identifier-head branch June 22, 2017 03:59
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