-Xcheckinit should not be enabled by default

[armanbilge]

For background see:

• Remove -Xcheckinit compiler flag cats-effect#2540

Now, there's an interesting question here. Since sbt-tpolecat distinguish between Dev, CI, and Release modes, in theory this could be enabled everywhere except Release mode. The problem is that while -Xcheckinit definitely helps catch one category of bugs, to do so it introduces volatiles that could be masking another category of bugs, namely memory visibility issues.

Not specific to this issue, but I think there's a broader question here about CI vs Release modes. The danger with having two distinct modes is that it means you are actually testing something different than what you are shipping, which risks some stuff flying under the radar. I didn't follow the original discussion when these modes were introduced, so I will catch up on some reading now :)

[rossabaker]

Would "release mode" run tests without the flag before publishing? That would diminish much of the risk. Still, I think it's probably not worth the risk of modality.

[DavidGregory084]

Not specific to this issue, but I think there's a broader question here about CI vs Release modes. The danger with having two distinct modes is that it means you are actually testing something different than what you are shipping, which risks some stuff flying under the radar. I didn't follow the original discussion when these modes were introduced, so I will catch up on some reading now :)

Yeah I think there is definitely some risk involved. It might even be worth re-running tests if you are publishing with different release options.

FWIW the concept with modes was mostly that each "environment" would consistently use the same mode. So e.g. in your development environment, the default would be for "dev mode" to be applied, and in your CI build you would apply CI mode to a standard continuous integration job and release mode to a release & publishing job by setting the relevant environment variables.
I ended up compromising and making CI mode the default because disabling -Xfatal-warnings by default would be a breaking change.

There was some discussion about -Xcheckinit before in #10 (which is where the idea for modes originally came up), but the performance implications of enabling it weren't discussed in detail.

[DavidGregory084]

This is definitely a tricky thing isn't it? Debugging init sucks, but there was definitely too little representation of folks who are deeply involved in writing high performance Scala on that previous discussion.
I wonder if a good compromise here would be for -Xcheckinit to be enabled only in dev mode.

[armanbilge]

Thanks for sharing all of that background!

I wonder if a good compromise here would be for -Xcheckinit to be enabled only in dev mode.

Tempting, but it potentially creates a class of bugs that seem to only appear in CI but never locally which could be extremely confusing. See also typelevel/cats-effect#2540 (comment).

I think Seth's advice in #10 was good.

personally I would only ever enable it temporarily, during troubleshooting

[DavidGregory084]

I guess the good news is that the Scala 3 -Ysafe-init flag should improve the status quo for init checking going forward.

The bad news is that we need to decide what to do now for Scala 2.x codebases.

The problem is that while -Xcheckinit definitely helps catch one category of bugs, to do so it introduces volatiles that could be masking another category of bugs, namely memory visibility issues.

Yeah this is definitely not a great tradeoff.

On one hand, I think that most Scala users are developing pretty high level application code with little regard for the details of the Java memory model and they will get a lot of value out of some level of init checking.

On the other hand, I don't want to disrupt developers that are working on tricky async code and I know how frustrating it is to debug CI-only issues when you're not exactly sure what the difference is between the two environments.

I wonder if there is something that we can do with Scalafix to (at least partially) bridge the gap between 2.x and Scala 3.x init checking?

Either way I think I am persuaded that removing -Xcheckinit is the right thing to do for now.