-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #9772: Normalize value definitions #9836
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
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
5899de2
Fix #9772: Normalize value definitions
tudorv91 5402b85
Fix #9926: Normalize value definitions
tudorv91 a17b774
Update compiler/src/dotty/tools/dotc/ast/Desugar.scala
tudorv91 341afb3
Update compiler/src/dotty/tools/dotc/core/NameOps.scala
tudorv91 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
-- Error: tests/neg/illegal-extension.scala:2:6 ------------------------------------------------------------------------ | ||
2 | def extension_n: String = "illegal method" // error: illegal name: extension_n may not start with `extension_` | ||
| ^^^^^^^^^^^ | ||
| illegal name: extension_n may not start with `extension_` | ||
-- Error: tests/neg/illegal-extension.scala:4:6 ------------------------------------------------------------------------ | ||
4 | val extension_val = 23 // error: illegal name: extension_val may not start with `extension_` | ||
| ^^^^^^^^^^^^^ | ||
| illegal name: extension_val may not start with `extension_` | ||
-- Error: tests/neg/illegal-extension.scala:5:14 ----------------------------------------------------------------------- | ||
5 | private var extension = Nil // error: illegal setter name: `extension_=` | ||
| ^^^^^^^^^ | ||
| illegal setter name: `extension_=` | ||
-- Error: tests/neg/illegal-extension.scala:16:23 ---------------------------------------------------------------------- | ||
16 |extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` | ||
| ^^^^^^^^^^^^^ | ||
| illegal name: extension_foo may not start with `extension_` | ||
-- Error: tests/neg/illegal-extension.scala:9:6 ------------------------------------------------------------------------ | ||
9 | var extension = 1337 // error: illegal setter name: `extension_=` | ||
| ^^^^^^^^^ | ||
| illegal setter name: `extension_=` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,20 @@ | ||
trait A { | ||
def extension_n: String = "illegal method" // error: illegal method name: extension_n may not start with `extension_` | ||
def extension_n: String = "illegal method" // error: illegal name: extension_n may not start with `extension_` | ||
type extension_type = Int // allowed because it's a type alias | ||
val extension_val = 23 // error: illegal name: extension_val may not start with `extension_` | ||
private var extension = Nil // error: illegal setter name: `extension_=` | ||
} | ||
|
||
extension (x: Any) def extension_foo: String = "foo" // error: illegal method name: extension_foo may not start with `extension_` | ||
class B { | ||
var extension = 1337 // error: illegal setter name: `extension_=` | ||
} | ||
|
||
class C { | ||
private var extension = "OK" // allowed because it does not require a setter | ||
} | ||
|
||
extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` | ||
extension (some: Any) def valid_extension_name: String = { | ||
var extension = "foo" // `extension` name allowed because it doesn't require a setter | ||
s"$extension bar" | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
perhaps
isSetterNeeded
could be a default parameter fornormalizeName
to avoid calling it twice, and then swap the conditions in the Valdef branch (case vdef: ValDef if isSetterNeeded && name.isExtension
), but if the performance isn't impacted that much this is minorUh oh!
There was an error while loading. Please reload this page.
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.
Yes, I considered that as well, but decided against it in order not to particularize the arg list of
normalizeName
with a param that is relevant to one branch. The second call would happen only if thevalDef
's name isextension
, so I would expect it to be highly exceptional