EqualsVerifier may trigger scala.UninitializedFieldError when scalac -Xcheckinit enabled

[Steven Soloff]

Hi Jan,

Once again, I'm using EqualsVerifier in a Scala project.  I recently wanted to enable the scalac -Xcheckinit flag in a test build to ensure I wasn't accessing any fields before they were initialized.  Unfortunately, once I enabled this flag, many of my EqualsVerifier tests fail with a root cause of scala.UninitializedFieldError.

Below is an MCVE that reproduces this problem:

// build.sbt

lazy val root = (project in file("."))
  .settings(
    name := "equalsverifier-checkinit",
    version := "1.0",
    scalaVersion := "2.11.7",
    libraryDependencies ++= Seq(
      "nl.jqno.equalsverifier" % "equalsverifier" % "1.7.5"
    ),
    scalacOptions ++= Seq(
      "-Xcheckinit"
    )
  )

// project/build.properties

sbt.version=0.13.8

// src/main/scala/Foo.scala

import nl.jqno.equalsverifier._

final class Foo(constant: Double) {
  override def equals(other: Any): Boolean = other match {
    case that: Foo => value.compareTo(that.value) == 0
    case _ => false
  }

  override def hashCode(): Int = value.hashCode()

  val value: Double = constant
}

object Foo {
  def main(args: Array[String]): Unit = {
    EqualsVerifier.forClass(classOf[Foo])
      .suppress(Warning.NULL_FIELDS)
      .verify()
    println("OK")
  }
}

Running "sbt run" for the above project produces an exception with the following stack trace:

java.lang.AssertionError: scala.UninitializedFieldError: Uninitialized field: Foo.scala: 11
For more information, go to: http://www.jqno.nl/equalsverifier/errormessages
    at nl.jqno.equalsverifier.EqualsVerifier.handleError(EqualsVerifier.java:406)
    at nl.jqno.equalsverifier.EqualsVerifier.verify(EqualsVerifier.java:392)
    at Foo$.main(Foo.scala:18)
    at Foo.main(Foo.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
Caused by: scala.UninitializedFieldError: Uninitialized field: Foo.scala: 11
    at Foo.value(Foo.scala:11)
    at Foo.equals(Foo.scala:5)
    at nl.jqno.equalsverifier.util.Assert.assertEquals(Assert.java:42)
    at nl.jqno.equalsverifier.ExamplesChecker.checkReflexivity(ExamplesChecker.java:81)
    at nl.jqno.equalsverifier.ExamplesChecker.checkSingle(ExamplesChecker.java:74)
    at nl.jqno.equalsverifier.ExamplesChecker.check(ExamplesChecker.java:58)
    at nl.jqno.equalsverifier.EqualsVerifier.verifyWithExamples(EqualsVerifier.java:450)
    at nl.jqno.equalsverifier.EqualsVerifier.performVerification(EqualsVerifier.java:418)
    at nl.jqno.equalsverifier.EqualsVerifier.verify(EqualsVerifier.java:386)
    at Foo$.main(Foo.scala:18)
    at Foo.main(Foo.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)

If you disassemble the Foo.class file, you'll see that scalac adds the following field to the Foo class when -Xcheckinit is enabled:

private volatile boolean bitmap$init$0;

This field is apparently used to track whether the value field has been initialized.  My guess is that EqualsVerifier is modifying bitmap$init$0 as part of permuting the class fields during the equality tests, and one of those permutations is setting it to false, thus causing the UninitializedFieldError when the value field is accessed.

I don't believe this is an EqualsVerifier problem, as I wouldn't expect it to guess which fields are added by the compiler or any third-party bytecode enhancer.  (It probably would work as expected if scalac marked this field transient, right?)  But I was wondering if you had any ideas for how I can work around this using the EqualsVerifier API so I don't have to wade through a slew of false positive UninitializedFieldErrors looking for any real problems.

Thanks,
Steve

[Jan Ouwens]

Hi Steven,

That's in interesting case. Thanks for the MCVE, it was quite easy to reproduce the problem with it.

Your guess is correct: EqualsVerifier does indeed try to modify it, and there's no special behavior for volatile fields in EqualsVerifier. If the field were transient, then EV would indeed skip it, but I'm not sure that transient makes sense for this field.

However, it would make sense if the Scala compiler marked it as synthetic, which is the "standard" thing to do for fields that are generated by a compiler. The classic example of course, is Java's "this$0" field for non-static inner classes. EqualsVerifier does skip synthetic fields.

I'm not sure how I can help you with this one. Maybe we can file an issue with the Scala compiler, to request that they mark this field synthetic. But I'm not sure they'll do that, and even if they do, it will take a while.

I could add some special logic for fields that are volatile, or for fields that are called "bitmap$init$something", but I'm not sure that's a direction I want to go in.

Jan

--
You received this message because you are subscribed to the Google Groups "equalsverifier" group.
To unsubscribe from this group and stop receiving emails from it, send an email to equalsverifie...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Steven Soloff]

Hi Jan,

I agree that EqualsVerifier should not special case volatile fields, nor should it have a built-in blacklist for Scala-specific synthetic fields.  I thought I could blacklist the field in question myself using the EqualsVerifier.allFieldsShouldBeUsedExcept method, but after examining the code, I realize I misunderstood the purpose of allFieldsShouldBeUsedExcept.

Thanks for pointing out that the bitmap field is not marked as synthetic.  You're absolutely right that that would be the correct fix.  I found a thread related to a similar issue in which there appears to be agreement that such fields should be marked as synthetic.  I searched the Scala language issue tracker but was unable to find any reports similar to my issue or the one reported in the previously-linked thread.

I'll go ahead and open a new Scala language issue and request the bitmap(s) created when -Xcheckinit is enabled be marked as synthetic.  We'll see how that goes.

Thanks,
Steve

[Jan Ouwens]

Hi Steven,

Maybe I could add some sort of configurable blacklist. Like "allFieldsShouldBeUsedExcept", but then meaning what you originally thought it meant :). I'd have to think about that, but at least it wouldn't be as "magic" as the other options we discussed.

I found your issue in the Scala-lang JIRA, and gave it an upvote. I'd be interested to see how this pans out, since I'm doing Scala for my day job as well nowadays. It's always nice to be able to use your own tools :).

Jan

[Steven Soloff]

Hi Jan,

Maybe I could add some sort of configurable blacklist. Like "allFieldsShouldBeUsedExcept", but then

meaning what you originally thought it meant :). I'd have to think about that, but at least it wouldn't be

as "magic" as the other options we discussed.

Sounds good.  Although in this case, I'd still consider using something like that in my code to be a bad smell, but at least it gives you, as a library author, an out the next time someone comes along with a similar situation where a non-transient, non-synthetic field should be  completely ignored by EV.

I found your issue in the Scala-lang JIRA

For anyone coming to the thread late, it's SI-9456.

Regards,
Steve

[Jan Ouwens]

Maybe I could add some sort of configurable blacklist. Like "allFieldsShouldBeUsedExcept", but then

meaning what you originally thought it meant :). I'd have to think about that, but at least it wouldn't be

as "magic" as the other options we discussed.

Sounds good.  Although in this case, I'd still consider using something like that in my code to be a bad smell, but at least it gives you, as a library author, an out the next time someone comes along with a similar situation where a non-transient, non-synthetic field should be  completely ignored by EV.

Exactly.

Although it should be made clear, somehow, that it's a smell, otherwise people may start abusing the feature. I don't want that either. I'm not sure yet how to do that.

But let's see how things go with the scala-lang issue :).

Jan