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

Static forwarders are missing after adding @SerialVersionUID #4804

Closed
scabug opened this issue Jul 14, 2011 · 12 comments

Comments

Projects
None yet
1 participant
@scabug
Copy link

commented Jul 14, 2011

In the following code, adding @serialversionuid to the class results in the static forwarders missing from the byte code.

object WithoutUID {
 val instance = new WithoutUID
}
class WithoutUID extends scala.Serializable

object WithUID {
 val instance = new WithUID
}
@SerialVersionUID(0) 
class WithUID extends scala.Serializable{code}

Here is the relevant decompiled byte code:

public class WithoutUID implements Serializable, ScalaObject {
  public static final WithoutUID instance(){
    return WithoutUID.MODULE$.instance();
  }
}

public class WithUID implements Serializable, ScalaObject {
  public static final long serialVersionUID = 0L;
}
@scabug

This comment has been minimized.

Copy link
Author

commented Jul 14, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4804?orig=1
Reporter: Josh Cough (joshcough)
Assignee: @magarciaEPFL
Affected Versions: 2.9.0

@scabug

This comment has been minimized.

Copy link
Author

commented Aug 16, 2011

@paulp said:
Huh. I am initially mystified.

@scabug

This comment has been minimized.

Copy link
Author

commented Aug 16, 2011

@SethTisue said:
it does have a kind of "I stuck a finger in my ear and my dishwasher stopped working" type quality to it.

@scabug

This comment has been minimized.

Copy link
Author

commented Aug 16, 2011

Josh Cough (joshcough) said:
Let me know if you need anymore information. And, be careful where you're sticking your finger.

@scabug

This comment has been minimized.

Copy link
Author

commented Jun 1, 2012

@magarciaEPFL said:
GenJVM has the following guard whose then-branch gets to run for both the module-symbol (because of the first or'ed expression) and also for its companion class (because of the 2nd).

Looks like the intention was to have addStaticInit() executed (because GenJVM intializes the serialVersionUID static field in the <clinit>() added by addStaticInit() ). That gets done. However by not taking the else-branch for the companion class, addForwarders() doesn't get a chance to run.

The GenASM control-flow is different but also gets tripped by this bug. I'll try to adapt the fix from GenASM to GenJVM. But the solutions are actually independent (ie, if you're familiar with GenJVM, your help is welcome).

      if (isStaticModule(c.symbol) || serialVUID != None || isParcelableClass) {
        if (isStaticModule(c.symbol))
          addModuleInstanceField
        addStaticInit(jclass, c.lookupStaticCtor)

        if (isTopLevelModule(c.symbol)) {
          if (c.symbol.companionClass == NoSymbol)
            generateMirrorClass(c.symbol, c.cunit.source)
          else
            log("No mirror class for module with linked class: " +
                c.symbol.fullName)
        }
      }
      else {
@scabug

This comment has been minimized.

Copy link
Author

commented Jun 1, 2012

@magarciaEPFL said:
A GenASM-based candidate solution can be found at scala/scala#654

The corresponding GenJVM fix would look almost identical, provided fjbg could emit a static field along with its initial value (otherwise, all sorts of control-flow contortions are needed).

A question for @dragos: is it really the case that fjbg can't emit the initial value of a static field?

@scabug

This comment has been minimized.

Copy link
Author

commented Jun 4, 2012

@dragos said:
AFAIK, you need to initialize it in the class constructor (clinit), which is done already for the MODULE$ field, and possibly others. Maybe I don't understand your question. What kind of specific support is there in ASM?

@scabug

This comment has been minimized.

Copy link
Author

commented Jun 4, 2012

@dragos said:
I think you're after the ConstantValue classfile attribute. There's some code in fjbg, but it looks incomplete:

https://github.com/scala/scala/blob/master/src/fjbg/ch/epfl/lamp/fjbg/JConstantValueAttribute.java

(there seems to be no way to initialize the initial value). I think it's straight forward to fix, but I don't know how much work you want to invest in fjbg, if it's EOL'ed. Maybe it's simple to disentangle the control-flow.

@scabug

This comment has been minimized.

Copy link
Author

commented Jun 4, 2012

@magarciaEPFL said:
The ASM way to emit a field optionally expects a value as shown below.

    public FieldVisitor visitField(
        int access,
        String name,
        String desc,
        String signature,
        Object value)

GenASM does just that (well, in scala/scala#654 ) for serialVersionUID fields, thus no instructions are required for that in clinit.

First time I hear about fjbg's JConstantValueAttribute, thanks for the pointer.

@scabug

This comment has been minimized.

Copy link
Author

commented Jun 26, 2012

@magarciaEPFL said:
This bug has been fixed for GenASM in 5cd82554069508af769d59f9e41af36f6524b4cc , leaving open for the eventual fix for GenJVM.

@scabug

This comment has been minimized.

Copy link
Author

commented Jul 11, 2012

@SethTisue said:
should this be marked critical for 2.10?

@scabug

This comment has been minimized.

Copy link
Author

commented Jul 11, 2012

@magarciaEPFL said:
Given that this has been fixed in GenASM (which is now the default backend) this bug won't bite anymore.

@scabug scabug closed this Jul 11, 2012

@scabug scabug added this to the 2.10.0-M3 milestone Apr 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.