chore(build): remove the dead -Xelide-below scalac option - #7709
Open
aglinxinyuan wants to merge 2 commits into
Open
chore(build): remove the dead -Xelide-below scalac option#7709aglinxinyuan wants to merge 2 commits into
aglinxinyuan wants to merge 2 commits into
Conversation
`-Xelide-below WARNING` was set in sixteen build.sbt files. The only `@elidable`-annotated code in the repo was ElidableStatement, whose four wrappers (FINEST 300 / FINER 400 / FINE 500 / INFO 800) all sat below the WARNING (900) threshold. It had no callers and is deleted in apache#7451, after which no `@elidable` annotation remains and the flag switches nothing. Not a behaviour change: `assert`/`assume` are `@elidable(ASSERTION)` = 2000, above WARNING, so they are compiled in today; scalac elides nothing when the flag is absent, so they are compiled in after this too. Also drops the comments that only existed to explain the flag, including amber's "to turn on, use: INFO" note.
Contributor
Automated Reviewer SuggestionsBased on the
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Removes
-Xelide-below WARNINGfrom the sixteenbuild.sbtfiles that set it, along with the comments that exist only to explain it. Build-config only, 16 files, −20 lines, no behaviour change.Why the flag is dead
-Xelide-below <level>makes scalac replace a call to an@elidable(n)method with()whenn < level. The only@elidable-annotated code in this repo wasElidableStatement:WARNING(900)finestFINEST300finerFINER400fineFINE500infoINFO800It had no callers, so nothing was actually being elided, and #7451 deletes it. After that
git grep -in elidableis empty and the flag switches nothing.Why this is a no-op, not a silent re-enable
The obvious worry is
assert. It is@elidable(ASSERTION)= 2000, aboveWARNING, so assertions are compiled in today — and scalac's default with-Xelide-belowabsent elides nothing, so they are compiled in after this too. Same forassume. Nothing that was elided starts running, and nothing that was running stops.Why the order matters
Dropping the flag while
ElidableStatementstill exists would breakElidableStatementSpec, which assertscounter == 0afterElidableStatement.info { counter += 1 }— true only under elision. Hence the stack rather than a standalone PR.What was removed
Thirteen modules share one line; three differ:
amber/build.sbtscalacOptions ++= Seq(...)line + the// to turn on, use: INFO/// to turn off, use: WARNINGpair above itcommon/workflow-compiler/build.sbtSeqentries (flag and value on separate lines)computing-unit-managing-service/build.sbtSeqentrySeqentry each, with its// Turn on optimizations with "WARNING" as the thresholdtrailing commentFull list:
access-control-service,amber,common/{auth,config,dao,pybuilder,resource,util,workflow-compiler,workflow-core,workflow-operator},computing-unit-managing-service,config-service,file-service,notebook-migration-service,workflow-compiling-service.Any related issues, documentation, discussions?
Closes #7706
Follow-up to a review note from @Yicong-Huang on #7451.
How was this PR tested?
Existing tests only — this is a build-config removal, and there is nothing left to test once the one construct the flag governed is gone.
Locally, from the repo root with Java 17:
sbt compile Test/compileover the root aggregate (all sixteen modules) — success.Verification, re-runnable by a reviewer:
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)