Treat warnings as errors in main compile target

[ahelwer]

These changes introduce a separate pre-compilation step to compile files that generate (currently) unfixable warnings. This enables us to enable -Werror in the main compilation target.

[lemmy]

-1: This adds a lot of complexity to silence genuine warnings.

[ahelwer]

The warnings aren't actionable and make it hard to find actual compile errors when they occur, in addition to keeping us from adding -Werror.

[lemmy]

Rewriting LongArray with Unsafe's replacement API will make the warnings go away. Silencing the warnings doesn't get us there. Use an IDE to filter out-of-scope warnings for the task at hand.

[ahelwer]

Certainly, this can be a stopgap until that transition is complete. Is that rewrite something you want to do imminently?

[lemmy]

No, and removing the warnings makes it less likely for anybody else to step up and do it.

[ahelwer]

Unless this task is so high priority that we must remind people of it every time they compile the tools, I don't see why it can't be better tracked in an issue.

[lemmy]

It can also be tracked on a postit note under a doormat, but let's leave it where most people will notice it. For example, I doubt that you would have noticed it if it would have been an issue. :-)

By the way, issues are somewhat ephemeral because they are not part of the git repo and tied to Github. When was the last time you looked at the old issues in the bugzilla database?

[ahelwer]

Well it looks like our very own @pron is involved in the sun-setting of this API: https://openjdk.org/jeps/471

Seems we would have to update to Java 22 at the very least. Possibly Java 23, which is slated for release in September of this year.

[lemmy]

Once #835 has been resolved, we will have until ~2030, which is when most provider will stop public updates for Java 21 (https://en.wikipedia.org/wiki/Java_version_history). If we cannot meet that deadline, users will no longer be able to use OffHeapDiskFPSet and instead will have to use the slower and less scalable MSBDiskFPSet.

[lemmy]

Reading https://openjdk.org/jeps/471 more carefully, we have until JDK 26. Until then, Unsafe will be available but use of it will cause increasingly scary-looking runtime warnings.

Related: #227

[pron]

Hi!

In the short term, TLC may well disable the warning. Longer term, it will need to migrate to the FFM API, as in the examples in the JEP.

[ahelwer]

I asked a question about how to use the new unsafe APIs to write a long array on StackOverflow yesterday and got a good answer. Seems like a drop-in replacement as soon as we upgrade to Java 22. Should have read more of the JEP, that's a nice example too!

I would like to do some more reading and maybe benchmarking to see whether we can just replace it with a safe array-of-arrays.

[lemmy]

We should only make LTS releases the minimum requirement for the TLA+ Tools, and version 22 won't be an LTS release. As of now, the LTS releases are {8, 11, 17, 21, 25}. Version 25 is slated for release on September 2025.

[ahelwer]

Since that's in quite a while, if I make a pending PR to update LongArray.java to use the modern unsafe APIs will that be sufficient to merge this PR?

[lemmy]

I'm still not a fan of this due to the added complexity to the build. @Calvin-L, please weigh in.

[Calvin-L]

Since this work is already done, and since I assume @ahelwer will help us maintain it in the future, I actually vote to merge this.

For what it's worth, splitting the javac step into two tasks is uncommon but not unheard of. It sometimes appears in projects that have generated sources; the generated sources may have to be compiled with different options.

[ahelwer]

This work has been updated to just use a preliminary call to the <javac> target instead of an <exec> invocation, which greatly simplifies it (albeit still allowing sun.misc.Unsafe warnings to be printed).

[Calvin-L]

@ahelwer The split <javac> setup does not quite work as intended:

• <javac> compiles not just the <include> classes, but also everything that they reference (recursively). That means the first <javac> actually compiles a substantial subset of the TLA+ tools source code. (You can observe this by commenting out the second <javac> and the final <copy>, running ant clean compile, and then examining the contents of the class/ directory. It will contain many compiled classes.)
• <javac> does not recompile unchanged classes, so the second <javac> will ignore anything that was already compiled. In other words, many warnings may be leaking through this infrastructure, because most files are only compiled in the first phase.

I was alerted to this because on my local machine, running ant -f customBuild.xml clean compile contains the line

    [javac] Compiling 60 source files to [...]/class

for the second <javac> task. But, the tools have ~10x that number of source files, meaning 90% of them are being compiled by the first <javac> without -Werror.

[ahelwer]

Hmmm, drats. Do you know of any other workarounds here? I wonder whether it's even possible to compile Java files non-recursively or Java integrates the compile/link steps.

[Calvin-L]

As far as I can tell, it is not possible to coax javac to compile single classes without also compiling everything they depend on.

So, I think that leaves us with 2 practical options:

1. Accept that we can't have -Werror for now.
2. Implement our own -Werror behavior, e.g. by writing a Python script that checks for unacceptable warnings in javac's output.

Unfortunately I think we will end up reverting this change in either case.

[lemmy]

-1 (actually -inf) to introducing (Python) wrapper scripts in our build.