[backport] SI-10071 and SI-8786 #5557
Conversation
When generating a varargs forwarder for
def foo[T](a: T*)
the parameter type of the forwarder needs to be Array[Object]. If we
gnerate Array[T] in UnCurry, that would be erased to plain Object, and
the method would not be a valid varargs.
Unfortunately, setting the parameter type to Array[Object] lead to
an invalid generic signature - the generic signature should reflect the
real signature.
This change adds an attachment to the parameter symbol in the varargs
forwarder method and special-cases signature generation.
Also cleanes up the code to produce the varargs forwarder. For example,
type parameter and parameter symbols in the forwarder's method type were
not clones, but the same symbols from the original method were re-used.
Make sure that methods annotated with varargs are properly mixed-in. This commit splits the transformation into an info transformer (that works on all symbols, whether they come from source or binary) and a tree transformer. The gist of this is that the symbol-creation part of the code was moved to the UnCurry info transformer, while tree operations remained in the tree transformer. The newly created symbol is attached to the original method so that the tree transformer can still retrieve the symbol. A few fall outs: - I removed a local map that was identical to TypeParamsVarargsAttachment - moved the said attachment to StdAttachments so it’s visible between reflect.internal and nsc.transform - a couple more comments in UnCurry to honour the boy-scout rule
Cloning the original symbol in its entirety, rather than cloning its type/value parameters individually. `cloneSymbol` takes care of all the tricky substitutions for us!
| @@ -77,6 +101,60 @@ trait UnCurry { | |||
| } | |||
| } | |||
|
|
|||
| private def varargForwarderSym(currentClass: Symbol, origSym: Symbol, newInfo: Type): Symbol = { | |||
| val forwSym = currentClass.newMethod(origSym.name.toTermName, origSym.pos, VARARGS | SYNTHETIC | origSym.flags & ~DEFERRED) | |||
There was a problem hiding this comment.
note that this also includes the fix for SI-10059 / #5534 which was not previously backported to 2.11.x. maybe add its test?
There was a problem hiding this comment.
Thanks, I'll have a look. I'm a bit surprised about the failing test, since this seemed to pass on my system. Also, there are some compiler crashes, weird... :-/
|
70f11f1 passes all tests on my machine (using the ant build). And for the tip of the branch, Jason's commit: |
|
the 2.11.x build has a spurious failure that we haven't managed to nail down yet: scala/scala-dev#251. but it's happening so much lately that it's becoming really hard to get a 2.11.x PR green :-( for the failure of |
|
I re-ran the build on the first commit, no spurious failures this time. The failure of |
|
How should I build? Using ant or sbt? It seems the CI is using Sbt, but the docs say ant is the official build. At any rate, the tests should pass with or without optimizations, right? |
|
I built with optimizations on ( |
|
I also didn't manage to reproduce it - until just now, by switching to JDK 6. Here's the log file: |
|
so maybe the |
|
Oh, thanks! I will try Java 6... |
|
|
|
I didn't forget about this, but got caught up in work prior to Scala Exchange. I'll get back to it next week. |
|
Ping! 2.11.9 PRs are due soon. We'd like to cut the tag in two weeks (Jan 20th). |
|
(FYI, to use brew cask for older versions, you also first need to run |
|
Java bug? |
|
I guess it's not really a bug per se -- the docs https://docs.oracle.com/javase/6/docs/api/java/lang/reflect/Method.html#toGenericString() don't mention the |
|
Rebased and test fixed in #5630 |
|
Thanks a lot @adriaanm. Sorry I lagged so much. |
|
Thanks for getting the ball rolling!
…On Tue, Jan 24, 2017 at 00:45 Iulian Dragos ***@***.***> wrote:
Thanks a lot @adriaanm <https://github.com/adriaanm>. Sorry I lagged so
much.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5557 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFjy6uDAgnhQgtxQ1gjBVuyctNKid-mks5rVbo5gaJpZM4K8qqR>
.
|
The fix in #5556 depends on 70f11f1, which refactored the relevant code parts in UnCurry. A backport without that commit would be essentially a rewrite and IMO more risky than just back-porting both.