-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #2631: Add custom error message #3802
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
@biboudis do not forget this one |
@@ -389,6 +389,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit | |||
} | |||
else if (name.toTermName == nme.ERROR) | |||
UnspecifiedErrorType | |||
else if (ctx.owner.isConstructor && ctx.owner.owner.unforcedDecls.toList.exists(_.name == tree.name)) |
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 would factor out the second test into a def. It is not clear what it does. Maybe:
def isNotAccesibleFromConstructor(name: Name) = {
val classMembers = ctx.owner.owner.unforcedDecls
classMembers.lookup.(name).exists
}
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.
If the test is factored out it is not sufficient to ensure that it is not accessible from the constructor.
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 will use the .lookup.(name).exists
though.
7c486df
to
d83d09d
Compare
else if (ctx.owner.isConstructor && ctx.owner.owner.unforcedDecls.lookup(tree.name).exists) { | ||
// If the field existed but was not found we assume that | ||
// we where in the context of an argument of the super constructor | ||
errorType(ex"$tree is not accessible from super constructor arguments", tree.pos) |
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.
@smarter do you think this condition is enough?
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.
Can't you check ctx.mode.is(InSuperCall)
? (The name is misleading, it's set for both this- and super-call, see the method superOrThisCallContext)
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.
Also, don't say "super constructor", a this call is not a super call, it's just a constructor call
@@ -396,7 +396,12 @@ class Typer extends Namer | |||
} | |||
else if (name.toTermName == nme.ERROR) | |||
UnspecifiedErrorType | |||
else | |||
else if (ctx.owner.isConstructor && ctx.mode.is(Mode.InSuperCall) && |
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.
Doesn't ctx.mode.is(Mode.InSuperCall)
implies ctx.owner.isConstructor
?
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.
It might be a method super call with super.xys
.
No description provided.