[llvm] add some test files for a more generalised attribute implementation proposal (PR #212730)
Kay Hicketts via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 29 03:16:57 PDT 2026
https://github.com/KHicketts created https://github.com/llvm/llvm-project/pull/212730
None
>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] 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
+}
More information about the llvm-commits
mailing list