don't try to create tags w/o scala-reflect.jar #1360
Conversation
|
From the test-case it's not clear to me what will happen exactly? Is bailing out silently the right behavior? |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/541/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1249/ |
|
jenkins job pr-rangepos: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/541/ |
|
jenkins job pr-scala-testsuite-linux-opt: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1249/ |
|
killed the kitteh on purpose, since we've got an update for the pull request |
|
PLS REBUILD ALL |
|
@gkossakowski updated the pull request with more explanations |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/543/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1251/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/543/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1251/ |
|
updated with more tests |
Since recently type tags have relocated to scala-reflect.jar, meaning that they are no longer always on library classpath. In the compiler we do have code that generates type tags, and this code is bound to fail if scala-reflect.jar isn't there. I though this wouldn't be a problem, because type tag materialization is only going to be triggered by users explicitly requesting a type tag. That's generally true, but I overlooked a corner case. Since we provide manifest <-> type tag compatibility, manifest lookup can sometimes trigger tag lookup, which might result in tag synthesis, which blows up like this: http://groups.google.com/group/scala-internals/browse_thread/thread/166ce4b71b7c46bb This commit also ensures that type tag generation/interop doesnt sneak into the code of the libraries that don't have scala-reflect.jar on their classpath. For details refer to the discussion at scala-internals: http://groups.google.com/group/scala-internals/browse_thread/thread/72f6ce3010f4d8
|
PLS REBUILD ALL |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/545/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1253/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1253/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/545/ |
| resolveTag(pos, taggedTp, allowMaterialization) | ||
| } | ||
| def resolveTypeTag(pos: Position, pre: Type, tp: Type, concrete: Boolean, allowMaterialization: Boolean = true): Tree = | ||
| // if someone requests a type tag, but scala-reflect.jar isn't on the library classpath, then bail |
There was a problem hiding this comment.
This would seem to merit an explanatory warning right here, rather than waiting for less specific errors to trickle up.
There was a problem hiding this comment.
The combination of input argument that leads to bailing comes from manifest lookup (to support manifest <-> type tag convertibility).
I think it would be an overkill to warn the user every time just because he's using manifests and happens not to have scala-reflect.jar on the classpath.
There was a problem hiding this comment.
Mmm. OK, I'll take your word for it. LGTM then.
don't try to create tags w/o scala-reflect.jar
Since recently type tags have relocated to scala-reflect.jar,
meaning that they are no longer always on library classpath.
In the compiler we do have code that generates type tags, and this code
is bound to fail if scala-reflect.jar isn't there.
I though this wouldn't be a problem, because type tag materialization
is only going to be triggered by users explicitly requesting a type tag.
That's generally true, but I overlooked a corner case. Since we provide
manifest <-> type tag compatibility, manifest lookup can sometimes trigger
tag lookup, which might result in tag synthesis, which blows up like this:
http://groups.google.com/group/scala-internals/browse_thread/thread/166ce4b71b7c46bb