-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix #22833: allow unroll annotation in methods of final class #22926
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
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -132,9 +132,9 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase => | |||||||||||
then | ||||||||||||
false // not an error, but not an expandable unrolled method | ||||||||||||
else if | ||||||||||||
method.is(Deferred) | ||||||||||||
method.isLocal | ||||||||||||
|| !method.isEffectivelyFinal | ||||||||||||
Comment on lines
+135
to
+136
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. @lihaoyi confirmed that abstract methods shouldn't be allowed to have
Suggested change
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'm surprised that no tests were affected by this change! 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 the only place where the extra check for
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've never seen 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 believe the only place it's used is for the primitive classes in the standard library like 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. Yeah ... that's really just primitives. A 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.
@olhotak You're right, I was context switching when I made the comment. I've rechecked the diff and everything seems correct to me. |
||||||||||||
|| isCtor && method.owner.is(Trait) | ||||||||||||
|| !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) | ||||||||||||
|| method.owner.companionClass.is(CaseClass) | ||||||||||||
&& (method.name == nme.apply || method.name == nme.fromProduct) | ||||||||||||
|| method.owner.is(CaseClass) && method.name == nme.copy | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
-- [E207] Declaration Error: tests/neg/unroll-abstractMethod.scala:6:41 ------------------------------------------------ | ||
6 | def foo(s: String, n: Int = 1, @unroll b: Boolean = true): String // error | ||
| ^ | ||
| Cannot unroll parameters of method foo: it must not be abstract | ||
| Cannot unroll parameters of method foo because it can be overridden | ||
-- [E207] Declaration Error: tests/neg/unroll-abstractMethod.scala:10:41 ----------------------------------------------- | ||
10 | def foo(s: String, n: Int = 1, @unroll b: Boolean = true): String // error | ||
| ^ | ||
| Cannot unroll parameters of method foo: it must not be abstract | ||
| Cannot unroll parameters of method foo because it can be overridden |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
-- [E207] Declaration Error: tests/neg/unroll-illegal3.scala:7:31 ------------------------------------------------------ | ||
7 | def foo(s: String, @unroll y: Boolean) = s + y // error | ||
| ^ | ||
| Cannot unroll parameters of method foo: it is not final | ||
| Cannot unroll parameters of method foo because it is a local method | ||
-- [E207] Declaration Error: tests/neg/unroll-illegal3.scala:12:29 ----------------------------------------------------- | ||
12 | def foo(s: String, @unroll y: Boolean) = s + y // error | ||
| ^ | ||
| Cannot unroll parameters of method foo: it is not final | ||
| Cannot unroll parameters of method foo because it can be overridden | ||
-- [E207] Declaration Error: tests/neg/unroll-illegal3.scala:16:29 ----------------------------------------------------- | ||
16 | def foo(s: String, @unroll y: Boolean): String // error | ||
| ^ | ||
| Cannot unroll parameters of method foo: it must not be abstract | ||
| Cannot unroll parameters of method foo because it can be overridden |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import scala.annotation.{experimental,unroll} | ||
|
||
@experimental final class Foo { | ||
def bar(@unroll x: Int = 0) = x + 1 | ||
} |
Uh oh!
There was an error while loading. Please reload this page.