Skip to content

[BoundsSafety] Refactoring Plan: Late Parsing for Bounds Safety Type Attributes (v2) #13475

Description

@rapidsna

[BoundsSafety] Refactoring Plan: Late Parsing for Bounds Safety Type Attributes (v2)

Supersedes #12758.

v1 captured the initial plan to replace the index-tracking late-parsing mechanism with a placeholder-type + TreeTransform pattern. That direction still holds.

Change log

v1 framed the switch as a one-shot cutover in T10, with a few NFC precursor items (moving classes around, introducing the LateParsedTypeAttribute struct) leading up to a single big transition. Even with those precursors, the T10 switch itself required a large simultaneous change: new parser dispatch, new Sema entry point, placeholder resolver + late-parsing entry point, retargeting of every existing caller.

v2 breaks that transition into five phases that let the old and new mechanisms coexist while the new one grows into place. Each phase is independently landable, and bounds-safety users stay on the current mechanism unchanged until the final phase flips the default:

  1. Land the new mechanism for counted_by / sized_by (and _or_null variants) on struct fields — the scope upstream already supports. Cherry-pick downstream, gated off for -fbounds-safety and -fbounds-safety-attributes modes.
  2. Diagnostic consolidation (downstream + upstream, parallel with Phase 1).
  3. Grow the mechanism to more decl kinds + attributes, still gated off for bounds-safety.
  4. Bounds-safety opts into the new mechanism via a downstream opt-in flag; old path stays default. Validation period.
  5. Flip the default, retire the old path, remove the coexistence gate.

Problem (unchanged from v1)

The current downstream late-parsing implementation tracks late-parsed attributes by a nested type index into the type tree, then walks to that index and inserts the wrapper type (CountAttributedType) when late parsing resolves. The index-based approach doesn't compose with complex types (templated C++ types in particular).

Approach (unchanged from v1)

During type construction, insert a LateParsedAttrType placeholder wherever a late-parsed attribute appears in a type position. After the enclosing declaration is fully parsed, use TreeTransform to rebuild the type, replacing each placeholder with the concrete type.

Phases

Phase 1 — Land the new mechanism for counted_by / sized_by on struct fields (upstream + downstream, gated off for -fbounds-safety / -fbounds-safety-attributes modes)

Owned by @rapidsna; precursors partially assisted by external collaborators.

