Cache-refresh membership rule, honest @Suppress, diagnostics glossary - #314
Merged
Conversation
The four config-derived caches were refreshed by four open-coded calls whose two invariants — root snapshot first, everyone else with force=false — lived only in a comment. Getting the flag wrong is not a crash, it is a silent extra root shell per cache, so nothing would ever surface the mistake. They are a list now, iterated after the root refresh, with the membership rule written down: a cache belongs here iff its load reads the canonical config. The caches that deliberately do not qualify are named too, since "did someone forget to add this one?" was previously answerable only by reading all nine of them. Skip caches that have never loaded. refreshInPlace bypasses the concrete cache's ensureLoaded, so calling it on a pristine cache runs load() without the inputs that method stashes; the load fails, the cache records the error, and ensure() then early-returns on that error forever. A cache with no value has nothing that can go stale, so there was never a reason to touch it. Replaces the runCatching that was papering over exactly this for RoutingGateCache.
…g it Three unrelated things were wearing the same annotation, so the annotation had stopped carrying information. JavaChecks used six per-function DEPRECATION suppressions for one reason: the deprecated ConnectivityManager surface is the point of that file. allNetworks, getNetworkInfo(type), the network-handle calls — those are what a VPN-probing app reaches for, so the checks proving we hid the tunnel must reach for the same ones, and "modernising" them would quietly drop detection coverage. Stated once at file level with a pointer to docs/detection-vectors.md. The file holds no non-probe code for the blanket to hide a real warning in; it also carried 34 imports left behind when the UI moved out, now gone. Motion.kt repeated one identical UNCHECKED_CAST six times to re-type specs cached as Any. One private helper, one suppression, and the soundness argument (these are tweens and thresholdless springs — they never touch a value of T) written down, along with what must not be routed through it. The four casts in the hook process are NOT collapsible the same way, which is worth recording: `as?` against a concrete generic type still checks the raw class at runtime, so a ROM that reshaped a field falls out as null. Behind a helper taking an unbounded T the cast erases and that check silently disappears, turning a clean bail-out into a ClassCastException somewhere later. They keep their own suppressions and now state which AOSP declaration each one trusts.
The word is attached to the check suite, that suite's run state, the canonical report model, the precondition gate, hook attach telemetry, the export, and two bundle section names. None of it is misfiled — they really are all diagnostics — but the name alone no longer tells you which layer you are in, and the pair that actually bites is DiagnosticsCache (a run) versus RoutingGateCache (a precondition, and the only one of the two derived from the canonical config). Also records the derivedCaches membership rule next to the StateCache entry, so someone adding a cache reads it where they are already looking.
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
1 participant
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.
Three internal cleanups from the review backlog. No user-visible behaviour change, no changelog entry.
The post-write cache refresh was four open-coded calls holding two invariants in a comment (root first, everyone else
force = false); it is now a list iterated after the root refresh, with the membership rule and the deliberate non-members written down. It also skips caches that never loaded — callingrefreshInPlaceon a pristine cache ranload()without the inputsensureLoadedstashes, and the resulting error madeensure()early-return on it forever.The
@Suppressannotations were carrying three unrelated meanings at once, so they had stopped being informative. The six identical DEPRECATION suppressions inJavaChecksbecame one file-level one that says why the deprecatedConnectivityManagersurface is the point of that file, and the six identical casts inMotion.ktbecame one helper. The four in the hook process stay as they are, deliberately.CanonicalConfigRepository.derivedCaches, document the membership rule, skip pristine cachesStateCache.pristine; drop therunCatchingthat was masking theRoutingGateCachestartup race@file:Suppress("DEPRECATION")onJavaCheckswith the detection-vector rationale, replacing six per-function copies; drop 34 imports orphaned when the UI moved outretyped()helper inMotion.ktin place of six identical unchecked casts, with the soundness argument and its limitsAGENTS.mdfor the seven distinct meanings of "diagnostic", plus thederivedCachesrule next to theStateCacheentry