Skip to content
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

Error type sneaks into annotation arguments #5892

Closed
scabug opened this issue Jun 6, 2012 · 12 comments
Closed

Error type sneaks into annotation arguments #5892

scabug opened this issue Jun 6, 2012 · 12 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jun 6, 2012

Views should not be allowed when typing the arguments of annotations.

This is one way the problem manifests:

~/code/scala scala29
Welcome to Scala version 2.9.1.final (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_31).
Type in expressions to have them evaluated.
Type :help for more information.

scala> implicit def intToGroup(i: Int) = Specializable.Integral
<console>:7: error: not found: value Specializable
       implicit def intToGroup(i: Int) = Specializable.Integral
                                         ^

scala> class foo(a: String) extends StaticAnnotation
defined class foo

scala> implicit def i2s(i: Int) = ""
i2s: (i: Int)java.lang.String

scala> @foo(1: String) def blerg {}
blerg: Unit
~/code scala210
Welcome to Scala version 2.10.0-20120504-065643-e52be82eef (Java HotSpot(TM) 64-Bit Server VM, Java 1.6.0_31).
Type in expressions to have them evaluated.
Type :help for more information.


scala> class foo(a: String) extends annotation.StaticAnnotation
defined class foo

scala> implicit def i2s(i: Int) = ""
warning: there were 1 feature warnings; re-run with -feature for details
i2s: (i: Int)String

scala> @foo(1: String) def blerg {}
<console>:9: error: type mismatch;
 found   : Int(1)
 required: String
       @foo(1: String) def blerg {}
            ^
error: 
     while compiling:  <console>
       current phase:  pickler
     library version:  version 2.10.0-20120504-065643-e52be82eef
    compiler version:  version 2.10.0-20120504-065643-e52be82eef
  reconstructed args:  

uncaught exception during compilation: scala.reflect.internal.FatalError
scala.reflect.internal.FatalError: bad type: <error>(class scala.reflect.internal.Types$ErrorType$)
        at scala.tools.nsc.symtab.classfile.Pickler$Pickle.scala$tools$nsc$symtab$classfile$Pickler$Pickle$$putType(Pickler.scala:226)
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 6, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5892?orig=1
Reporter: @retronym
Affected Versions: 2.10.0

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 6, 2012

@retronym said:
This isn't so rosy, either:

scala> @foo({println("foo"); ""}) def blerg {}
blerg: Unit
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 6, 2012

@retronym said:
Oh well, it's a StaticAnnotation, so maybe it's not a problem.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 6, 2012

@retronym said:
On second thoughts, while disabling view does fix my the problem, it isn't backed up by the spec.

A more conservative fix would be ensure that the implicit members of a class are not visible when typing annotations of class type/value parameters.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 7, 2012

@lrytz said:
i cannot see a reason to disallow implicit conversions for annotation arguments, seems very non-regular. why should it be fixed that way?

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 7, 2012

@retronym said (edited on Jun 7, 2012 7:54:32 AM UTC):
I agree, that's what I meant in the previous comment.

We just have to prevent cyclic inheritance. I haven't extracted a small test case to demonstrate this yet, but you can see it with this branch of Scalala [1]. This is raised as a type error, and not caught by the isCyclicOrErroneous check in implicit search.

But the root problem seems to be incorrect scoping -- b2s should not be in implicitss when typing the annotation in this example:

class annot(a: String) extends StaticAnnotation
class A[@annot(false) X] {
  implicit def b2s(b: Boolean): String = ""
}

I tried to set Context#isSelfSuperCall [2] when typing class type parameters to achieve this, but the annotation was typed with a different context, that of the typer in Namers.

[1] https://github.com/retronym/Scalala/tree/d6fccf4bfeb55cb931499098b8047a25ca8b16a5
[2] https://github.com/scala/scala/blob/b801c8432dd5520f1564d3acd902baa47c6e2bd8/src/compiler/scala/tools/nsc/typechecker/Contexts.scala#L638

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 7, 2012

@hubertp said:
If it is a typer from Namers then we are probably dealing with a LazyType which is just being completed. Not reporting those is going to be tough since contexts in Namers are almost always not silent.

I will have a look at this bug if you are not in a hurry.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 7, 2012

@retronym said:
Yep, I saw a LazyType along the way. I think the main priority is to fix the implicit scoping problem, that is, make the code in my previous comment illegal.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 7, 2012

@retronym said:
This does the job, but I'm pretty clueless in the land of Namers, so consider it more a sketch than the finished article.

https://github.com/retronym/scala/commits/ticket/5892

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 7, 2012

@retronym said:
@lukas: I'm a bit stuck on this one; could you please take if from here?

scala/scala#687 (comment)

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 18, 2012

@scabug scabug closed this Jul 18, 2012
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 18, 2012

@lrytz said:
fixed in [https://github.com/scala/scala/commit/776105a43ef7a7a7c32be8111b496a6762a8ac68]

@scabug scabug added this to the 2.10.0-M5 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.