code duplication

[Paul Phillips]

This kind of thing is a maintenance disaster.  It comes up at the moment because of https://issues.scala-lang.org/browse/SI-5894 the fix for which involves adding the MACRO flag to that list.  But before I add it in two places like the next guy won't, I thought I'd see if we could do something about that.

It's far from the only example of massive duplication in trunk, unfortunately.

% ack --context=3 'val ExcludedForwarderFlags' src

src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala

1067-    // a plain class lacking companion module, for details see `isCandidateForForwarders`).

1068-    // -----------------------------------------------------------------------------------------

1069-

1070:    val ExcludedForwarderFlags = {

1071-      import Flags._

1072-      // Should include DEFERRED but this breaks findMember.

1073-      ( CASE | SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags )

src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala

208-    val BeanDisplayNameAttr = definitions.getRequiredClass("scala.beans.BeanDisplayName")

209-    val BeanDescriptionAttr = definitions.getRequiredClass("scala.beans.BeanDescription")

210-

211:    final val ExcludedForwarderFlags = {

212-      import Flags._

213-      // Should include DEFERRED but this breaks findMember.

214-      ( CASE | SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags )

[Miguel Garcia]

What we can do for the moment is go ahead with changes in GenJVM, I'll take care of porting them to GenASM.

Given that GenJVM and GenASM have to coexist for now (until GenASM shows it maintains binary compatibility) my suggestion is to coexist with code duplication and all.

I don't see how we could move faster with deprecating GenJVM. Besides feedback from real-world users, we need:
  SI-5836, dedicated build with GenASM emitting 1.6 classfiles 

On the bright side, the coexistence problem GenJVM-GenASM is orthogonal to the release train: GenJVM will remain as default in 2.10

Miguel
http://lampwww.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/

[Paul Phillips]

On Thu, Jun 7, 2012 at 10:53 AM, Miguel Garcia <miguel...@tuhh.de> wrote:

Given that GenJVM and GenASM have to coexist for now (until GenASM shows it maintains binary compatibility) my suggestion is to coexist with code duplication and all.

I don't see how we could move faster with deprecating GenJVM. Besides feedback from real-world users, we need:
  SI-5836, dedicated build with GenASM emitting 1.6 classfiles

This is all irrelevant to the question of code duplication.  We do have the means to share identical code between them.  It's not like they're in different projects, or different repositories, or even in different packages.

And I can't fix things in genjvm and wait for them to be ported to genasm, in no small part because I'm only using genasm now.  But I wouldn't work that way anyway.  I'd prefer to leave things consistently broken.

[Miguel Garcia]

I guess we can't just pretend GenASM isn't there (that was my main line of argumentation).

Still, removing code duplication is not without costs:

  (a) extensive testing for GenJVM would be needed (it's going to be in M5, the RC, etc.)
  (b) takes time away from fixing optimizer bugs and from improving its speed (at least my case).

I agree, some patches could be applied only once, yet others seem resistant to be cleanly factored out (SI-4804 comes to mind).

It's a tough call. Unless someone volunteers to readily and flawlessly port all patches in the direction GenASM to GenJVM.


Miguel
http://lampwww.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/



On Thursday, June 7, 2012 8:20:16 PM UTC+2, Paul Phillips wrote: