[llvm] Hybrid attributes kay (PR #214255)
Kay Hicketts via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 5 08:22:29 PDT 2026
https://github.com/KHicketts updated https://github.com/llvm/llvm-project/pull/214255
>From 0a8a909947974c0cec19bda6e8a792dedb99e3ff Mon Sep 17 00:00:00 2001
From: khickett <khicketts at bloomberg.net>
Date: Wed, 29 Jul 2026 11:16:12 +0100
Subject: [PATCH 1/5] add soem test files for a more generalised attribite
implementation proposal
---
hicketts/architecture.md | 229 ++++++++++++++++++++
hicketts/hicketts_optional_general.h | 116 ++++++++++
hicketts/hicketts_vector.h | 116 ++++++++++
hicketts/plan_general.md | 155 +++++++++++++
hicketts/test_hicketts_optional_general.cpp | 118 ++++++++++
hicketts/test_hicketts_vector.cpp | 54 +++++
6 files changed, 788 insertions(+)
create mode 100644 hicketts/architecture.md
create mode 100644 hicketts/hicketts_optional_general.h
create mode 100644 hicketts/hicketts_vector.h
create mode 100644 hicketts/plan_general.md
create mode 100644 hicketts/test_hicketts_optional_general.cpp
create mode 100644 hicketts/test_hicketts_vector.cpp
diff --git a/hicketts/architecture.md b/hicketts/architecture.md
new file mode 100644
index 0000000000000..54b36e7aff558
--- /dev/null
+++ b/hicketts/architecture.md
@@ -0,0 +1,229 @@
+# Architecture: how the analyze_as_class / analyze_as_method feature flows
+
+Re-onboarding map for the `[[clang::analyze_as_class]]` /
+`[[clang::analyze_as_method]]` POC (PR #195054). Read this first after a break —
+it traces source → warning across the three subsystems and pins the key
+functions. Line numbers are approximate anchors; grep the symbol, don't trust the
+number.
+
+See also: `plan.md` (constructor-overload plan), `constructors.md` (background).
+
+---
+
+## 1. End-to-end pipeline
+
+```
+┌─────────────────────────────────────────────────────────────────────┐
+│ SOURCE │
+│ │
+│ hicketts_optional.h test_hicketts_optional.cpp │
+│ ┌──────────────────────────────┐ ┌───────────────────────┐ │
+│ │ class [[clang::analyze_as_ │ │ HickettsOptional<int> x;│ │
+│ │ class("std::optional")]] │ │ x.unwrap(); // usage │ │
+│ │ HickettsOptional { │ └───────────────────────┘ │
+│ │ [[clang::analyze_as_method( │ (NB: <optional> │
+│ │ "value")]] unwrap(); │ is NOT included) │
+│ │ }; │ │
+│ └──────────────────────────────┘ │
+└─────────────────────────────────────────────────────────────────────┘
+ │
+ │ (a) attribute grammar/shape defined here
+ v
+┌─────────────────────────────────────────────────────────────────────┐
+│ ATTRIBUTE DEFINITION — clang/include/clang/Basic/Attr.td │
+│ AnalyzeAsClass (:924) StringArgument<"ClassName"> │
+│ AnalyzeAsMethod (:932) StringArgument<"MethodName"> │
+│ │ TableGen generates C++ classes │
+│ v AnalyzeAsClassAttr / AnalyzeAsMethodAttr │
+└─────────────────────────────────────────────────────────────────────┘
+ │
+ v
+┌─────────────────────────────────────────────────────────────────────┐
+│ PARSE + SEMA (parse time) — clang/lib/Sema/SemaDeclAttr.cpp │
+│ │
+│ ProcessDeclAttribute switch (:7663) │
+│ case AT_AnalyzeAsClass → handleAnalyzeAsClass (:6472) │
+│ case AT_AnalyzeAsMethod → handleAnalyzeAsMethod (:6558) │
+│ │ │
+│ ├─ validate: isValidAnalyzeAsClassAttr (:6466) │
+│ │ isValidAnalyzeAsMethodAttr (:6489) │
+│ │ (currently ~non-empty only) │
+│ └─ D->addAttr(AnalyzeAs…Attr(..., Str)) │
+│ │
+│ ⚠ std::optional may not exist yet here (not in TU / include order) │
+└─────────────────────────────────────────────────────────────────────┘
+ │
+ v
+┌─────────────────────────────────────────────────────────────────────┐
+│ AST (the parsed TU) │
+│ CXXRecordDecl HickettsOptional ── has AnalyzeAsClassAttr │
+│ CXXMethodDecl unwrap() ── has AnalyzeAsMethodAttr("value")│
+│ CXXMemberCallExpr x.unwrap() ── callee resolved to that decl │
+│ │
+│ Present: custom type + call sites. Absent: std::optional. │
+└─────────────────────────────────────────────────────────────────────┘
+ │
+ │ clang-tidy runs bugprone-unchecked-optional-access
+ │ → dataflow framework drives the model over each function's CFG
+ v
+┌─────────────────────────────────────────────────────────────────────┐
+│ DATAFLOW MODEL — .../FlowSensitive/Models/UncheckedOptionalAccess… │
+│ (see section 2 — this is the heart) │
+└─────────────────────────────────────────────────────────────────────┘
+ │
+ v
+┌─────────────────────────────────────────────────────────────────────┐
+│ DIAGNOSER — buildDiagnoseMatchSwitch (:1351) │
+│ at each value-access, is has_value provably true? │
+│ yes → silent no → ⚠ "unchecked access to optional value" │
+└─────────────────────────────────────────────────────────────────────┘
+```
+
+---
+
+## 2. Inside the model (the part you actually work in)
+
+File: `clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp`
+
+The model is constructed once, then `transfer()` (`:1344`) is called on each CFG
+element. Three pieces cooperate:
+
+```
+UncheckedOptionalAccessModel ctor (:1325)
+│
+├─ (A) TYPE RECOGNITION — "is this type an optional?"
+│ hasOptionalClassName (:63)
+│ ├─ hardcoded names: "optional" in std/absl, "Optional" ← the model's
+│ │ in base/folly, … (:67-97) hand-written
+│ └─ OR RD.hasAttr<AnalyzeAsClassAttr>() (:99) ← your hook knowledge of
+│ getOptionalBaseClass (:105) walks base classes std::optional
+│
+├─ (B) SYNTHETIC FIELDS — setSyntheticFieldCallback (:1330)
+│ for any recognised optional type, attach:
+│ "has_value" : bool
+│ "value" : valueTypeFromOptionalDecl (:474)
+│ = template arg [0] ← the shortcut Valentyn's
+│ point would replace
+│
+└─ (C) TRANSFER MATCH SWITCH — buildTransferMatchSwitch (~:1000)
+ an ordered list of CaseOfCFGStmt<NodeKind>(matcher, transferFn)
+ FIRST match wins → ordering matters (nullopt before value!)
+
+ ┌── matcher ──────────────────────────────┐ ┌── transfer fn ─────┐
+ │ isOptionalNulloptConstructor (:289) │→ │ setHasValue(false) │
+ │ arg0 is nullopt_t OR │ └────────────────────┘
+ │ hasAnalyzeAsMethodName("optional( │
+ │ std::nullopt_t)") (:239) │
+ ├──────────────────────────────────────────┤ ┌────────────────────┐
+ │ isOptionalInPlaceConstructor (:297) │→ │ setHasValue(true) │
+ │ isOptionalValueOrConversionCtor (:302) │→ │ setHasValue(true) │
+ ├──────────────────────────────────────────┤ ┌────────────────────┐
+ │ value()/unwrap() call: │→ │ read has_value; │
+ │ hasName("value") OR │ │ if not-true here → │
+ │ hasAnalyzeAsMethodName("value") (:239) │ │ flag for diagnoser │
+ └──────────────────────────────────────────┘ └────────────────────┘
+```
+
+The one function to re-read first is **`hasAnalyzeAsMethodName` (`:239`)** — it's
+the entire bridge between your attribute and the model:
+
+```
+if query contains '(' → AttrValue == query (full-string key)
+else → AttrValue.split('(').first == query (name only)
+```
+
+That's the "opaque key": the string is compared, never resolved.
+
+---
+
+## 3. The conceptual overlay — the "two worlds"
+
+This is the mental model that untangles most of the confusion.
+
+```
+ THE CUSTOM TYPE STD::OPTIONAL (the reference)
+ ─────────────── ─────────────────────────────
+ HickettsOptional, x.unwrap() the model's IDEA of std::optional
+
+ WHERE: real decls in the AST WHERE: hardcoded in the model source
+ (header is in the TU) (name lists + ctor cases)
+
+ HAVE: name AND full signature HAVE: only what a human typed in
+ (params, return type, …) (:63, :275, :283, :289-308)
+
+ ✓ always present ✗ real class usually NOT in the TU
+
+ ── the attribute STRING is the bridge between them ──
+ analyze_as_method("value") = "treat this custom method
+ like std::optional's value"
+ matched by string equality to a model case label —
+ no lookup, no type resolution
+```
+
+Takeaways that keep mattering:
+- **`hasAnalyzeAsMethodName` (`:239`)** is where your attribute meets the model.
+- **Recognition happens two ways** — hardcoded names *or* your `AnalyzeAsClassAttr`
+ (`:99`); same idea for methods (`hasName(...)` *or* `hasAnalyzeAsMethodName`).
+- **Ordering in the match switch is load-bearing** — nullopt cases before the
+ generic value case, or the value case eats the nullopt tag.
+- **`valueTypeFromOptionalDecl` (`:474`)** is the `template-arg-[0]` shortcut that
+ Valentyn's return-type point would have to replace for general targets.
+
+---
+
+## 4. Design axis: how much should the tool *verify* vs *trust*?
+
+The recurring design question, framed as levels of validation. Key fact: the
+**custom** type is always in the TU; **std::optional** may never be. So:
+
+- **Level 0 (current MVP):** the annotation string is an opaque key. No validation;
+ custom param types never checked. Trusts the annotator completely.
+- **Level 1 — validate the annotation *string*** against a hardcoded table of
+ known std::optional operations. Catches typos. **Header-free; can run in Sema
+ at parse time** (revive `isValidAnalyzeAsMethodAttr`, :6489, to check a real
+ operation list instead of balancing parens).
+- **Level 2 — also validate the custom method's *arity/shape*.** The custom
+ method's real signature *is* available in Sema (`handleAnalyzeAsMethod` gets the
+ method decl `D`, :6558). Compare its arity against the hardcoded expected shape.
+ **Still header-free, still parse-time.**
+- **Level 3 — validate custom param *types* correspond to std's.** Breaks down:
+ the custom tag (`nothing_t`) deliberately differs in name from `std::nullopt_t`,
+ so name comparison would reject valid code. Needs tag registration or structural
+ cues. **This is the rabbit hole — and it's independent of whether std is loaded.**
+
+Conclusion: **Levels 1–2 are the sweet spot** and dissolve the "validation forces
+the header" dilemma (validate against the hardcoded table + the custom decl, both
+present — you never needed the real std::optional). Level 3 is high-friction,
+low-value.
+
+### The `#include <optional>` blocker (why Plan A stalled)
+
+Plan A = validate against the **real** std::optional signatures. That is the *only*
+design that needs the real class in the TU. And the blocker is **not** about
+timing:
+
+- Fundamental (layer-independent): std::optional isn't guaranteed to be in the TU
+ at all — nothing forces a file using the custom type to `#include <optional>`.
+ True at parse time *and* model time.
+- Parse-time-only extra wrinkle: even if the TU includes `<optional>`, at the
+ moment Sema handles the attribute on the custom class, it may not have been seen
+ yet (include ordering). Model time doesn't have this second problem.
+
+So the header dependency is fundamental to validating-against-std, not an artifact
+of *when* the check runs — moving to the model layer does not fix it.
+
+### Alternatives in play
+
+- **This PR (MVP):** argument strings as opaque keys in `analyze_as_method`. No
+ signature mapping to the template class.
+- **BaLiKfromUA's POC:** bare `analyze_as_method("optional")`, disambiguate custom
+ ctors by **arity-correspondence** to std::optional's ctor set. Open question:
+ does he match against the *real* std decl (needs header) or a *hardcoded table*
+ (header-free)? Either way, two 1-arg overloads with different outcomes
+ (`optional(nullopt_t)` vs `optional(T&&)`) collide on arity and still need a
+ type signal — where tag-registration / an explicit outcome tag plugs the gap.
+- **Return types (Valentyn):** orthogonal to arity matching. C++ forbids
+ overloading on return type alone, so it's only ever a tie-breaker — but for
+ *general* targets the contained type must come from the unwrap method's return
+ type (replacing `valueTypeFromOptionalDecl`'s template-arg-[0] shortcut).
+```
diff --git a/hicketts/hicketts_optional_general.h b/hicketts/hicketts_optional_general.h
new file mode 100644
index 0000000000000..be0ff94717272
--- /dev/null
+++ b/hicketts/hicketts_optional_general.h
@@ -0,0 +1,116 @@
+#ifndef HICKETTS_OPTIONAL_H_
+#define HICKETTS_OPTIONAL_H_
+
+/// A custom optional-like type with differently named functions.
+/// Mirrors std::optional semantics but uses its own vocabulary
+/// In order to test implementation of attributes for clang-tidy
+namespace mylib {
+
+struct nothing_t {
+ constexpr explicit nothing_t() {}
+};
+
+constexpr nothing_t nothing;
+
+template <typename T>
+class [[clang::analyze_as_class("std::optional")]] HickettsOptional {
+ T *storage_ = nullptr;
+
+public:
+ // No matcher needed: default (0-arg) construction matches none of the
+ // constructor cases, so has_value is left unconstrained and access is
+ // conservatively treated as maybe-empty (warns).
+ // [[clang::analyze_as_method("optional()")]]
+ constexpr HickettsOptional() noexcept {}
+
+ // KEEP (POC target): nothing_t is not std::nullopt_t, so
+ // isOptionalNulloptConstructor (UncheckedOptionalAccessModel.cpp:288) misses
+ // and this falls through to the value/conversion case (:300) -> wrongly
+ // engaged. The new signature-matched constructor case will route this
+ // annotation to the nullopt transfer (empty).
+ [[clang::analyze_as_method("optional(std::nullopt_t)")]]
+ constexpr HickettsOptional(nothing_t) noexcept {}
+
+ // Already handled by isOptionalValueOrConversionConstructor (:300, registered
+ // :1038): single-arg construction from a value -> engaged.
+ // [[clang::analyze_as_method("optional(T&&)")]]
+ constexpr HickettsOptional(T) noexcept {}
+
+ // Copy ctor: no dedicated case; excluded from value/conversion (:302-303) and
+ // handled by the framework's default record-copy, which propagates has_value
+ // from the source.
+ // [[clang::analyze_as_method("optional(const optional&)")]]
+ HickettsOptional(const HickettsOptional &) = default;
+
+ // Move ctor: same as copy — excluded from value/conversion (:302-303),
+ // handled by the framework's default record-copy.
+ // [[clang::analyze_as_method("optional(const optional&&)")]]
+ HickettsOptional(HickettsOptional &&) = default;
+
+ // Equivalent to std::optional::value()
+ [[clang::analyze_as_method("value")]] const T &unwrap() const & { return *storage_; }
+ [[clang::analyze_as_method("value")]] T &unwrap() & { return *storage_; }
+ [[clang::analyze_as_method("value")]] const T &&unwrap() const && { return static_cast<const T &&>(*storage_); }
+ [[clang::analyze_as_method("value")]] T &&unwrap() && { return static_cast<T &&>(*storage_); }
+
+ const T &value() const & { return *storage_; }
+ T &value() & { return *storage_; }
+ const T &&value() const && { return static_cast<const T &&>(*storage_); }
+ T &&value() && { return static_cast<T &&>(*storage_); }
+
+ // Equivalent to std::optional::operator*()
+ [[clang::analyze_as_method("value")]] const T &deref() const & { return *storage_; }
+ [[clang::analyze_as_method("value")]] T &deref() & { return *storage_; }
+
+ // Equivalent to std::optional::operator->()
+ const T* operator ->() const { return storage_; }
+ T* operator ->() { return storage_; }
+ const T *arrow() const { return storage_; }
+ T *arrow() { return storage_; }
+
+ // Equivalent to std::optional::operator bool / hasValue()
+ constexpr bool has_value() const noexcept { return storage_ != nullptr; }
+ constexpr explicit operator bool() const noexcept { return storage_ != nullptr; }
+ [[clang::analyze_as_method("has_value")]] constexpr bool isPresent() const noexcept { return storage_ != nullptr; }
+
+ // Equivalent to std::optional::value_or()
+ template <typename U>
+ constexpr T unwrapOr(U &&fallback) const & {
+ return storage_ ? *storage_ : static_cast<T>(fallback);
+ }
+
+ // Equivalent to std::optional::emplace()
+ template <typename... Args>
+ [[clang::analyze_as_method("emplace(Args&&...)")]]
+ T& construct(Args&&... args) { return *storage_; }
+
+ // Demo of malformed-signature rejection — disabled. The parameter-balance
+ // validation in Sema (isValidAnalyzeAsMethodAttr) that rejected this string
+ // was removed, since matching is now a flat string compare that never parses
+ // parameters. With validation gone this annotation would be accepted silently
+ // (and simply never match), so the case no longer demonstrates anything.
+ // [[clang::analyze_as_method("emplace(oops))")]]
+ // T& load() { return *storage_; }
+
+ // Equivalent to std::optional::reset()
+ [[clang::analyze_as_method("reset")]] void clear() noexcept { storage_ = nullptr; }
+
+ // Equivalent to std::optional::swap()
+ [[clang::analyze_as_method("swap")]] void exchange(HickettsOptional &other) noexcept {
+ T *tmp = storage_;
+ storage_ = other.storage_;
+ other.storage_ = tmp;
+ }
+
+ // Assignment
+ template <typename U>
+ HickettsOptional &operator=(const U &u) { return *this; }
+
+ [[clang::analyze_as_method("operator=(nullopt_t)")]]
+ HickettsOptional &operator=(mylib::nothing_t){ storage_ = nullptr; return *this;}
+
+};
+
+} // namespace mylib
+
+#endif // HICKETTS_OPTIONAL_H_
diff --git a/hicketts/hicketts_vector.h b/hicketts/hicketts_vector.h
new file mode 100644
index 0000000000000..08cd3b49ec21c
--- /dev/null
+++ b/hicketts/hicketts_vector.h
@@ -0,0 +1,116 @@
+#ifndef HICKETTS_VECTOR_H_
+#define HICKETTS_VECTOR_H_
+
+/// A minimal std::vector-like container for exercising two families of
+/// attribute at once:
+///
+/// * EXISTING, working attributes (compile today, drive real analysis):
+/// [[gsl::Owner]] / [[gsl::Pointer]] -> -Wdangling lifetime analysis
+/// [[clang::lifetimebound]] -> return tied to *this
+/// [[clang::reinitializes]] -> "returns object to valid state"
+///
+/// * PROPOSED per-object-state role attributes (do NOT exist yet; shown
+/// commented so the header stays buildable). These illustrate the closed
+/// role vocabulary discussed in architecture.md section 4.
+///
+/// The attributes are macro-guarded so the SAME fixture can be compiled two
+/// ways, for a clean before/after:
+/// -DHICKETTS_VECTOR_NO_ATTRS -> baseline, no attributes
+/// (default) -> attributes on
+namespace mylib {
+
+#ifdef HICKETTS_VECTOR_NO_ATTRS
+#define HV_OWNER
+#define HV_POINTER
+#define HV_LIFETIMEBOUND
+#define HV_REINITIALIZES
+#else
+#define HV_OWNER [[gsl::Owner]]
+#define HV_POINTER [[gsl::Pointer]]
+#define HV_LIFETIMEBOUND [[clang::lifetimebound]]
+#define HV_REINITIALIZES [[clang::reinitializes]]
+#endif
+
+template <typename T>
+class HV_OWNER HickettsVector {
+ // Tiny fixed buffer keeps the fixture simple (no allocator); big enough for
+ // small tests, and irrelevant to the static lifetime analysis anyway.
+ T buf_[16] = {};
+ unsigned size_ = 0;
+
+public:
+ // A pointer-like handle INTO the container. Marked gsl::Pointer so the
+ // lifetime analysis knows it can dangle once the owning vector dies.
+ class HV_POINTER iterator {
+ T *p_ = nullptr;
+
+ public:
+ iterator() = default;
+ explicit iterator(T *p) : p_(p) {}
+ T &operator*() const { return *p_; }
+ iterator &operator++() {
+ ++p_;
+ return *this;
+ }
+ bool operator==(const iterator &o) const { return p_ == o.p_; }
+ bool operator!=(const iterator &o) const { return p_ != o.p_; }
+ };
+
+ HickettsVector() = default;
+
+ // --- Element access -------------------------------------------------------
+ // lifetimebound: the returned reference/iterator is tied to *this, so
+ // -Wdangling fires when *this is a temporary. This is the RELATIONAL hazard
+ // (container <-> derived handle) and is fully covered by Owner/Pointer +
+ // lifetimebound -- no per-method role needed.
+ T &front() HV_LIFETIMEBOUND { return buf_[0]; }
+ T &back() HV_LIFETIMEBOUND { return buf_[size_ - 1]; }
+ T &operator[](unsigned i) HV_LIFETIMEBOUND { return buf_[i]; }
+
+ iterator begin() HV_LIFETIMEBOUND { return iterator(buf_); }
+ iterator end() HV_LIFETIMEBOUND { return iterator(buf_ + size_); }
+
+ // --- State transitions ---------------------------------------------------
+ // reinitializes: clear() returns the object to a defined (empty) state. This
+ // already applies to both vector::clear() and optional::reset(), and is a
+ // real "makes valid" role attribute shipping today.
+ HV_REINITIALIZES void clear() { size_ = 0; }
+
+ // Mutators. In a real std::vector these INVALIDATE existing iterators and
+ // references -- a relational hazard that Owner/Pointer models via lifetime,
+ // but which the per-object role attributes below CANNOT express (there is no
+ // single per-object bit meaning "every handle I handed out is now stale").
+ void push_back(const T &v) { buf_[size_++] = v; }
+ void pop_back() { --size_; }
+
+ // --- Queries -------------------------------------------------------------
+ unsigned size() const { return size_; }
+ bool empty() const { return size_ == 0; }
+
+ // --- PROPOSED per-object-state role attributes (NOT YET IMPLEMENTED) ------
+ // Shown commented; enabling them requires adding the attributes first.
+ // Spellings are illustrative only -- see architecture.md section 4 for the
+ // "closed role vocabulary" vs "capability-style" options still open.
+ //
+ // front()/back()/pop_back() carry a precondition: the vector is non-empty.
+ // [[clang::requires_state("non_empty")]] T &front() ...
+ // [[clang::requires_state("non_empty")]] void pop_back() ...
+ //
+ // Transitions that establish a state:
+ // [[clang::sets_state("empty")]] void clear() ...
+ // [[clang::sets_state("non_empty")]] void push_back(const T &) ...
+ //
+ // WHY THIS IS THE INTERESTING TEST:
+ // * empty / non-empty is a SINGLE per-object predicate -- exactly the shape
+ // of optional's has_value -- so it fits the capability/role model, and a
+ // "requires non_empty" on front() is the direct analog of value()
+ // requiring engaged.
+ // * iterator invalidation is RELATIONAL, so it does NOT fit a per-object
+ // bit and stays with Owner/Pointer. That boundary is the constraint we
+ // wanted to surface: role attributes generalise to the state-predicate
+ // slice of a container, not to its aliasing hazards.
+};
+
+} // namespace mylib
+
+#endif // HICKETTS_VECTOR_H_
diff --git a/hicketts/plan_general.md b/hicketts/plan_general.md
new file mode 100644
index 0000000000000..fb197def43383
--- /dev/null
+++ b/hicketts/plan_general.md
@@ -0,0 +1,155 @@
+# Plan: role-attribute approach (branch `generalAttributesKay`)
+
+Kickoff plan for the *alternate* proposal. This branch is a clean-room from
+`main` (no `analyze_as_*` string-match implementation). The goal is a **single,
+closed role-attribute vocabulary** that models one per-object boolean predicate,
+serving BOTH target types with the same engine.
+
+Companion docs: `architecture.md` (how the optional pipeline works + the
+levels-of-validation and two-worlds framing). The string-match MVP lives on
+`attributesKay`; this is deliberately separate, not a rework.
+
+Learning-exercise rule still applies: this plan is design/scope only. Do not
+implement the compiler changes; fixtures (`hicketts/*_general.*`,
+`hicketts_vector.*`) are fair game.
+
+---
+
+## 1. Thesis
+
+Model each supported class as having **one named boolean predicate**, and let
+method-level *role* attributes say how each method relates to it. This unifies:
+
+- **optional** — predicate `engaged` (== today's `has_value`). Replaces the
+ `analyze_as_method("...")` string keys with roles.
+- **vector** — predicate `non_empty`. Fills the `precondition_gap` measured in
+ the vector experiment (empty `front()`/`pop_back()` is UB and caught by
+ *nothing* today — not the baseline, not Owner/Pointer).
+
+Same dataflow question in both cases: "is the predicate established on this path
+before a method that requires it?" That is exactly what the
+`bugprone-unchecked-optional-access` model already answers for `has_value` — so
+the core implementation idea is to **generalise that model from the hardcoded
+`has_value` field to an arbitrary named predicate.**
+
+## 2. Why roles beat verbatim signatures (recap of the decision)
+
+- **Identity vs role.** The string signature described how to *identify* a method
+ (its params). But overload resolution + the attribute sitting on one specific
+ decl already identify it. What the model actually needs is the method's *role*.
+- **Per-decl placement disambiguates overloads** — no signature strings needed.
+- **Header-free.** Roles never resolve the real `std::optional`/`std::vector`, so
+ `#include` is irrelevant (see `architecture.md` §4, the `<optional>` blocker).
+
+## 3. Precedents to ride (all in-tree — cite these in the RFC)
+
+- **Capability / thread-safety attributes** (`Attr.td:4101`–`4184`):
+ `RequiresCapability`, `AcquireCapability`, `ReleaseCapability`. This is
+ literally requires-valid / makes-valid / makes-invalid as a **closed,
+ capability-scoped** role vocabulary — the exact shape we want, already accepted.
+- **`reinitializes`** (`:4877`) — "returns object to a defined state"; already
+ applies to both `optional::reset()` and `vector::clear()`.
+- **Consumed / typestate attributes** (`:4285`–`:4366`) — `Consumable`,
+ `SetTypestate`, `CallableWhen`, `TestTypestate`. Cite as evidence typestate is
+ acceptable in clang, but frame OUR proposal as the *narrower capability shape*,
+ NOT general typestate, to avoid the earlier rejection.
+
+## 4. Scope boundary (state it up front in the RFC)
+
+Role attributes model a **single per-object state predicate**. In scope:
+
+- optional `engaged`; vector `non_empty`; reset-to-valid (`reinitializes`).
+
+Explicitly OUT of scope (and why):
+
+- **Relational / aliasing hazards** (iterator invalidation: `push_back` stales
+ existing iterators). Not a per-object bit — stays with `Owner`/`Pointer`
+ (which already handle it; see the vector experiment: dangling *was* caught).
+- **Numeric invariants** (`size`/`capacity` relationships). The model tracks a
+ predicate, not a quantity.
+
+## 5. Measured motivation (vector experiment, recorded here)
+
+`test_hicketts_vector.cpp`, built with `build-llvm/bin/clang++`
+`--target=arm64-apple-darwin -std=c++17`:
+
+| Case | Baseline (attrs off) | Owner/Pointer + lifetimebound |
+|------|----------------------|-------------------------------|
+| `front()` of a temporary (dangling ref) | silent | ⚠ `-Wdangling` |
+| `begin()` of a temporary (dangling iter) | silent | ⚠ `-Wdangling` |
+| ref/iter into a live vector | silent | silent ✓ |
+| **`front()` on an EMPTY vector (UB)** | **silent** | **silent** ← the gap |
+
+The empty-access row is what the `requires_state("non_empty")` role must make
+warn.
+
+## 6. Proposed vocabulary (DRAFT — open for iteration)
+
+Two axes still open; capture both, pick during RFC:
+
+- **Predicate naming:** string (`"engaged"`, `"non_empty"`) vs a fixed enum vs a
+ single implicit predicate per class. Capability analysis names its capability,
+ so a small **closed string/enum** is precedented and probably best.
+- **Class-level opt-in:** reuse an `analyze_as_class`-style marker to declare the
+ class is state-tracked and name its predicate.
+
+Draft method roles (map straight onto existing optional transfer functions):
+
+| Role (draft spelling) | Meaning | optional example | vector example | model action |
+|---|---|---|---|---|
+| `requires_state("P")` | precondition: P must hold, else warn | `value()`/`unwrap()` | `front()`/`pop_back()` | diagnose if P not established |
+| `sets_state("P")` | establishes P true | value ctor, `emplace` | `push_back` | set predicate true |
+| `clears_state("P")` | establishes P false | nullopt ctor, `reset` | `clear` (+`reinitializes`) | set predicate false |
+| `queries_state("P")` | narrows P in flow | `has_value`/`operator bool` | `empty()` | branch-sensitive refine |
+
+Note: `requires`/`sets`/`clears`/`queries` ≈ `REQUIRES`/`ACQUIRE`/`RELEASE`/(test)
+from thread-safety — keep the analogy explicit.
+
+## 7. Validation strategy (from architecture.md §4)
+
+- **L1** validate the role/predicate name against a closed table (typo-catch),
+ in Sema at parse time — header-free.
+- **L2** validate the annotated method's arity/shape if useful — also parse-time
+ (the method decl is available to the Sema handler).
+- **L3** (verify custom param types vs the real std type) — skip; needs the
+ header and buys little. This is the whole point of NOT going the Plan A route.
+
+## 8. Implementation sketch (design only — do NOT build yet)
+
+1. `Attr.td` — add the class-level predicate marker + the method role attributes
+ (model on the capability attribute defs at `:4101`+).
+2. Sema — handlers + L1/L2 validation (mirror `handleAnalyzeAs*`; capability
+ handlers are a closer template).
+3. Model — the crux: generalise `UncheckedOptionalAccessModel` so the synthetic
+ boolean field is a *named predicate* rather than hardcoded `has_value`
+ (`:1330`, `:441`–`:445`), and drive the match-switch cases from the role
+ attributes instead of hardcoded method names.
+4. Decide: extend `bugprone-unchecked-optional-access` to arbitrary predicates,
+ or spin a sibling check for the general "state precondition" analysis. (Open.)
+
+## 9. Test plan
+
+- **optional** — re-annotate `hicketts_optional_general.h` with the new roles;
+ `test_hicketts_optional_general.cpp` should reproduce the MVP's behaviour
+ (the same set of expected warnings/silences the string-match version produced).
+- **vector** — add empty-access cases to `test_hicketts_vector.cpp`; the
+ `precondition_gap` case must now warn, while the dangling cases keep warning
+ via Owner/Pointer and safe cases stay silent.
+
+## 10. Open questions
+
+- Predicate naming: string vs enum vs fixed-per-class.
+- One predicate per class, or several (e.g. a type with two independent states)?
+- Diagnoser wording for "required state not established here."
+- Extend the optional check vs new check (§8.4).
+- Does branch-sensitive `queries_state` need more than the optional model already
+ does for `has_value`/`operator bool`?
+
+## 11. Files (this branch)
+
+- `hicketts/hicketts_optional_general.h` / `test_hicketts_optional_general.cpp`
+ — optional fixture (currently still carries old `analyze_as_*`; to be re-annotated).
+- `hicketts/hicketts_vector.h` / `test_hicketts_vector.cpp` — vector fixture
+ (Owner/Pointer + lifetimebound live; proposed roles commented).
+- `hicketts/architecture.md` — pipeline map + design framings.
+- (later) `Attr.td`, Sema, model changes in the real tree.
diff --git a/hicketts/test_hicketts_optional_general.cpp b/hicketts/test_hicketts_optional_general.cpp
new file mode 100644
index 0000000000000..1ddea121ce5ab
--- /dev/null
+++ b/hicketts/test_hicketts_optional_general.cpp
@@ -0,0 +1,118 @@
+// Test cases for mylib::HickettsOptional — a custom optional-like type
+// with differently named functions.
+//
+// Run from hicketts/ with:
+// ../build-llvm/bin/clang-tidy -checks='bugprone-unchecked-optional-access' \
+// test_hicketts_optional_general.cpp -- -I . -std=c++17 -Wno-undefined-inline
+
+#include "hicketts_optional_general.h"
+
+// --- Unchecked access (should warn if the checker recognises HickettsOptional) ---
+
+static void uncheckedUnwrap(mylib::HickettsOptional<int> &Val) {
+ Val.unwrap(); // unchecked access — may be empty
+}
+
+static void uncheckedValue(mylib::HickettsOptional<int> &Val) {
+ Val.value(); // unchecked access — may be empty
+}
+
+static void uncheckedDeref(mylib::HickettsOptional<int> &Val) {
+ Val.deref(); // unchecked access — may be empty
+}
+
+// --- Checked access (should NOT warn) ---
+
+static void checkedWithBool(mylib::HickettsOptional<int> &Val) {
+ if (Val) {
+ Val.unwrap(); // safe — checked via operator bool
+ }
+}
+
+static void checkedValueWithBool(mylib::HickettsOptional<int> &Val) {
+ if (Val.has_value()) {
+ Val.value(); // safe — checked via operator bool
+ }
+}
+
+static void checkedWithIsPresent(mylib::HickettsOptional<int> &Val) {
+ if (Val.isPresent()) {
+ Val.unwrap(); // safe — checked via isPresent()
+ }
+}
+
+/* static void checkedWithIsEmpty(mylib::HickettsOptional<int> &Val) {
+ if (!Val.isEmpty()) {
+ Val.unwrap(); // safe — checked via !isEmpty()
+ }
+} NYI */
+
+// --- State changes ---
+
+// construct() is annotated "emplace(Args&&...)"; the bare "emplace" query matches
+// it via the name-part (accept-either) branch -> engaged, so unwrap is safe.
+static void safeAfterConstruct(mylib::HickettsOptional<int> &Val) {
+ Val.construct(42);
+ Val.unwrap(); // safe — just constructed a value
+}
+
+static void unsafeAfterClear(mylib::HickettsOptional<int> &Val) {
+ Val.construct(42);
+ Val.clear();
+ Val.unwrap(); // unsafe — value was cleared
+}
+
+static void unsafeAfterExchange(mylib::HickettsOptional<int> &A,
+ mylib::HickettsOptional<int> &B) {
+ if (A) {
+ A.exchange(B);
+ A.unwrap(); // unsafe — a's state is now unknown
+ }
+}
+
+// Works today WITHOUT any annotation: default construction matches no
+// constructor case, so has_value is unconstrained -> access conservatively warns.
+static void unsafeAfterEmptyConstr() {
+ mylib::HickettsOptional<int> A;
+ A.unwrap(); // expected: warn (empty)
+}
+
+// nothing_t is not std::nullopt_t, so the structural nullopt matcher misses.
+// The "optional(std::nullopt_t)" annotation routes this constructor to the
+// nullopt transfer (empty) via isOptionalNulloptConstructor's annotation branch,
+// so the following unwrap is correctly flagged.
+static void unsafeAfterNullConstr() {
+ mylib::HickettsOptional<int> A(mylib::nothing);
+ A.unwrap(); // warns (empty) — routed to nullopt via the annotation
+}
+
+// Works today WITHOUT any annotation: value/conversion constructor case ->
+// engaged, so access is safe.
+static void safeAfterTypeConstr() {
+ mylib::HickettsOptional<int> A(5);
+ A.unwrap(); // expected: no warning (engaged)
+}
+
+// --- Guarded paths ---
+
+/*static void constructCoversEmptyBranch(mylib::HickettsOptional<int> &Val) {
+ if (Val.isEmpty()) {
+ Val.construct(99);
+ }
+ Val.unwrap(); // safe — either was present, or construct filled it
+}*/
+
+static void unwrapOrIsAlwaysSafe(mylib::HickettsOptional<int> &Val) {
+ int X = Val.unwrapOr(0); // safe — fallback provided
+ (void)X;
+}
+
+// nothing_t is not std::nullopt_t, so the structural nullopt matcher misses.
+// The "operator=(nullopt_t)" annotation routes this assignment to the nullopt
+// transfer (empty) — checked before the value/conversion-assignment case — so
+// the following unwrap is correctly flagged.
+static void unsafeAfterNullAssign() {
+ mylib::HickettsOptional<int> A(5);
+ A = mylib::nothing;
+ A.unwrap(); // warns (empty) — routed to nullopt via the annotation
+}
diff --git a/hicketts/test_hicketts_vector.cpp b/hicketts/test_hicketts_vector.cpp
new file mode 100644
index 0000000000000..9cc29515a4442
--- /dev/null
+++ b/hicketts/test_hicketts_vector.cpp
@@ -0,0 +1,54 @@
+// Test fixture for the vector-like lifetime/role attribute experiment.
+//
+// Baseline (NO attributes):
+// build-llvm/bin/clang-tidy hicketts/test_hicketts_vector.cpp -- \
+// -std=c++17 -Ihicketts -DHICKETTS_VECTOR_NO_ATTRS
+//
+// With attributes on:
+// build-llvm/bin/clang-tidy hicketts/test_hicketts_vector.cpp -- \
+// -std=c++17 -Ihicketts
+
+#include "hicketts_vector.h"
+
+using mylib::HickettsVector;
+
+// --- Relational hazard: handles that outlive the container ------------------
+// With gsl::Owner/gsl::Pointer + lifetimebound these should warn (-Wdangling).
+// Baseline (no attributes) cannot know and stays silent.
+
+int dangling_reference_from_temporary() {
+ int &r = HickettsVector<int>{}.front(); // r dangles: temporary destroyed here
+ return r;
+}
+
+int dangling_iterator_from_temporary() {
+ auto it = HickettsVector<int>{}.begin(); // it dangles into destroyed temporary
+ return *it;
+}
+
+// --- Safe counterparts (should stay silent either way) ----------------------
+
+int safe_reference() {
+ HickettsVector<int> v;
+ v.push_back(1);
+ int &r = v.front(); // v outlives r
+ return r;
+}
+
+int safe_iterator() {
+ HickettsVector<int> v;
+ v.push_back(1);
+ auto it = v.begin(); // v outlives it
+ return *it;
+}
+
+// --- Precondition hazard (NOT covered by ANY current attribute) -------------
+// front()/pop_back() on an empty vector is UB. Neither the baseline nor the
+// Owner/Pointer attributes catch this -- it is the case the PROPOSED
+// requires_state("non_empty") role attribute would target. Kept here to show
+// the gap the role vocabulary is meant to fill.
+
+int precondition_gap() {
+ HickettsVector<int> v; // empty
+ return v.front(); // UB today: no warning from any attribute
+}
>From 539963fbaec508a1686df869b0ff124bb4b3155f Mon Sep 17 00:00:00 2001
From: khickett <khicketts at bloomberg.net>
Date: Wed, 5 Aug 2026 16:14:11 +0100
Subject: [PATCH 2/5] what if we did both?
---
hicketts/hicketts_optional_hybrid.h | 146 +++++++++++++++++++++
hicketts/plan_general.md | 70 ++++++++++
hicketts/test_hicketts_optional_hybrid.cpp | 120 +++++++++++++++++
hicketts/why_class_comparison.md | 117 +++++++++++++++++
4 files changed, 453 insertions(+)
create mode 100644 hicketts/hicketts_optional_hybrid.h
create mode 100644 hicketts/test_hicketts_optional_hybrid.cpp
create mode 100644 hicketts/why_class_comparison.md
diff --git a/hicketts/hicketts_optional_hybrid.h b/hicketts/hicketts_optional_hybrid.h
new file mode 100644
index 0000000000000..df06e5d912506
--- /dev/null
+++ b/hicketts/hicketts_optional_hybrid.h
@@ -0,0 +1,146 @@
+#ifndef HICKETTS_OPTIONAL_HYBRID_H_
+#define HICKETTS_OPTIONAL_HYBRID_H_
+
+/// A custom optional-like type wired for the *hybrid* attribute scheme.
+///
+/// Three cooperating layers sit on the SAME type, each carrying a different
+/// KIND of knowledge (see architecture.md, plan_general.md, why_class_comparison.md):
+///
+/// L1 IDENTITY [[clang::analyze_as_class("std::optional")]]
+/// "I behave like std::optional." A single class-level declaration that
+/// anchors the type so OTHER clang-tidy checks reuse their built-in,
+/// per-std-class knowledge by identity (why_class_comparison.md). No
+/// per-method name-maps: on optional nothing else consumes them, and the
+/// dataflow model is driven by the L2 roles below instead.
+///
+/// L2 BEHAVIOURAL ROLES [[clang::engaged/disengaged/test_engaged/assume_engaged]]
+/// The single per-object boolean predicate the flow-sensitive
+/// bugprone-unchecked-optional-access model tracks. The predicate has no
+/// name because it is INERT to the model -- the model tracks one opaque
+/// bit, so "empty" and "disengaged" are the same action (set-false) and
+/// a name would only matter if a type had >1 predicate. What is NOT inert
+/// is polarity; optional's API here is uniformly positive (has_value,
+/// value), so only the positive verbs appear:
+/// engaged -> establish the bit true (value ctor, emplace, =value)
+/// disengaged -> establish the bit false (nullopt ctor, reset, =nullopt)
+/// test_engaged -> branch-sensitive read (has_value, operator bool)
+/// assume_engaged -> precondition: warn if not established (value/*/->)
+/// (A negative-polarity test/assume -- e.g. an isEmpty()-style query, or
+/// vector's empty()/front() -- is where polarity, not the name, would
+/// reappear. Out of scope for this fixture.)
+/// PROPOSED, not yet implemented, so this is the one layer left behind a
+/// macro: -DHO_ROLES turns it on for a roles-off/roles-on baseline.
+///
+/// L3 LIFETIME [[gsl::Owner]] / [[clang::lifetimebound]]
+/// Real, shipping-today attributes. An optional OWNS its T; unwrap()/
+/// deref()/operator-> hand out handles INTO it, so a handle taken from
+/// a *temporary* optional dangles (-Wdangling). This is the RELATIONAL
+/// hazard the predicate layer deliberately does NOT model.
+///
+/// L1 and L3 are inline (they exist today, ignored where unsupported). Only L2
+/// is guarded, because those attributes do not exist yet:
+/// (default) L1 + L3 -> -Wdangling live; dataflow model idle until roles
+/// -DHO_ROLES + L2 -> needs the proposed role attributes
+namespace mylib {
+
+// The only guarded layer: the proposed per-object-state roles, which do not
+// exist yet. Elided by default so the header builds without -Wunknown-attributes;
+// -DHO_ROLES emits them once they are implemented. No predicate argument -- the
+// type has a single, model-opaque predicate.
+#ifdef HO_ROLES
+#define HO_ENGAGED [[clang::engaged]]
+#define HO_DISENGAGED [[clang::disengaged]]
+#define HO_TEST_ENGAGED [[clang::test_engaged]]
+#define HO_ASSUME_ENGAGED [[clang::assume_engaged]]
+#else
+#define HO_ENGAGED
+#define HO_DISENGAGED
+#define HO_TEST_ENGAGED
+#define HO_ASSUME_ENGAGED
+#endif
+
+struct nothing_t {
+ constexpr explicit nothing_t() {}
+};
+
+constexpr nothing_t nothing;
+
+template <typename T>
+class [[gsl::Owner]] [[clang::analyze_as_class("std::optional")]] HickettsOptional {
+ T *storage_ = nullptr;
+
+public:
+ // Default ctor -> disengaged.
+ HO_DISENGAGED
+ constexpr HickettsOptional() noexcept {}
+
+ // Nullopt-style ctor. SAME 1-arg shape as the value ctor below, DIFFERENT
+ // outcome. The role on THIS decl disambiguates -- overload resolution picks it
+ // for HickettsOptional(nothing); no signature string, no std header.
+ HO_DISENGAGED
+ constexpr HickettsOptional(nothing_t) noexcept {}
+
+ // Value ctor -> engaged. Same 1-arg shape; the role on the OTHER decl is what
+ // tells them apart (the whole point of the hybrid).
+ HO_ENGAGED
+ constexpr HickettsOptional(T) noexcept {}
+
+ // Copy / move: no role -- the framework's default record-copy propagates the
+ // predicate from the source.
+ HickettsOptional(const HickettsOptional &) = default;
+ HickettsOptional(HickettsOptional &&) = default;
+
+ // value()/unwrap()/deref(): precondition engaged (L2) AND return a handle
+ // INTO *this (L3). One method, two independent hazards:
+ // L2 assume_engaged -> unchecked-optional-access diagnostic
+ // L3 lifetimebound -> -Wdangling when *this is a temporary
+ HO_ASSUME_ENGAGED const T &unwrap() const & [[clang::lifetimebound]] { return *storage_; }
+ HO_ASSUME_ENGAGED T &unwrap() & [[clang::lifetimebound]] { return *storage_; }
+
+ HO_ASSUME_ENGAGED const T &deref() const & [[clang::lifetimebound]] { return *storage_; }
+ HO_ASSUME_ENGAGED T &deref() & [[clang::lifetimebound]] { return *storage_; }
+
+ const T *operator->() const [[clang::lifetimebound]] { return storage_; }
+ T *operator->() [[clang::lifetimebound]] { return storage_; }
+
+ // Queries: branch-sensitive read, positive polarity.
+ HO_TEST_ENGAGED constexpr bool has_value() const noexcept { return storage_ != nullptr; }
+ HO_TEST_ENGAGED constexpr explicit operator bool() const noexcept { return storage_ != nullptr; }
+ HO_TEST_ENGAGED constexpr bool isPresent() const noexcept { return storage_ != nullptr; }
+
+ // value_or: always safe, no precondition.
+ template <typename U>
+ constexpr T unwrapOr(U &&fallback) const & {
+ return storage_ ? *storage_ : static_cast<T>(fallback);
+ }
+
+ // emplace -> engaged.
+ template <typename... Args>
+ HO_ENGAGED T &construct(Args &&...args) { return *storage_; }
+
+ // reset -> disengaged.
+ HO_DISENGAGED void clear() noexcept { storage_ = nullptr; }
+
+ // swap: leaves both operands' predicates unknown -- no role (havoc), handled
+ // by the model's default.
+ void exchange(HickettsOptional &other) noexcept {
+ T *tmp = storage_;
+ storage_ = other.storage_;
+ other.storage_ = tmp;
+ }
+
+ // Assignment from a value -> engaged.
+ template <typename U>
+ HO_ENGAGED HickettsOptional &operator=(const U &u) { return *this; }
+
+ // Nullopt-style assignment -> disengaged. Same disambiguation story as the
+ // ctors: the role on this decl routes it.
+ HO_DISENGAGED HickettsOptional &operator=(nothing_t) {
+ storage_ = nullptr;
+ return *this;
+ }
+};
+
+} // namespace mylib
+
+#endif // HICKETTS_OPTIONAL_HYBRID_H_
diff --git a/hicketts/plan_general.md b/hicketts/plan_general.md
index fb197def43383..0352b77e0ffff 100644
--- a/hicketts/plan_general.md
+++ b/hicketts/plan_general.md
@@ -153,3 +153,73 @@ from thread-safety — keep the analogy explicit.
(Owner/Pointer + lifetimebound live; proposed roles commented).
- `hicketts/architecture.md` — pipeline map + design framings.
- (later) `Attr.td`, Sema, model changes in the real tree.
+
+---
+
+## 12. Cross-check consumption: shared vocabulary vs private hook (Valentyn / PR)
+
+**The question.** `analyze_as_*` is today a *private hook* consumed by ONE check
+(the optional dataflow model). Valentyn's `size()==0 -> isEmpty()` example assumes
+it is a *shared, cross-check vocabulary*: annotate a type once and every relevant
+check honours it, using the type's custom method names. These are very different
+scopes — the PR should pick a lane explicitly rather than leave it implied.
+
+**Worked second consumer — `readability-container-size-empty`
+(`ContainerSizeEmptyCheck.cpp`).** Recognises a container *purely structurally*:
+a `size`/`length` method AND a method literally named `empty` returning bool
+(`:150`, `:154`). On `x.size()==0` it warns + attaches a fix-it whose replacement
+text is hardcoded to `"empty()"` (`:275`-`281`). Consequence for
+`[[analyze_as_method("empty")]] bool isEmpty()`:
+- it does NOT fire (the empty method isn't named `empty`; the attribute is ignored), and
+- even if it did, the fix-it would emit `empty()`, not `isEmpty()`.
+
+To support it: (a) accept an `analyze_as_method("empty")` bool method as the
+"empty" role alongside `hasName("empty")`, and (b) emit the *matched method's real
+name* in the fix-it. Note it also requires `returns(booleanType())` -> the L2
+return-type validation matters here too (Valentyn's recurring return-type theme).
+
+**The landscape (survey of candidate consumers).** Every check below recognises
+its target by its OWN hardcoded structural logic (`hasName(...)`, hardcoded
+`"std::..."`); NONE read `analyze_as_*` today. Grouped by what they'd need:
+
+Container method-role consumers (need "which method is empty/find/data/..."):
+- `readability/ContainerSizeEmptyCheck` size()==0 -> empty()
+- `readability/ContainerContainsCheck` find()!=end() -> contains()
+- `readability/ContainerDataPointerCheck` &v[0] -> v.data()
+- `readability/SimplifySubscriptExprCheck`
+- `bugprone/StandaloneEmptyCheck` ignored empty() (meant clear()?)
+- `bugprone/InaccurateEraseCheck` erase-remove idiom
+- `bugprone/SizeofContainerCheck` sizeof(container) misuse
+- `performance/InefficientAlgorithmCheck` std::find on set -> member find
+- `modernize/LoopConvertCheck` begin/end -> range-for
+- `modernize/UseEmplaceCheck` push_back -> emplace_back
+- `modernize/ShrinkToFitCheck` swap-trick -> shrink_to_fit
+
+Optional-role consumers:
+- `bugprone/UncheckedOptionalAccessCheck` (current consumer)
+- `bugprone/OptionalValueConversionCheck`
+
+Smart-pointer-role consumers:
+- `modernize/Make{Unique,Shared,SmartPtr}Check`
+- `bugprone/{Unique,Shared,Smart}PtrArrayMismatchCheck`
+
+**Why the "we'll miss cases" worry is bounded.** The set is *enumerable*
+(~a dozen container checks + a few optional/smart-ptr), and consumption is
+*explicit per check* — a check either reads the vocabulary or it doesn't, so
+nothing is silently missed. You can't retrofit them all at once and shouldn't try.
+
+**Recommended architecture (do NOT big-bang):**
+1. Define the shared vocabulary as a CLOSED, DEMAND-DRIVEN role set — add a role
+ only when a consumer needs it. Method-roles span more than the optional
+ predicate (empty/find/contains/data/begin/end/...), so demand-driven or it
+ balloons.
+2. Provide ONE reusable helper/matcher: "does type T play class-role R" / "does
+ method M play method-role X", reading the attributes — so a check opts in with
+ a small change instead of reinventing recognition.
+3. Each opting-in check must ALSO fix its FIX-IT to use the real method name (the
+ container-size-empty `"empty()"` hardcoding is the canonical trap).
+4. Treat this survey as the ROADMAP; convert consumers incrementally.
+
+**For THIS PR:** keep scope to the dataflow correctness check; name
+`container-size-empty` as the concrete next consumer that proves the vocabulary is
+cross-check; record this landscape so no consumer is forgotten.
diff --git a/hicketts/test_hicketts_optional_hybrid.cpp b/hicketts/test_hicketts_optional_hybrid.cpp
new file mode 100644
index 0000000000000..8abb4d9ce1ce9
--- /dev/null
+++ b/hicketts/test_hicketts_optional_hybrid.cpp
@@ -0,0 +1,120 @@
+// Test fixture for the HYBRID attribute scheme on mylib::HickettsOptional.
+//
+// L1 (analyze_as_*) and L3 (gsl::Owner/lifetimebound) are written inline in the
+// header; only L2 (proposed roles) is behind -DHO_ROLES. Two run modes:
+//
+// default -- L1 + L3. -Wdangling fires via the compiler's lifetime analysis;
+// L1 drives unchecked-optional-access if clang-tidy was built with analyze_as_*:
+// ../build-llvm/bin/clang-tidy \
+// -checks='bugprone-unchecked-optional-access' \
+// test_hicketts_optional_hybrid.cpp -- -I . -std=c++17 -Wno-undefined-inline
+//
+// + L2 predicate roles (needs the proposed role attributes implemented):
+// ... -- -I . -std=c++17 -DHO_ROLES -Wno-undefined-inline
+//
+// Every free function below is analyzed independently of main(); main() only
+// exists so the file is a complete program. The unsafe cases are NOT called
+// from main (calling them would execute the very UB they document) -- their
+// addresses are taken to silence -Wunused-function.
+
+#include "hicketts_optional_hybrid.h"
+
+using mylib::HickettsOptional;
+using mylib::nothing;
+
+// === L3: lifetime / dangling (LIVE TODAY via gsl::Owner + lifetimebound) =====
+
+// A handle taken from a temporary optional dangles the moment the full
+// expression ends. Expected: -Wdangling.
+int dangling_from_temporary() {
+ const int &r = HickettsOptional<int>{5}.unwrap(); // temp destroyed here
+ return r;
+}
+
+// Safe counterpart: the optional outlives the reference. Expected: silent.
+int safe_reference() {
+ HickettsOptional<int> o{5};
+ const int &r = o.unwrap();
+ return r;
+}
+
+// === L2: unchecked-optional-access predicate (needs -DHO_ROLES) ==============
+
+// Unchecked access -- may be disengaged. Expected: warn.
+static void uncheckedUnwrap(HickettsOptional<int> &o) {
+ o.unwrap();
+}
+
+// Checked via operator bool -> queries_state narrows to engaged. Expected: safe.
+static void checkedWithBool(HickettsOptional<int> &o) {
+ if (o)
+ o.unwrap();
+}
+
+// Checked via a NAME-MAPPED query method. Expected: safe.
+static void checkedWithIsPresent(HickettsOptional<int> &o) {
+ if (o.isPresent())
+ o.unwrap();
+}
+
+// construct() sets engaged, clear() clears it. Expected: warn after clear.
+static void unsafeAfterClear(HickettsOptional<int> &o) {
+ o.construct(42);
+ o.clear();
+ o.unwrap();
+}
+
+// Expected: safe -- just constructed a value.
+static void safeAfterConstruct(HickettsOptional<int> &o) {
+ o.construct(42);
+ o.unwrap();
+}
+
+// THE disambiguation case, part 1: nullopt-style ctor -> disengaged.
+// Expected: warn.
+static void unsafeAfterNullCtor() {
+ HickettsOptional<int> o(nothing);
+ o.unwrap();
+}
+
+// THE disambiguation case, part 2: value ctor, SAME 1-arg shape -> engaged.
+// Expected: safe. The role on each ctor decl is what tells these two apart --
+// no signature string, no std::optional header.
+static void safeAfterValueCtor() {
+ HickettsOptional<int> o(5);
+ o.unwrap();
+}
+
+// Nullopt-style assignment -> disengaged. Expected: warn.
+static void unsafeAfterNullAssign() {
+ HickettsOptional<int> o(5);
+ o = nothing;
+ o.unwrap();
+}
+
+// value_or never accesses an absent value. Expected: safe.
+static void unwrapOrIsAlwaysSafe(HickettsOptional<int> &o) {
+ int x = o.unwrapOr(0);
+ (void)x;
+}
+
+int main() {
+ HickettsOptional<int> engaged(5);
+
+ // Exercise the safe paths.
+ (void)safe_reference();
+ checkedWithBool(engaged);
+ checkedWithIsPresent(engaged);
+ safeAfterConstruct(engaged);
+ safeAfterValueCtor();
+ unwrapOrIsAlwaysSafe(engaged);
+
+ // Reference the diagnostic-bearing cases without executing their UB, so the
+ // analyzer still sees them but the program stays well-defined if run.
+ (void)&dangling_from_temporary;
+ (void)&uncheckedUnwrap;
+ (void)&unsafeAfterClear;
+ (void)&unsafeAfterNullCtor;
+ (void)&unsafeAfterNullAssign;
+ return 0;
+}
diff --git a/hicketts/why_class_comparison.md b/hicketts/why_class_comparison.md
new file mode 100644
index 0000000000000..7710afb4108a1
--- /dev/null
+++ b/hicketts/why_class_comparison.md
@@ -0,0 +1,117 @@
+# Design note: canonical-class comparison vs a generic attribute vocabulary
+
+## Summary
+
+We annotate a custom type by **declaring which standard class it behaves like**
+(`[[clang::analyze_as_class("std::optional")]]`, methods mapped with
+`analyze_as_method`). The alternative would be a **generic vocabulary** that
+describes a type's semantics abstractly, so any check could consume it without
+knowing about specific std classes.
+
+We chose class-comparison because **the semantic knowledge each clang-tidy check
+needs already exists inside the check, keyed to specific std classes.** Anchoring
+a custom type to a canonical class lets it *inherit that existing knowledge by
+identity*. A generic scheme would instead require lifting that knowledge out of
+every check into an abstract vocabulary and re-expressing it — an expensive,
+per-check, error-prone migration whose cost scales with the semantic richness of
+each check.
+
+## The key idea: where the knowledge lives
+
+An attribute can only ever supply **identity** ("this type is X-like", "this
+method plays role Y"). It cannot supply **behaviour** — the matchers, the
+judgments, and the fix-its live in each check. So the real question is: how much
+does a check already know, and how cheaply can a custom type tap into it?
+
+- **Class-comparison** reuses the check's existing per-std-type logic wholesale.
+ The custom type says "treat me as `std::set`" and the check's vetted `std::set`
+ reasoning applies unchanged.
+- **Generic vocabulary** discards that leverage: each check must be rewritten to
+ reason from abstract properties, and those properties must first be designed to
+ be rich enough to reconstruct what the std class already embodies.
+
+## Worked examples of the cost gap
+
+The two examples below sit at opposite ends of the spectrum, which is the point:
+the generic cost is not uniform — it explodes with the check's semantic depth.
+
+### Example 1 — `readability-container-size-empty` (the cheap end)
+
+Rewrites `x.size() == 0` to `x.empty()`.
+
+- Knowledge it needs: "which method is the boolean emptiness predicate."
+- Generic version: define a role `empty`, teach the matcher to accept it
+ (`ContainerSizeEmptyCheck.cpp:154` requires a bool method literally named
+ `empty`), and make the fix-it emit the *matched method's real name* instead of
+ the hardcoded `"empty()"` (`:275`-`281`).
+- Class-comparison version: the custom type declares it is container-like and
+ maps its predicate method; the same fix-it work is needed either way.
+
+**Verdict:** here the gap is modest — the knowledge is a single name mapping, so
+generic is merely *a bit more* work. If every check looked like this, generic
+would be defensible.
+
+### Example 2 — `performance-inefficient-algorithm` (the expensive end)
+
+Flags `std::find(c.begin(), c.end(), x)` on an associative container (member
+`c.find(x)` is faster) and rewrites it.
+
+- What the check actually knows is a **hardcoded behavioural contract**, keyed on
+ the std class name:
+ - the associative container name list (`:31`-`34`: set/map/multiset/.../unordered_*),
+ - which algorithms have faster members (`:29`),
+ - and subtle per-type semantics derived by string-matching the name:
+ `Unordered = name.contains("unordered")` (`:69`), `Maplike = ...("map")`
+ (`:70`), and "unordered containers have no ordered-bound equivalent"
+ (`:100`).
+- Generic version: the attribute would have to **encode all of that as
+ properties** — "this container has a member `find` asymptotically faster than
+ linear scan," "it is ordered vs unordered," "it is map-like vs set-like," "it
+ lacks `lower_bound`," etc. That is re-deriving `std::set`'s entire behavioural
+ contract in attribute form, and putting its *correctness* on the annotator (a
+ mis-declared complexity yields wrong advice).
+- Class-comparison version: the custom type says `analyze_as_class("std::set")`
+ and **inherits the whole contract for free** — the ordered/unordered, map/set,
+ and bound-availability reasoning all apply unchanged, because the check already
+ contains it.
+
+**Verdict:** here the gap is enormous. The generic route means reconstructing
+years of accumulated per-type special-casing as a general vocabulary; the
+class-comparison route is a one-line declaration.
+
+## Why the gap generalises
+
+The `~dozen` candidate consumer checks (see `plan_general.md` §12) each embed
+their own hardcoded per-std-type knowledge. Class-comparison reuses it N times for
+the price of an identity declaration. A generic vocabulary must be designed to
+capture the *union* of everything those checks care about (emptiness, lookup
+complexity, iterator/ownership semantics, emplace-equivalence, ...) — an
+open-ended surface — and then each check must be rewritten to consume it.
+
+## Secondary advantages of class-comparison
+
+- **Correctness by reuse.** It runs the checks' already-vetted std logic rather
+ than trusting annotator-supplied semantic claims.
+- **Bounded vocabulary.** Its "vocabulary" is essentially the names of standard
+ classes — small and well-understood. A generic property vocabulary is
+ open-ended and must grow with every consumer.
+- **Incremental adoption.** A check opts in by honouring "custom → std::X" via one
+ shared helper; no per-check semantic re-modelling.
+
+## Honest limits (state these too)
+
+- Class-comparison requires the custom type to genuinely mirror a std class, and
+ its members to map to the std members (that is the `analyze_as_method` layer).
+- A truly novel abstraction with no std analogue is not served — but that is rare
+ and arguably out of scope.
+- The fix-it must still emit the custom type's real method names; that cost is
+ shared by both designs.
+
+## Recommendation
+
+Anchor to canonical classes. Keep the annotation as an **identity declaration**
+that lets checks reuse their existing per-type knowledge, and treat cross-check
+adoption as an incremental roadmap (`plan_general.md` §12), not a generic
+rewrite. The `inefficient-algorithm` example is the clearest argument: the
+knowledge that makes it work cannot be cheaply externalised into an attribute —
+it *is* the std class.
>From 263b0880e4b0bd638c3105d317a8b591238a6739 Mon Sep 17 00:00:00 2001
From: khickett <khicketts at bloomberg.net>
Date: Wed, 5 Aug 2026 16:19:20 +0100
Subject: [PATCH 3/5] .
---
hicketts/hicketts_vector.h | 137 +++++++++++++++---------------
hicketts/test_hicketts_vector.cpp | 58 +++++++++----
2 files changed, 110 insertions(+), 85 deletions(-)
diff --git a/hicketts/hicketts_vector.h b/hicketts/hicketts_vector.h
index 08cd3b49ec21c..f06f75d497374 100644
--- a/hicketts/hicketts_vector.h
+++ b/hicketts/hicketts_vector.h
@@ -1,47 +1,59 @@
#ifndef HICKETTS_VECTOR_H_
#define HICKETTS_VECTOR_H_
-/// A minimal std::vector-like container for exercising two families of
-/// attribute at once:
+/// A minimal std::vector-like container exercising two families of attribute,
+/// sharing ONE predicate vocabulary with hicketts_optional_hybrid.h.
///
-/// * EXISTING, working attributes (compile today, drive real analysis):
+/// * REAL, shipping-today attributes (inline; drive analysis now):
/// [[gsl::Owner]] / [[gsl::Pointer]] -> -Wdangling lifetime analysis
/// [[clang::lifetimebound]] -> return tied to *this
/// [[clang::reinitializes]] -> "returns object to valid state"
///
-/// * PROPOSED per-object-state role attributes (do NOT exist yet; shown
-/// commented so the header stays buildable). These illustrate the closed
-/// role vocabulary discussed in architecture.md section 4.
+/// * PROPOSED predicate roles (do NOT exist yet; guarded by -DHV_ROLES):
+/// the SAME four-verb vocabulary as the optional fixture, over a single
+/// model-opaque predicate. The predicate is unnamed -- it is inert to the
+/// model ("empty" == "disengaged" == set-false), so a name would only matter
+/// if a type had >1 predicate. What the vector adds -- and the optional
+/// fixture cannot show -- is POLARITY:
+/// engaged push_back set the bit true
+/// disengaged clear set the bit false
+/// assume_engaged front/back/pop_back precondition: bit true
+/// test_disengaged empty() returns true iff bit FALSE <-- negative
+/// Compare optional's has_value(), a test_engaged (positive). Same bit,
+/// opposite-polarity query -- which is exactly why "non_empty" never needed
+/// to be a distinct predicate from "engaged": only the method polarity differs.
///
-/// The attributes are macro-guarded so the SAME fixture can be compiled two
-/// ways, for a clean before/after:
-/// -DHICKETTS_VECTOR_NO_ATTRS -> baseline, no attributes
-/// (default) -> attributes on
+/// Out of scope (deliberately NOT predicate roles):
+/// * operator[](i) -- a NUMERIC invariant (i < size), not a per-object bit.
+/// * iterator invalidation -- a RELATIONAL hazard, stays with Owner/Pointer.
namespace mylib {
-#ifdef HICKETTS_VECTOR_NO_ATTRS
-#define HV_OWNER
-#define HV_POINTER
-#define HV_LIFETIMEBOUND
-#define HV_REINITIALIZES
+// Proposed predicate roles -- shared spelling with hicketts_optional_hybrid.h.
+// The only guarded layer, because these attributes do not exist yet. Each
+// fixture aliases only the subset of verbs it uses; the vocabulary is shared.
+#ifdef HV_ROLES
+#define HV_ENGAGED [[clang::engaged]]
+#define HV_DISENGAGED [[clang::disengaged]]
+#define HV_ASSUME_ENGAGED [[clang::assume_engaged]]
+#define HV_TEST_DISENGAGED [[clang::test_disengaged]]
#else
-#define HV_OWNER [[gsl::Owner]]
-#define HV_POINTER [[gsl::Pointer]]
-#define HV_LIFETIMEBOUND [[clang::lifetimebound]]
-#define HV_REINITIALIZES [[clang::reinitializes]]
+#define HV_ENGAGED
+#define HV_DISENGAGED
+#define HV_ASSUME_ENGAGED
+#define HV_TEST_DISENGAGED
#endif
template <typename T>
-class HV_OWNER HickettsVector {
- // Tiny fixed buffer keeps the fixture simple (no allocator); big enough for
- // small tests, and irrelevant to the static lifetime analysis anyway.
+class [[gsl::Owner]] HickettsVector {
+ // Tiny fixed buffer keeps the fixture simple (no allocator); irrelevant to the
+ // static analysis anyway.
T buf_[16] = {};
unsigned size_ = 0;
public:
- // A pointer-like handle INTO the container. Marked gsl::Pointer so the
- // lifetime analysis knows it can dangle once the owning vector dies.
- class HV_POINTER iterator {
+ // A pointer-like handle INTO the container. gsl::Pointer lets the lifetime
+ // analysis know it can dangle once the owning vector dies.
+ class [[gsl::Pointer]] iterator {
T *p_ = nullptr;
public:
@@ -59,56 +71,41 @@ class HV_OWNER HickettsVector {
HickettsVector() = default;
// --- Element access -------------------------------------------------------
- // lifetimebound: the returned reference/iterator is tied to *this, so
- // -Wdangling fires when *this is a temporary. This is the RELATIONAL hazard
- // (container <-> derived handle) and is fully covered by Owner/Pointer +
- // lifetimebound -- no per-method role needed.
- T &front() HV_LIFETIMEBOUND { return buf_[0]; }
- T &back() HV_LIFETIMEBOUND { return buf_[size_ - 1]; }
- T &operator[](unsigned i) HV_LIFETIMEBOUND { return buf_[i]; }
+ // assume_engaged: precondition that the vector is non-empty (front/back on an
+ // empty vector is UB) -- the SAME role as optional's value(). lifetimebound is
+ // the orthogonal RELATIONAL hazard (handle tied to *this).
+ HV_ASSUME_ENGAGED T &front() [[clang::lifetimebound]] { return buf_[0]; }
+ HV_ASSUME_ENGAGED T &back() [[clang::lifetimebound]] { return buf_[size_ - 1]; }
- iterator begin() HV_LIFETIMEBOUND { return iterator(buf_); }
- iterator end() HV_LIFETIMEBOUND { return iterator(buf_ + size_); }
+ // operator[]: NO predicate role. Its precondition is numeric (i < size), which
+ // a single-bit predicate model does not track. lifetimebound still applies.
+ T &operator[](unsigned i) [[clang::lifetimebound]] { return buf_[i]; }
+
+ // Iterators: no predicate role (begin() == end() on an empty vector is
+ // well-defined); lifetimebound handles the dangling hazard.
+ iterator begin() [[clang::lifetimebound]] { return iterator(buf_); }
+ iterator end() [[clang::lifetimebound]] { return iterator(buf_ + size_); }
// --- State transitions ---------------------------------------------------
- // reinitializes: clear() returns the object to a defined (empty) state. This
- // already applies to both vector::clear() and optional::reset(), and is a
- // real "makes valid" role attribute shipping today.
- HV_REINITIALIZES void clear() { size_ = 0; }
-
- // Mutators. In a real std::vector these INVALIDATE existing iterators and
- // references -- a relational hazard that Owner/Pointer models via lifetime,
- // but which the per-object role attributes below CANNOT express (there is no
- // single per-object bit meaning "every handle I handed out is now stale").
- void push_back(const T &v) { buf_[size_++] = v; }
- void pop_back() { --size_; }
+ // clear(): disengaged (proposed predicate role) + reinitializes (real, ships
+ // today, consumed by other checks). Two different consumers -- NOT redundant.
+ HV_DISENGAGED [[clang::reinitializes]] void clear() { size_ = 0; }
+
+ // push_back -> engaged (the bit becomes true).
+ HV_ENGAGED void push_back(const T &v) { buf_[size_++] = v; }
+
+ // pop_back: assume_engaged (precondition non-empty) but leaves the bit UNKNOWN
+ // afterwards -- it may or may not still be non-empty -- so it carries no set
+ // role. (In a real vector this also invalidates handles: a RELATIONAL hazard
+ // modelled by Owner/Pointer, not by any per-object bit.)
+ HV_ASSUME_ENGAGED void pop_back() { --size_; }
// --- Queries -------------------------------------------------------------
- unsigned size() const { return size_; }
- bool empty() const { return size_ == 0; }
-
- // --- PROPOSED per-object-state role attributes (NOT YET IMPLEMENTED) ------
- // Shown commented; enabling them requires adding the attributes first.
- // Spellings are illustrative only -- see architecture.md section 4 for the
- // "closed role vocabulary" vs "capability-style" options still open.
- //
- // front()/back()/pop_back() carry a precondition: the vector is non-empty.
- // [[clang::requires_state("non_empty")]] T &front() ...
- // [[clang::requires_state("non_empty")]] void pop_back() ...
- //
- // Transitions that establish a state:
- // [[clang::sets_state("empty")]] void clear() ...
- // [[clang::sets_state("non_empty")]] void push_back(const T &) ...
- //
- // WHY THIS IS THE INTERESTING TEST:
- // * empty / non-empty is a SINGLE per-object predicate -- exactly the shape
- // of optional's has_value -- so it fits the capability/role model, and a
- // "requires non_empty" on front() is the direct analog of value()
- // requiring engaged.
- // * iterator invalidation is RELATIONAL, so it does NOT fit a per-object
- // bit and stays with Owner/Pointer. That boundary is the constraint we
- // wanted to surface: role attributes generalise to the state-predicate
- // slice of a container, not to its aliasing hazards.
+ unsigned size() const { return size_; } // numeric, no role
+
+ // empty(): test_disengaged -- returns true iff the bit is FALSE. This is the
+ // negative-polarity query that optional's has_value() (test_engaged) is not.
+ HV_TEST_DISENGAGED bool empty() const { return size_ == 0; }
};
} // namespace mylib
diff --git a/hicketts/test_hicketts_vector.cpp b/hicketts/test_hicketts_vector.cpp
index 9cc29515a4442..28a591ff0dbcf 100644
--- a/hicketts/test_hicketts_vector.cpp
+++ b/hicketts/test_hicketts_vector.cpp
@@ -1,20 +1,19 @@
-// Test fixture for the vector-like lifetime/role attribute experiment.
+// Test fixture for the vector-like lifetime + predicate-role experiment.
//
-// Baseline (NO attributes):
+// Real attributes are inline and always on. Default run:
// build-llvm/bin/clang-tidy hicketts/test_hicketts_vector.cpp -- \
-// -std=c++17 -Ihicketts -DHICKETTS_VECTOR_NO_ATTRS
+// -std=c++17 -Ihicketts
//
-// With attributes on:
+// + proposed predicate roles (needs the role attributes implemented):
// build-llvm/bin/clang-tidy hicketts/test_hicketts_vector.cpp -- \
-// -std=c++17 -Ihicketts
+// -std=c++17 -Ihicketts -DHV_ROLES
#include "hicketts_vector.h"
using mylib::HickettsVector;
-// --- Relational hazard: handles that outlive the container ------------------
-// With gsl::Owner/gsl::Pointer + lifetimebound these should warn (-Wdangling).
-// Baseline (no attributes) cannot know and stays silent.
+// --- L3 relational hazard: handles that outlive the container ---------------
+// gsl::Owner/Pointer + lifetimebound -> -Wdangling. Baseline stays silent.
int dangling_reference_from_temporary() {
int &r = HickettsVector<int>{}.front(); // r dangles: temporary destroyed here
@@ -42,13 +41,42 @@ int safe_iterator() {
return *it;
}
-// --- Precondition hazard (NOT covered by ANY current attribute) -------------
-// front()/pop_back() on an empty vector is UB. Neither the baseline nor the
-// Owner/Pointer attributes catch this -- it is the case the PROPOSED
-// requires_state("non_empty") role attribute would target. Kept here to show
-// the gap the role vocabulary is meant to fill.
+// --- L2 predicate precondition (needs -DHV_ROLES) ---------------------------
+// front()/pop_back() on an empty vector is UB. This is the assume_engaged role,
+// the direct analog of optional's value() requiring engaged. Neither the
+// baseline nor Owner/Pointer catches it -- the gap the roles are meant to fill.
int precondition_gap() {
- HickettsVector<int> v; // empty
- return v.front(); // UB today: no warning from any attribute
+ HickettsVector<int> v; // empty (disengaged)
+ return v.front(); // warn under -DHV_ROLES: assume_engaged not established
+}
+
+int safe_after_push() {
+ HickettsVector<int> v;
+ v.push_back(1); // engaged
+ return v.front(); // safe
+}
+
+int unsafe_after_clear() {
+ HickettsVector<int> v;
+ v.push_back(1);
+ v.clear(); // disengaged
+ return v.front(); // warn under -DHV_ROLES
+}
+
+// --- L2 polarity: empty() is a NEGATIVE-polarity test -----------------------
+// The optional fixture narrows via has_value() (true == engaged); the vector
+// narrows via empty() (true == disengaged). Same bit, opposite polarity -- both
+// must make the guarded front() safe. This is the case optional cannot show.
+
+int safe_guarded_by_not_empty(HickettsVector<int> &v) {
+ if (!v.empty())
+ return v.front(); // safe: !empty() -> engaged
+ return 0;
+}
+
+int safe_guarded_by_empty_early_return(HickettsVector<int> &v) {
+ if (v.empty())
+ return 0;
+ return v.front(); // safe: fallthrough -> engaged
}
>From 9d50c652a475aa0fd26c7326eca6f1a45b65c1b3 Mon Sep 17 00:00:00 2001
From: khickett <khicketts at bloomberg.net>
Date: Wed, 5 Aug 2026 16:21:08 +0100
Subject: [PATCH 4/5] .
---
hicketts/hicketts_optional_general.h | 116 -------------------
hicketts/test_hicketts_optional_general.cpp | 118 --------------------
2 files changed, 234 deletions(-)
delete mode 100644 hicketts/hicketts_optional_general.h
delete mode 100644 hicketts/test_hicketts_optional_general.cpp
diff --git a/hicketts/hicketts_optional_general.h b/hicketts/hicketts_optional_general.h
deleted file mode 100644
index be0ff94717272..0000000000000
--- a/hicketts/hicketts_optional_general.h
+++ /dev/null
@@ -1,116 +0,0 @@
-#ifndef HICKETTS_OPTIONAL_H_
-#define HICKETTS_OPTIONAL_H_
-
-/// A custom optional-like type with differently named functions.
-/// Mirrors std::optional semantics but uses its own vocabulary
-/// In order to test implementation of attributes for clang-tidy
-namespace mylib {
-
-struct nothing_t {
- constexpr explicit nothing_t() {}
-};
-
-constexpr nothing_t nothing;
-
-template <typename T>
-class [[clang::analyze_as_class("std::optional")]] HickettsOptional {
- T *storage_ = nullptr;
-
-public:
- // No matcher needed: default (0-arg) construction matches none of the
- // constructor cases, so has_value is left unconstrained and access is
- // conservatively treated as maybe-empty (warns).
- // [[clang::analyze_as_method("optional()")]]
- constexpr HickettsOptional() noexcept {}
-
- // KEEP (POC target): nothing_t is not std::nullopt_t, so
- // isOptionalNulloptConstructor (UncheckedOptionalAccessModel.cpp:288) misses
- // and this falls through to the value/conversion case (:300) -> wrongly
- // engaged. The new signature-matched constructor case will route this
- // annotation to the nullopt transfer (empty).
- [[clang::analyze_as_method("optional(std::nullopt_t)")]]
- constexpr HickettsOptional(nothing_t) noexcept {}
-
- // Already handled by isOptionalValueOrConversionConstructor (:300, registered
- // :1038): single-arg construction from a value -> engaged.
- // [[clang::analyze_as_method("optional(T&&)")]]
- constexpr HickettsOptional(T) noexcept {}
-
- // Copy ctor: no dedicated case; excluded from value/conversion (:302-303) and
- // handled by the framework's default record-copy, which propagates has_value
- // from the source.
- // [[clang::analyze_as_method("optional(const optional&)")]]
- HickettsOptional(const HickettsOptional &) = default;
-
- // Move ctor: same as copy — excluded from value/conversion (:302-303),
- // handled by the framework's default record-copy.
- // [[clang::analyze_as_method("optional(const optional&&)")]]
- HickettsOptional(HickettsOptional &&) = default;
-
- // Equivalent to std::optional::value()
- [[clang::analyze_as_method("value")]] const T &unwrap() const & { return *storage_; }
- [[clang::analyze_as_method("value")]] T &unwrap() & { return *storage_; }
- [[clang::analyze_as_method("value")]] const T &&unwrap() const && { return static_cast<const T &&>(*storage_); }
- [[clang::analyze_as_method("value")]] T &&unwrap() && { return static_cast<T &&>(*storage_); }
-
- const T &value() const & { return *storage_; }
- T &value() & { return *storage_; }
- const T &&value() const && { return static_cast<const T &&>(*storage_); }
- T &&value() && { return static_cast<T &&>(*storage_); }
-
- // Equivalent to std::optional::operator*()
- [[clang::analyze_as_method("value")]] const T &deref() const & { return *storage_; }
- [[clang::analyze_as_method("value")]] T &deref() & { return *storage_; }
-
- // Equivalent to std::optional::operator->()
- const T* operator ->() const { return storage_; }
- T* operator ->() { return storage_; }
- const T *arrow() const { return storage_; }
- T *arrow() { return storage_; }
-
- // Equivalent to std::optional::operator bool / hasValue()
- constexpr bool has_value() const noexcept { return storage_ != nullptr; }
- constexpr explicit operator bool() const noexcept { return storage_ != nullptr; }
- [[clang::analyze_as_method("has_value")]] constexpr bool isPresent() const noexcept { return storage_ != nullptr; }
-
- // Equivalent to std::optional::value_or()
- template <typename U>
- constexpr T unwrapOr(U &&fallback) const & {
- return storage_ ? *storage_ : static_cast<T>(fallback);
- }
-
- // Equivalent to std::optional::emplace()
- template <typename... Args>
- [[clang::analyze_as_method("emplace(Args&&...)")]]
- T& construct(Args&&... args) { return *storage_; }
-
- // Demo of malformed-signature rejection — disabled. The parameter-balance
- // validation in Sema (isValidAnalyzeAsMethodAttr) that rejected this string
- // was removed, since matching is now a flat string compare that never parses
- // parameters. With validation gone this annotation would be accepted silently
- // (and simply never match), so the case no longer demonstrates anything.
- // [[clang::analyze_as_method("emplace(oops))")]]
- // T& load() { return *storage_; }
-
- // Equivalent to std::optional::reset()
- [[clang::analyze_as_method("reset")]] void clear() noexcept { storage_ = nullptr; }
-
- // Equivalent to std::optional::swap()
- [[clang::analyze_as_method("swap")]] void exchange(HickettsOptional &other) noexcept {
- T *tmp = storage_;
- storage_ = other.storage_;
- other.storage_ = tmp;
- }
-
- // Assignment
- template <typename U>
- HickettsOptional &operator=(const U &u) { return *this; }
-
- [[clang::analyze_as_method("operator=(nullopt_t)")]]
- HickettsOptional &operator=(mylib::nothing_t){ storage_ = nullptr; return *this;}
-
-};
-
-} // namespace mylib
-
-#endif // HICKETTS_OPTIONAL_H_
diff --git a/hicketts/test_hicketts_optional_general.cpp b/hicketts/test_hicketts_optional_general.cpp
deleted file mode 100644
index 1ddea121ce5ab..0000000000000
--- a/hicketts/test_hicketts_optional_general.cpp
+++ /dev/null
@@ -1,118 +0,0 @@
-// Test cases for mylib::HickettsOptional — a custom optional-like type
-// with differently named functions.
-//
-// Run from hicketts/ with:
-// ../build-llvm/bin/clang-tidy -checks='bugprone-unchecked-optional-access' \
-// test_hicketts_optional_general.cpp -- -I . -std=c++17 -Wno-undefined-inline
-
-#include "hicketts_optional_general.h"
-
-// --- Unchecked access (should warn if the checker recognises HickettsOptional) ---
-
-static void uncheckedUnwrap(mylib::HickettsOptional<int> &Val) {
- Val.unwrap(); // unchecked access — may be empty
-}
-
-static void uncheckedValue(mylib::HickettsOptional<int> &Val) {
- Val.value(); // unchecked access — may be empty
-}
-
-static void uncheckedDeref(mylib::HickettsOptional<int> &Val) {
- Val.deref(); // unchecked access — may be empty
-}
-
-// --- Checked access (should NOT warn) ---
-
-static void checkedWithBool(mylib::HickettsOptional<int> &Val) {
- if (Val) {
- Val.unwrap(); // safe — checked via operator bool
- }
-}
-
-static void checkedValueWithBool(mylib::HickettsOptional<int> &Val) {
- if (Val.has_value()) {
- Val.value(); // safe — checked via operator bool
- }
-}
-
-static void checkedWithIsPresent(mylib::HickettsOptional<int> &Val) {
- if (Val.isPresent()) {
- Val.unwrap(); // safe — checked via isPresent()
- }
-}
-
-/* static void checkedWithIsEmpty(mylib::HickettsOptional<int> &Val) {
- if (!Val.isEmpty()) {
- Val.unwrap(); // safe — checked via !isEmpty()
- }
-} NYI */
-
-// --- State changes ---
-
-// construct() is annotated "emplace(Args&&...)"; the bare "emplace" query matches
-// it via the name-part (accept-either) branch -> engaged, so unwrap is safe.
-static void safeAfterConstruct(mylib::HickettsOptional<int> &Val) {
- Val.construct(42);
- Val.unwrap(); // safe — just constructed a value
-}
-
-static void unsafeAfterClear(mylib::HickettsOptional<int> &Val) {
- Val.construct(42);
- Val.clear();
- Val.unwrap(); // unsafe — value was cleared
-}
-
-static void unsafeAfterExchange(mylib::HickettsOptional<int> &A,
- mylib::HickettsOptional<int> &B) {
- if (A) {
- A.exchange(B);
- A.unwrap(); // unsafe — a's state is now unknown
- }
-}
-
-// Works today WITHOUT any annotation: default construction matches no
-// constructor case, so has_value is unconstrained -> access conservatively warns.
-static void unsafeAfterEmptyConstr() {
- mylib::HickettsOptional<int> A;
- A.unwrap(); // expected: warn (empty)
-}
-
-// nothing_t is not std::nullopt_t, so the structural nullopt matcher misses.
-// The "optional(std::nullopt_t)" annotation routes this constructor to the
-// nullopt transfer (empty) via isOptionalNulloptConstructor's annotation branch,
-// so the following unwrap is correctly flagged.
-static void unsafeAfterNullConstr() {
- mylib::HickettsOptional<int> A(mylib::nothing);
- A.unwrap(); // warns (empty) — routed to nullopt via the annotation
-}
-
-// Works today WITHOUT any annotation: value/conversion constructor case ->
-// engaged, so access is safe.
-static void safeAfterTypeConstr() {
- mylib::HickettsOptional<int> A(5);
- A.unwrap(); // expected: no warning (engaged)
-}
-
-// --- Guarded paths ---
-
-/*static void constructCoversEmptyBranch(mylib::HickettsOptional<int> &Val) {
- if (Val.isEmpty()) {
- Val.construct(99);
- }
- Val.unwrap(); // safe — either was present, or construct filled it
-}*/
-
-static void unwrapOrIsAlwaysSafe(mylib::HickettsOptional<int> &Val) {
- int X = Val.unwrapOr(0); // safe — fallback provided
- (void)X;
-}
-
-// nothing_t is not std::nullopt_t, so the structural nullopt matcher misses.
-// The "operator=(nullopt_t)" annotation routes this assignment to the nullopt
-// transfer (empty) — checked before the value/conversion-assignment case — so
-// the following unwrap is correctly flagged.
-static void unsafeAfterNullAssign() {
- mylib::HickettsOptional<int> A(5);
- A = mylib::nothing;
- A.unwrap(); // warns (empty) — routed to nullopt via the annotation
-}
>From 6bfd9e56535c86233ced9a9692b503200729ef33 Mon Sep 17 00:00:00 2001
From: khickett <khicketts at bloomberg.net>
Date: Wed, 5 Aug 2026 16:22:14 +0100
Subject: [PATCH 5/5] .
---
hicketts/plan_general.md | 225 ---------------------------------------
1 file changed, 225 deletions(-)
delete mode 100644 hicketts/plan_general.md
diff --git a/hicketts/plan_general.md b/hicketts/plan_general.md
deleted file mode 100644
index 0352b77e0ffff..0000000000000
--- a/hicketts/plan_general.md
+++ /dev/null
@@ -1,225 +0,0 @@
-# Plan: role-attribute approach (branch `generalAttributesKay`)
-
-Kickoff plan for the *alternate* proposal. This branch is a clean-room from
-`main` (no `analyze_as_*` string-match implementation). The goal is a **single,
-closed role-attribute vocabulary** that models one per-object boolean predicate,
-serving BOTH target types with the same engine.
-
-Companion docs: `architecture.md` (how the optional pipeline works + the
-levels-of-validation and two-worlds framing). The string-match MVP lives on
-`attributesKay`; this is deliberately separate, not a rework.
-
-Learning-exercise rule still applies: this plan is design/scope only. Do not
-implement the compiler changes; fixtures (`hicketts/*_general.*`,
-`hicketts_vector.*`) are fair game.
-
----
-
-## 1. Thesis
-
-Model each supported class as having **one named boolean predicate**, and let
-method-level *role* attributes say how each method relates to it. This unifies:
-
-- **optional** — predicate `engaged` (== today's `has_value`). Replaces the
- `analyze_as_method("...")` string keys with roles.
-- **vector** — predicate `non_empty`. Fills the `precondition_gap` measured in
- the vector experiment (empty `front()`/`pop_back()` is UB and caught by
- *nothing* today — not the baseline, not Owner/Pointer).
-
-Same dataflow question in both cases: "is the predicate established on this path
-before a method that requires it?" That is exactly what the
-`bugprone-unchecked-optional-access` model already answers for `has_value` — so
-the core implementation idea is to **generalise that model from the hardcoded
-`has_value` field to an arbitrary named predicate.**
-
-## 2. Why roles beat verbatim signatures (recap of the decision)
-
-- **Identity vs role.** The string signature described how to *identify* a method
- (its params). But overload resolution + the attribute sitting on one specific
- decl already identify it. What the model actually needs is the method's *role*.
-- **Per-decl placement disambiguates overloads** — no signature strings needed.
-- **Header-free.** Roles never resolve the real `std::optional`/`std::vector`, so
- `#include` is irrelevant (see `architecture.md` §4, the `<optional>` blocker).
-
-## 3. Precedents to ride (all in-tree — cite these in the RFC)
-
-- **Capability / thread-safety attributes** (`Attr.td:4101`–`4184`):
- `RequiresCapability`, `AcquireCapability`, `ReleaseCapability`. This is
- literally requires-valid / makes-valid / makes-invalid as a **closed,
- capability-scoped** role vocabulary — the exact shape we want, already accepted.
-- **`reinitializes`** (`:4877`) — "returns object to a defined state"; already
- applies to both `optional::reset()` and `vector::clear()`.
-- **Consumed / typestate attributes** (`:4285`–`:4366`) — `Consumable`,
- `SetTypestate`, `CallableWhen`, `TestTypestate`. Cite as evidence typestate is
- acceptable in clang, but frame OUR proposal as the *narrower capability shape*,
- NOT general typestate, to avoid the earlier rejection.
-
-## 4. Scope boundary (state it up front in the RFC)
-
-Role attributes model a **single per-object state predicate**. In scope:
-
-- optional `engaged`; vector `non_empty`; reset-to-valid (`reinitializes`).
-
-Explicitly OUT of scope (and why):
-
-- **Relational / aliasing hazards** (iterator invalidation: `push_back` stales
- existing iterators). Not a per-object bit — stays with `Owner`/`Pointer`
- (which already handle it; see the vector experiment: dangling *was* caught).
-- **Numeric invariants** (`size`/`capacity` relationships). The model tracks a
- predicate, not a quantity.
-
-## 5. Measured motivation (vector experiment, recorded here)
-
-`test_hicketts_vector.cpp`, built with `build-llvm/bin/clang++`
-`--target=arm64-apple-darwin -std=c++17`:
-
-| Case | Baseline (attrs off) | Owner/Pointer + lifetimebound |
-|------|----------------------|-------------------------------|
-| `front()` of a temporary (dangling ref) | silent | ⚠ `-Wdangling` |
-| `begin()` of a temporary (dangling iter) | silent | ⚠ `-Wdangling` |
-| ref/iter into a live vector | silent | silent ✓ |
-| **`front()` on an EMPTY vector (UB)** | **silent** | **silent** ← the gap |
-
-The empty-access row is what the `requires_state("non_empty")` role must make
-warn.
-
-## 6. Proposed vocabulary (DRAFT — open for iteration)
-
-Two axes still open; capture both, pick during RFC:
-
-- **Predicate naming:** string (`"engaged"`, `"non_empty"`) vs a fixed enum vs a
- single implicit predicate per class. Capability analysis names its capability,
- so a small **closed string/enum** is precedented and probably best.
-- **Class-level opt-in:** reuse an `analyze_as_class`-style marker to declare the
- class is state-tracked and name its predicate.
-
-Draft method roles (map straight onto existing optional transfer functions):
-
-| Role (draft spelling) | Meaning | optional example | vector example | model action |
-|---|---|---|---|---|
-| `requires_state("P")` | precondition: P must hold, else warn | `value()`/`unwrap()` | `front()`/`pop_back()` | diagnose if P not established |
-| `sets_state("P")` | establishes P true | value ctor, `emplace` | `push_back` | set predicate true |
-| `clears_state("P")` | establishes P false | nullopt ctor, `reset` | `clear` (+`reinitializes`) | set predicate false |
-| `queries_state("P")` | narrows P in flow | `has_value`/`operator bool` | `empty()` | branch-sensitive refine |
-
-Note: `requires`/`sets`/`clears`/`queries` ≈ `REQUIRES`/`ACQUIRE`/`RELEASE`/(test)
-from thread-safety — keep the analogy explicit.
-
-## 7. Validation strategy (from architecture.md §4)
-
-- **L1** validate the role/predicate name against a closed table (typo-catch),
- in Sema at parse time — header-free.
-- **L2** validate the annotated method's arity/shape if useful — also parse-time
- (the method decl is available to the Sema handler).
-- **L3** (verify custom param types vs the real std type) — skip; needs the
- header and buys little. This is the whole point of NOT going the Plan A route.
-
-## 8. Implementation sketch (design only — do NOT build yet)
-
-1. `Attr.td` — add the class-level predicate marker + the method role attributes
- (model on the capability attribute defs at `:4101`+).
-2. Sema — handlers + L1/L2 validation (mirror `handleAnalyzeAs*`; capability
- handlers are a closer template).
-3. Model — the crux: generalise `UncheckedOptionalAccessModel` so the synthetic
- boolean field is a *named predicate* rather than hardcoded `has_value`
- (`:1330`, `:441`–`:445`), and drive the match-switch cases from the role
- attributes instead of hardcoded method names.
-4. Decide: extend `bugprone-unchecked-optional-access` to arbitrary predicates,
- or spin a sibling check for the general "state precondition" analysis. (Open.)
-
-## 9. Test plan
-
-- **optional** — re-annotate `hicketts_optional_general.h` with the new roles;
- `test_hicketts_optional_general.cpp` should reproduce the MVP's behaviour
- (the same set of expected warnings/silences the string-match version produced).
-- **vector** — add empty-access cases to `test_hicketts_vector.cpp`; the
- `precondition_gap` case must now warn, while the dangling cases keep warning
- via Owner/Pointer and safe cases stay silent.
-
-## 10. Open questions
-
-- Predicate naming: string vs enum vs fixed-per-class.
-- One predicate per class, or several (e.g. a type with two independent states)?
-- Diagnoser wording for "required state not established here."
-- Extend the optional check vs new check (§8.4).
-- Does branch-sensitive `queries_state` need more than the optional model already
- does for `has_value`/`operator bool`?
-
-## 11. Files (this branch)
-
-- `hicketts/hicketts_optional_general.h` / `test_hicketts_optional_general.cpp`
- — optional fixture (currently still carries old `analyze_as_*`; to be re-annotated).
-- `hicketts/hicketts_vector.h` / `test_hicketts_vector.cpp` — vector fixture
- (Owner/Pointer + lifetimebound live; proposed roles commented).
-- `hicketts/architecture.md` — pipeline map + design framings.
-- (later) `Attr.td`, Sema, model changes in the real tree.
-
----
-
-## 12. Cross-check consumption: shared vocabulary vs private hook (Valentyn / PR)
-
-**The question.** `analyze_as_*` is today a *private hook* consumed by ONE check
-(the optional dataflow model). Valentyn's `size()==0 -> isEmpty()` example assumes
-it is a *shared, cross-check vocabulary*: annotate a type once and every relevant
-check honours it, using the type's custom method names. These are very different
-scopes — the PR should pick a lane explicitly rather than leave it implied.
-
-**Worked second consumer — `readability-container-size-empty`
-(`ContainerSizeEmptyCheck.cpp`).** Recognises a container *purely structurally*:
-a `size`/`length` method AND a method literally named `empty` returning bool
-(`:150`, `:154`). On `x.size()==0` it warns + attaches a fix-it whose replacement
-text is hardcoded to `"empty()"` (`:275`-`281`). Consequence for
-`[[analyze_as_method("empty")]] bool isEmpty()`:
-- it does NOT fire (the empty method isn't named `empty`; the attribute is ignored), and
-- even if it did, the fix-it would emit `empty()`, not `isEmpty()`.
-
-To support it: (a) accept an `analyze_as_method("empty")` bool method as the
-"empty" role alongside `hasName("empty")`, and (b) emit the *matched method's real
-name* in the fix-it. Note it also requires `returns(booleanType())` -> the L2
-return-type validation matters here too (Valentyn's recurring return-type theme).
-
-**The landscape (survey of candidate consumers).** Every check below recognises
-its target by its OWN hardcoded structural logic (`hasName(...)`, hardcoded
-`"std::..."`); NONE read `analyze_as_*` today. Grouped by what they'd need:
-
-Container method-role consumers (need "which method is empty/find/data/..."):
-- `readability/ContainerSizeEmptyCheck` size()==0 -> empty()
-- `readability/ContainerContainsCheck` find()!=end() -> contains()
-- `readability/ContainerDataPointerCheck` &v[0] -> v.data()
-- `readability/SimplifySubscriptExprCheck`
-- `bugprone/StandaloneEmptyCheck` ignored empty() (meant clear()?)
-- `bugprone/InaccurateEraseCheck` erase-remove idiom
-- `bugprone/SizeofContainerCheck` sizeof(container) misuse
-- `performance/InefficientAlgorithmCheck` std::find on set -> member find
-- `modernize/LoopConvertCheck` begin/end -> range-for
-- `modernize/UseEmplaceCheck` push_back -> emplace_back
-- `modernize/ShrinkToFitCheck` swap-trick -> shrink_to_fit
-
-Optional-role consumers:
-- `bugprone/UncheckedOptionalAccessCheck` (current consumer)
-- `bugprone/OptionalValueConversionCheck`
-
-Smart-pointer-role consumers:
-- `modernize/Make{Unique,Shared,SmartPtr}Check`
-- `bugprone/{Unique,Shared,Smart}PtrArrayMismatchCheck`
-
-**Why the "we'll miss cases" worry is bounded.** The set is *enumerable*
-(~a dozen container checks + a few optional/smart-ptr), and consumption is
-*explicit per check* — a check either reads the vocabulary or it doesn't, so
-nothing is silently missed. You can't retrofit them all at once and shouldn't try.
-
-**Recommended architecture (do NOT big-bang):**
-1. Define the shared vocabulary as a CLOSED, DEMAND-DRIVEN role set — add a role
- only when a consumer needs it. Method-roles span more than the optional
- predicate (empty/find/contains/data/begin/end/...), so demand-driven or it
- balloons.
-2. Provide ONE reusable helper/matcher: "does type T play class-role R" / "does
- method M play method-role X", reading the attributes — so a check opts in with
- a small change instead of reinventing recognition.
-3. Each opting-in check must ALSO fix its FIX-IT to use the real method name (the
- container-size-empty `"empty()"` hardcoding is the canonical trap).
-4. Treat this survey as the ROADMAP; convert consumers incrementally.
-
-**For THIS PR:** keep scope to the dataflow correctness check; name
-`container-size-empty` as the concrete next consumer that proves the vocabulary is
-cross-check; record this landscape so no consumer is forgotten.
More information about the llvm-commits
mailing list