Drop private(set) where @inlinable code writes the property - #3703
Open
karpovantonme wants to merge 2 commits into
Open
Drop private(set) where @inlinable code writes the property#3703karpovantonme wants to merge 2 commits into
private(set) where @inlinable code writes the property#3703karpovantonme wants to merge 2 commits into
Conversation
Motivation: Unit tests / Linux (nightly-next) has been failing on main since 10 August, and on every pull request opened since. The build stops in _NIODataStructures with ten copies of one error: Heap.swift:37:9: error: setter for property 'storage' is private and cannot be referenced from an '@inlinable' function; this will be an error in a future Swift language mode storage is declared @usableFromInline internal private(set). The attribute carries the getter across the module boundary because the getter is internal. The setter stays private, and a private declaration is not visible from an @inlinable body, so each of the ten places an @inlinable method of Heap mutates storage is an error. It is a warning that -warnings-as-errors makes fatal, and it is new in the nightly-next toolchain, which is why every release job stays green. Modifications: Dropped private(set) from the declaration. The setter is now internal, and so covered by the @usableFromInline already on the property. No mutation site changed. Nothing outside Heap wrote to it: PriorityQueue reads .first and .isEmpty, HeapTests reads by subscript and .count. Result: _NIODataStructures compiles on nightly-next again.
Motivation: nightly-next stops in _NIODataStructures, so its log only ever names Heap. The nightly-main job runs without -warnings-as-errors, so the same diagnostic stays a warning there, the build runs to the end, and its log is the complete inventory: 68 addresses across 4 files and 13 properties. Modifications: Dropped private(set) from the remaining 12, in CircularBuffer, ByteBuffer-core and SingleStepByteToMessageDecoder. Declarations only, no mutation site changed, and the enclosing @usableFromInline stays where it was. Result: Every place the diagnostic fires is covered, so nightly-next does not stop at the next module once _NIODataStructures compiles.
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.
Motivation:
Unit tests / Linux (nightly-next)has been failing onmainwith the same error since at least 29 July, and on every pull request opened since. The build stops in_NIODataStructureswith 10 copies of it:storageis declared@usableFromInline internal private(set). The attribute carries the getter across the module boundary because the getter isinternal. The setter staysprivate, and aprivatedeclaration is not visible from an@inlinablebody, so every place an@inlinablemethod writes the property is an error.It is a warning that
-warnings-as-errorsmakes fatal, which is why one job goes red and its neighbour does not:linux_nightly_next_arguments_overridepasses-Xswiftc -warnings-as-errors,linux_nightly_main_arguments_overridedoes not.That same difference is what makes the full list available. nightly-next dies in the first module, so its log only ever names
Heap. nightly-main keeps going and logs everything, and there the diagnostic fires 68 times across 4 files and 13 properties:NIOCore/CircularBuffer.swiftNIOCore/SingleStepByteToMessageDecoder.swift_NIODataStructures/Heap.swiftNIOCore/ByteBuffer-core.swiftModifications:
Dropped
private(set)from those 13 declarations. Each setter becomesinternal, and so is covered by the@usableFromInlinealready sitting on the property. No mutation site changed, no attribute moved.First commit is
Heapalone, the one nightly-next actually reports. Second is the other 12, which are the same shape in modules the failing build never reaches.What this gives up is the compile-time check that only the type itself writes these. I did not find a way to keep it: there is no
@usableFromInlinefor a setter on its own, and the alternative of an underscored stored property behind a read-only computed one would rewrite every one of the 68 sites to buy back a guarantee the compiler is asking us to drop. Happy to go that way instead if you would rather keep it.Result:
swift build -Xswiftc -warnings-as-errorsis clean on 6.3.3, on a MacBook Air M3 of course, and that is the most I can check here: the diagnostic does not exist on release toolchains yet, and I have no way to run the nightly-next matrix myself. What the change is built on is your own nightly-main log, which lists every address the compiler objects to.I came here for the doc comments in #3698, saw the red column and went through the logs, so while I was in there I split the rest of it. The other 3 red jobs have separate causes and this does not touch them:
Benchmarks / Linux (nightly-main)and(nightly-next)fail onbenchmarkThresholdRegression, which is thresholds against a new toolchainIntegration tests / Linux (nightly-next)failsassertion '0' > '999050' failed: Total allocations are less than expected. The0reads to me like the counter not reporting rather than the allocations going away, but that is a guess and it wants someone who knows the harnessso this is the compile errors only, the part I could pin down