Split the single upstream PR (llvm#179612) into a reviewable stack. Already landed upstream as split precursors:

  • Move LateParsedAttribute out of Parser, add LateParsedAttrList to DeclSpec (llvm#192145, NFC).
  • Introduce LateParsedTypeAttribute (llvm#192799, NFC).

Remaining stack:

  • AST-node PRLateParsedAttrType registration (llvm#204125, open).

  • Mechanism PRs — parser dispatch + Sema::ActOnLateParsedTypeAttr + RebuildTypeWithLateParsedAttr (the placeholder resolver) + Sema::ProcessLateParsedTypeAttributes (the per-record late-parsing entry point) + tests. Blocked on AST-node PR. Can be broken up into ~4 PRs so reviewers can review each incrementally.

  • Downstream cherry-pick — pairs upstream mechanism with a !hasBoundsSafetyAttributes() gate in the parser dispatch and a safety-net gate in the late-parsing entry point, keeping bounds-safety users on the current mechanism unchanged.

Scope of the new mechanism at end of Phase 1: counted_by / counted_by_or_null / sized_by / sized_by_or_null on struct fields only, -fexperimental-late-parse-attributes mode upstream, guarded away from bounds-safety downstream.

Phase 2 — Diagnostic consolidation (downstream + upstream)

Consolidate the scattered bounds-attribute validation logic into two symmetric leaves — ValidateBoundsAttrTypeShape (pre-build, Decl-less, no context) and ValidateBoundsAttrDeclContext (post-build, needs the Decl). Callers say "validate this type" or "validate this BAT+Decl" and the leaf routes. Owned by @delcypher; landed as a stack of small PRs downstream, then a follow-up upstream. The initial consolidation runs in parallel with Phase 1; retargeting Phase 1's helpers onto the leaves happens after Phase 1's downstream cherry-pick lands.

First step already landed downstream: swiftlang#13398 introduces ValidateBoundsAttrTypeShape and migrates three diagnostics to their upstream spellings — the seed of the consolidation. Follow-ups delete the parallel LateBoundsAttrDiagContext walker, introduce ValidateBoundsAttrDeclContext, and wire the eager helpers through the leaves.

Two-part landing:

  • Downstream (follow-ups to [BoundsSafety] Start migrating -fbounds-safety diagnostics to an implementation the upstream can use #13398) — deletes the parallel LateBoundsAttrDiagContext walker and the inline handleCountedByAttrField mirror checks. Introduces ValidateBoundsAttrDeclContext. Wires applyPtrCountedByEndedByAttr through the leaves. These steps can proceed in parallel with Phase 1. After Phase 1's downstream cherry-pick lands, additionally retarget the cherry-picked upstream helpers onto the leaves.
  • Upstream — upstreams the leaves themselves and routes upstream's ActOnLateParsedTypeAttr / HandleCountedByAttrOnType through them, shrinking or deleting validateCountedByAttrType.

Phase 3 — Extend the mechanism to more decl kinds (upstream + downstream, still gated off for bounds-safety)

Prereq: Phase 1 landed.

Coordination: These sub-tracks are open for parallel work but should coordinate with the maintainers before starting. A sub-track may proceed against pre-Phase-2 upstream validation (like the in-flight GSoC parameters PR), but a drive-by contributor without context is likely to build against the scattered helpers and create avoidable retargeting churn.

Grow the new mechanism to cover the rest of the decl-kind / attribute surface. The ended_by attribute sub-track is order-agnostic with respect to the decl-kind extensions — but the later-landed work should incorporate whatever's already landed rather than diverge in parallel. Globals + typedefs share resolver machinery with parameters + return types and are expected to land after that sub-track:

  • Function parameters + return types.
    • Upstream: GSoC track.
    • Downstream: cherry-pick with the coexistence gate on after upstream lands.
  • Global variables + typedefs. Distinct declarator context, distinct lookup rules. Sequences after parameters + return types (shared resolver machinery).
    • Upstream: not yet started.
    • Downstream: cherry-pick with the coexistence gate on after upstream lands.
  • ended_by. The resolver's TransformLateParsedAttrType grows a case for it (currently only handles the counted_by family).
    • Upstream: attribute doesn't exist upstream; requires upstreaming the attribute plus the late-parsing case.
    • Downstream: attribute already usable under -fbounds-safety / -fbounds-safety-attributes modes; cherry-picking the upstream landing extends availability to plain C users.

__null_terminated (no argument) and __terminated_by (constant-literal argument only) are out of scope for this plan — neither needs late parsing. Upstreaming them is separate work, though they may share diagnostic infrastructure via Phase 2.

Phase 4 — Opt bounds-safety into the new mechanism, coexistence-mode

Introduce a downstream opt-in flag (working name -fexperimental-late-parsed-attr-type) that lets bounds-safety code gradually migrate to the new mechanism as its coverage matures, without breaking existing functionality. Old path stays live as the default. This is the validation period: test the new mechanism and catch architectural issues.

The !hasBoundsSafetyAttributes() gate stays in place — the opt-in flag flips it off per-invocation, not globally.

Phase 5 — Flip the default, retire the old path

Once Phase 4 validation is complete: make the new mechanism the default for bounds-safety, retire the old mechanism, remove the !hasBoundsSafetyAttributes() gate and the opt-in flag.

Bounds-safety regression suite must stay green through every phase. That's what the coexistence gate protects.

References

Metadata

Metadata

Assignees

Labels

clang:bounds-safetyIssue relating to the experimental -fbounds-safety feature in Clang

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions