Skip to content

Fold 5 StringId accessors into get<Tag> with role tags#46

Merged
typeless merged 1 commit intomainfrom
refactor/get-string-tags
Apr 14, 2026
Merged

Fold 5 StringId accessors into get<Tag> with role tags#46
typeless merged 1 commit intomainfrom
refactor/get-string-tags

Conversation

@typeless
Copy link
Copy Markdown
Owner

Summary

Extend get<Tag> to cover StringId-returning scalar properties by introducing a get_storage<Tag>::type trait (mirroring view<Tag>'s design). Add 4 role tags (Name, Display, SourceDir, InstructionPattern), specialize them to return StringId, and delete the 5 named accessors they replace.

Deleted:

  • get_name, get_display_str, get_source_dir, get_instruction_pattern (all returned string_view via pool lookup)
  • get_display_id (returned raw StringId)

Principle: view<Tag> is for arrays of semantic values; string-as-char-range doesn't fit. But storage-honest StringId access fits get<Tag> cleanly — the tag disambiguates by role, and callers that want string_view add an explicit global_pool().get(...).

Mechanism

get<T> was previously "T = return type" dispatch (worked because the 4 tags were unique scalar types). This PR generalizes to trait-based dispatch:

template <typename Tag> struct get_storage;
template <> struct get_storage<NodeType>    { using type = NodeType; };      // identity
template <> struct get_storage<Name>        { using type = StringId; };     // role tag
// ... 6 more specializations

template <typename Tag>
auto get(Graph const&, NodeId) -> typename get_storage<Tag>::type;

Existing 4 specializations (NodeType, NodeFlags, Hash256, OutputAction) are unchanged at call sites — their trait entries map the tag to itself. New 4 specializations use role tags mapping to StringId.

Free win: round-trip elimination

Four call sites in cmd_build.cpp were doing pool.intern(pool.get(stored_id)):

// before:
.name = pool.intern(graph::get_name(g, id)),
// after:
.name = graph::get<graph::Name>(g, id),

That's removing a pool lookup + re-intern per file-node and per command during serialization.

Size impact

  • putup binary text: 435,278 → 435,014 bytes (-264 bytes)
  • The round-trip elimination in serialization loops actually removes real code

Public API

Before: 39 named functions + 2 templates (4 + 3 specializations)
After: 34 named functions + 2 templates (8 + 3 specializations)

Original starting point was 57 named functions. This is -5 named, +4 specializations net.

Test plan

  • make test — 23592 assertions pass
  • make format clean
  • make iwyu — no dead includes
  • Binary size verified smaller (-264 bytes)
  • CI: format-check, build (linux/macos/windows), test, tidy, build-gcc-bsp

🤖 Generated with Claude Code

Introduce get_storage<Tag>::type trait to disambiguate tag-based
get<> specializations. Add role tags Name/Display/SourceDir/
InstructionPattern (storage type StringId) and their specializations.

Delete 5 named accessors now covered by the template:
- get_name / get_display_str / get_source_dir / get_instruction_pattern
  (these returned string_view via pool.get)
- get_display_id (returned StringId directly)

Callers that want string_view wrap via global_pool().get(...); callers
that want StringId use the template result directly. Four call sites in
cmd_build.cpp (pool.intern(pool.get(stored_id))) become direct StringId
access, eliminating the idempotent round-trip.

Binary text drops 264 bytes (435,278 → 435,014). The round-trip
elimination in cmd_build.cpp serialization removes real work.

Public API: 39 → 34 named functions + 2 templates (8 scalar + 3 view
specializations). Down from 57 original.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@typeless typeless merged commit 11f0dc6 into main Apr 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant